Skip to content

fix(markdown-toolbar): fixed markdown formatting toolbar toggle behaviour and duplicate marker handling #39479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Subhooo5 wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix(markdown-toolbar): fixed markdown formatting toolbar toggle behaviour and duplicate marker handling #39479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cffcfed
fix(markdown-toolbar): proper toggle behavior, and marker handling
Subhooo5 Mar 9, 2026
46938d2
chore(changeset): add changeset for markdown toolbar fix (#39300)
Subhooo5 Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/hip-pans-hug.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Fix markdown formatting toolbar toggle behavior and marker handling
90 changes: 62 additions & 28 deletions apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { IMessage } from '@rocket.chat/core-typings';
import { Emitter } from '@rocket.chat/emitter';
import { Accounts } from 'meteor/accounts-base';
import { Tracker } from 'meteor/tracker';
import type { RefObject } from 'react';

import { limitQuoteChain } from './limitQuoteChain';
Expand Down Expand Up @@ -223,48 +222,83 @@ export const createComposerAPI = (
};

const wrapSelection = (pattern: string): void => {
const { selectionEnd = input.value.length, selectionStart = 0 } = input;
const initText = input.value.slice(0, selectionStart);
const selectedText = input.value.slice(selectionStart, selectionEnd);
const finalText = input.value.slice(selectionEnd, input.value.length);
const token = '{{text}}';
const i = pattern.indexOf(token);
if (i === -1) return;

const startPattern = pattern.slice(0, i);
const endPattern = pattern.slice(i + token.length);

const text = input.value;
let { selectionStart: start, selectionEnd: end } = input;

focus();

const startPattern = pattern.slice(0, pattern.indexOf('{{text}}'));
const startPatternFound = input.value.slice(selectionStart - startPattern.length, selectionStart) === startPattern;
const before = text.slice(0, start);
const selected = text.slice(start, end);
const after = text.slice(end);

if (startPatternFound) {
const endPattern = pattern.slice(pattern.indexOf('{{text}}') + '{{text}}'.length);
const endPatternFound = input.value.slice(selectionEnd, selectionEnd + endPattern.length) === endPattern;
const left = before.lastIndexOf(startPattern);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: lastIndexOf(startPattern) is not pair-aware: for symmetric delimiters like ** and ~~, it can match a closing delimiter instead of an opening one. For example, with the cursor between **foo** and **bar**, lastIndexOf('**') in before finds the closer of the first span, while indexOf('**') in after finds the opener of the second span, causing the code to believe the cursor is inside a formatted region and incorrectly unwrap across both spans. This needs balanced pair matching.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts, line 241:

<comment>`lastIndexOf(startPattern)` is not pair-aware: for symmetric delimiters like `**` and `~~`, it can match a *closing* delimiter instead of an opening one. For example, with the cursor between `**foo**` and `**bar**`, `lastIndexOf('**')` in `before` finds the closer of the first span, while `indexOf('**')` in `after` finds the opener of the second span, causing the code to believe the cursor is inside a formatted region and incorrectly unwrap across both spans. This needs balanced pair matching.</comment>

<file context>
@@ -223,48 +222,83 @@ export const createComposerAPI = (
-		if (startPatternFound) {
-			const endPattern = pattern.slice(pattern.indexOf('{{text}}') + '{{text}}'.length);
-			const endPatternFound = input.value.slice(selectionEnd, selectionEnd + endPattern.length) === endPattern;
+		const left = before.lastIndexOf(startPattern);
+		const rightRelative = after.indexOf(endPattern);
+		const right = rightRelative === -1 ? -1 : end + rightRelative;
</file context>
Fix with Cubic

const rightRelative = after.indexOf(endPattern);
const right = rightRelative === -1 ? -1 : end + rightRelative;

if (endPatternFound) {
insertText(selectedText);
input.selectionStart = selectionStart - startPattern.length;
input.selectionEnd = selectionEnd + endPattern.length;
const inside =
left !== -1 &&
right !== -1 &&
left + startPattern.length <= start &&
right >= end;

if (!document.execCommand?.('insertText', false, selectedText)) {
input.value = initText.slice(0, initText.length - startPattern.length) + selectedText + finalText.slice(endPattern.length);
}
if (inside) {
const unwrapStart = left;
const unwrapEnd = right + endPattern.length;

input.selectionStart = selectionStart - startPattern.length;
input.selectionEnd = input.selectionStart + selectedText.length;
triggerEvent(input, 'input');
triggerEvent(input, 'change');
const inner = text.slice(
left + startPattern.length,
right
);

focus();
return;
input.setSelectionRange(unwrapStart, unwrapEnd);

if (!document.execCommand?.('insertText', false, inner)) {
input.value =
text.slice(0, unwrapStart) +
inner +
text.slice(unwrapEnd);
}

const pos = unwrapStart;
input.setSelectionRange(pos, pos + inner.length);

triggerEvent(input, 'input');
triggerEvent(input, 'change');
focus();
return;
}

if (!document.execCommand?.('insertText', false, pattern.replace('{{text}}', selectedText))) {
input.value = initText + pattern.replace('{{text}}', selectedText) + finalText;
if (!selected && before.endsWith(startPattern)) {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Formatting toggle can delete the closing marker of an existing valid markdown span when the caret is immediately after it.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts, line 278:

<comment>Formatting toggle can delete the closing marker of an existing valid markdown span when the caret is immediately after it.</comment>

<file context>
@@ -223,48 +222,83 @@ export const createComposerAPI = (
 
-		if (!document.execCommand?.('insertText', false, pattern.replace('{{text}}', selectedText))) {
-			input.value = initText + pattern.replace('{{text}}', selectedText) + finalText;
+		if (!selected && before.endsWith(startPattern)) {
+			const newBefore = before.slice(0, before.length - startPattern.length);
+			input.value = newBefore + after;
</file context>
Fix with Cubic

const newBefore = before.slice(0, before.length - startPattern.length);
input.value = newBefore + after;
const pos = newBefore.length;
input.setSelectionRange(pos, pos);
triggerEvent(input, 'input');
triggerEvent(input, 'change');
focus();
return;
}
Comment on lines +241 to 287
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Delimiter matching is not pair-aware for symmetric markers.

For **, ~~, and `, lastIndexOf(startPattern) and before.endsWith(startPattern) can hit a closing delimiter just as easily as an opening one. With the caret after **foo** or between **foo** and **bar**, these branches will strip the closer or unwrap across both spans, producing malformed text. This needs real pair matching instead of nearest-token matching.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts` around
lines 241 - 287, The current logic using lastIndexOf(startPattern) and
before.endsWith(startPattern) treats symmetric delimiters (startPattern equal to
endPattern like "**", "~~", "`") as interchangeable opener/closer and can pick a
closing marker; replace this with pair-aware matching: implement a helper
(referenced from this block using startPattern, endPattern, left/right, before,
after) that scans backward through before and forward through after counting
delimiter occurrences or tokenizing so you only consider an opener if it pairs
correctly (e.g., for symmetric markers ensure an odd/even parity or find the
nearest delimiter that is not itself a closer by verifying surrounding chars),
then use that matching index instead of lastIndexOf and replace the
before.endsWith(startPattern) check with a call to this helper to decide whether
to unwrap or delete the opener. Ensure setSelectionRange, insert/replace logic
remains unchanged but driven by the new, pair-aware match.


input.selectionStart = selectionStart + pattern.indexOf('{{text}}');
input.selectionEnd = input.selectionStart + selectedText.length;
const wrapped = `${startPattern}${selected}${endPattern}`;

input.setSelectionRange(start, end);

if (!document.execCommand?.('insertText', false, wrapped)) {
input.value = before + wrapped + after;
}

const caret = start + startPattern.length;
input.setSelectionRange(caret, caret + selected.length);

triggerEvent(input, 'input');
triggerEvent(input, 'change');

focus();
};

Expand Down
29 changes: 9 additions & 20 deletions apps/meteor/app/ui-message/client/messageBox/messageBoxFormatting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ type TextButton = {
type PatternButton = {
icon: IconName;
pattern: string;
// text?: () => string | undefined;
command?: string;
link?: string;
} & FormattingButtonDefault;
Expand All @@ -35,19 +34,19 @@ export const formattingButtons: ReadonlyArray<FormattingButton> = [
{
label: 'Bold',
icon: 'bold',
pattern: '*{{text}}*',
pattern: '**{{text}}**',
command: 'b',
},
{
label: 'Italic',
icon: 'italic',
pattern: '_{{text}}_',
pattern: '\u200B_{{text}}_\u200B',
command: 'i',
},
{
label: 'Strikethrough',
icon: 'strike',
pattern: '~{{text}}~',
pattern: '~~{{text}}~~',
},
{
label: 'Inline_code',
Expand All @@ -57,14 +56,13 @@ export const formattingButtons: ReadonlyArray<FormattingButton> = [
{
label: 'Multi_line_code',
icon: 'multiline',
pattern: '```\n{{text}}\n``` ',
pattern: '```\n{{text}}\n```',
},
{
label: 'Link',
icon: 'link',
prompt: (composerApi: ComposerAPI) => {
const { selection } = composerApi;

const selectedText = composerApi.substring(selection.start, selection.end);

const onClose = () => {
Expand All @@ -73,10 +71,7 @@ export const formattingButtons: ReadonlyArray<FormattingButton> = [
};

const onConfirm = (url: string, text: string) => {
// Composer API can't handle the selection of the text while the modal is open
flushSync(() => {
onClose();
});
flushSync(() => onClose());
flushSync(() => {
composerApi.replaceText(`[${text}](${url})`, selection);
composerApi.setCursorToEnd();
Expand All @@ -90,17 +85,11 @@ export const formattingButtons: ReadonlyArray<FormattingButton> = [
label: 'KaTeX' as TranslationKey,
icon: 'katex',
text: () => {
if (!settings.peek('Katex_Enabled')) {
return;
}
if (settings.peek('Katex_Dollar_Syntax')) {
return '$$KaTeX$$';
}
if (settings.peek('Katex_Parenthesis_Syntax')) {
return '\\[KaTeX\\]';
}
if (!settings.peek('Katex_Enabled')) return;
if (settings.peek('Katex_Dollar_Syntax')) return '$$KaTeX$$';
if (settings.peek('Katex_Parenthesis_Syntax')) return '\\[KaTeX\\]';
},
link: 'https://khan.github.io/KaTeX/function-support.html',
condition: () => settings.watch('Katex_Enabled') ?? true,
},
] as const;
] as const;
57 changes: 34 additions & 23 deletions ...iews/room/composer/messageBox/MessageBoxFormattingToolbar/MessageBoxFormattingToolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MessageComposerAction } from '@rocket.chat/ui-composer';
import { memo } from 'react';
import { memo, useEffect, useState } from 'react';
import { useTranslation } from 'react-i18next';

import FormattingToolbarDropdown from './FormattingToolbarDropdown';
Expand All @@ -16,6 +16,26 @@ type MessageBoxFormattingToolbarProps = {

const MessageBoxFormattingToolbar = ({ items, variant = 'large', composer, disabled }: MessageBoxFormattingToolbarProps) => {
const { t } = useTranslation();
const [activeModes, setActiveModes] = useState<Record<string, boolean>>({});

useEffect(() => {
const textarea = composer.composerRef.current?.querySelector('textarea');
if (!textarea) return;

const handleInput = () => {
if (!textarea.value) {
setActiveModes({});
}
};

textarea.addEventListener('input', handleInput);
return () => textarea.removeEventListener('input', handleInput);
}, [composer]);

const toggleMode = (pattern: string, label: string) => {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: toggleMode calls composer.wrapSelection(pattern) unconditionally on both activation and deactivation. When deactivating, the cursor is inside the formatted span (e.g. **hello**), so wrapSelection's unwrap logic strips the formatting the user just typed. The deactivation path should only close the formatting mode (e.g. move the cursor past the closing marker) without calling wrapSelection.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/room/composer/messageBox/MessageBoxFormattingToolbar/MessageBoxFormattingToolbar.tsx, line 35:

<comment>`toggleMode` calls `composer.wrapSelection(pattern)` unconditionally on both activation and deactivation. When deactivating, the cursor is inside the formatted span (e.g. `**hello**`), so `wrapSelection`'s unwrap logic strips the formatting the user just typed. The deactivation path should only close the formatting mode (e.g. move the cursor past the closing marker) without calling `wrapSelection`.</comment>

<file context>
@@ -16,6 +16,26 @@ type MessageBoxFormattingToolbarProps = {
+		return () => textarea.removeEventListener('input', handleInput);
+	}, [composer]);
+
+	const toggleMode = (pattern: string, label: string) => {
+		composer.wrapSelection(pattern);
+		setActiveModes((prev) => ({ ...prev, [label]: !prev[label] }));
</file context>
Fix with Cubic

composer.wrapSelection(pattern);
setActiveModes((prev) => ({ ...prev, [label]: !prev[label] }));
};
Comment on lines +35 to +38
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

toggleMode() still removes the formatted text on the second click.

composer.wrapSelection() runs on both enable and disable. In apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts:251-275, that API unwraps when the caret is already inside a formatted span, so clicking Bold/Italic/etc. again turns **abc** back into abc instead of just ending the typing mode. The on/off path needs separate open/close handling.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@apps/meteor/client/views/room/composer/messageBox/MessageBoxFormattingToolbar/MessageBoxFormattingToolbar.tsx`
around lines 35 - 38, toggleMode currently always calls
composer.wrapSelection(pattern) which causes the createComposerAPI unwrap-path
to run when disabling (turning Bold/Italic off) and thus removes formatting from
existing text. Change toggleMode so it checks the current state for the label
and only calls composer.wrapSelection(pattern) when turning the mode on; when
turning the mode off simply update setActiveModes to false without calling
wrapSelection. Use the existing activeModes state (or the prev value in
setActiveModes) to decide enable vs disable, keeping the label/key logic the
same.


if (variant === 'small') {
const collapsedItems = [...items];
Expand All @@ -26,11 +46,14 @@ const MessageBoxFormattingToolbar = ({ items, variant = 'large', composer, disab
{'icon' in featuredFormatter && (
<MessageComposerAction
onClick={() =>
isPromptButton(featuredFormatter) ? featuredFormatter.prompt(composer) : composer.wrapSelection(featuredFormatter.pattern)
isPromptButton(featuredFormatter)
? featuredFormatter.prompt(composer)
: toggleMode(featuredFormatter.pattern, featuredFormatter.label)
}
icon={featuredFormatter.icon}
title={t(featuredFormatter.label)}
disabled={disabled}
pressed={!!activeModes[featuredFormatter.label]}
/>
)}
<FormattingToolbarDropdown composer={composer} items={collapsedItems} disabled={disabled} />
Expand All @@ -43,33 +66,21 @@ const MessageBoxFormattingToolbar = ({ items, variant = 'large', composer, disab
{items.map((formatter) =>
'icon' in formatter ? (
<MessageComposerAction
disabled={disabled}
icon={formatter.icon}
key={formatter.label}
data-id={formatter.label}
icon={formatter.icon}
title={t(formatter.label)}
onClick={(): void => {
if (isPromptButton(formatter)) {
formatter.prompt(composer);
return;
}
if ('link' in formatter) {
window.open(formatter.link, '_blank', 'rel=noreferrer noopener');
return;
}
composer.wrapSelection(formatter.pattern);
disabled={disabled}
pressed={!!activeModes[formatter.label]}
onClick={() => {
if (isPromptButton(formatter)) return formatter.prompt(composer);
if ('link' in formatter) return window.open(formatter.link, '_blank', 'rel=noreferrer noopener');
toggleMode(formatter.pattern, formatter.label);
}}
/>
) : (
<span key={formatter.label} {...(disabled && { style: { pointerEvents: 'none' } })} title={formatter.label}>
<a href={formatter.link} target='_blank' rel='noopener noreferrer'>
{formatter.text()}
</a>
</span>
),
) : null,
Comment on lines 66 to +80
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

The large toolbar now drops TextButton items entirely.

This branch renders only formatters with an icon, but apps/meteor/app/ui-message/client/messageBox/messageBoxFormatting.ts:84-94 still exposes KaTeX as a TextButton. Returning null here makes that action unreachable in the large variant, and the 'link' in formatter path below never runs for it.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@apps/meteor/client/views/room/composer/messageBox/MessageBoxFormattingToolbar/MessageBoxFormattingToolbar.tsx`
around lines 66 - 80, The toolbar mapping in MessageBoxFormattingToolbar
currently filters to only formatters with an 'icon' by checking 'icon' in
formatter inside items.map, which drops TextButton entries (e.g., KaTeX) so
their 'link' and prompt paths never run; update the rendering so items without
an 'icon' are not skipped — render a TextButton (or the appropriate button
component) for formatters missing an icon and wire its onClick to the same logic
used for MessageComposerAction (respecting isPromptButton(formatter), the 'link'
in formatter branch that calls window.open(...), and
toggleMode(formatter.pattern, formatter.label)) so TextButton actions become
reachable in the large variant.

)}
</>
);
};

export default memo(MessageBoxFormattingToolbar);
export default memo(MessageBoxFormattingToolbar);