fix(Realime Composer): Various issues fixes

apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.spec.ts

@@ -5,10 +5,6 @@ jest.mock('../../../../client/lib/chats/uploads', () => ({
 	createUploadsAPI: () => ({}),
 }));
 
-jest.mock('../../../../client/lib/utils/renderEmoji', () => ({
-	getEmojiClassNameAndDataTitle: () => ({}),
-}));
-
 let innerTextDescriptor: PropertyDescriptor | undefined;
 let originalExecCommand: typeof document.execCommand | undefined;
 
@@ -114,7 +110,7 @@ describe('RichText Composer API - insertText', () => {
 
 		composer.insertText('hi');
 
-		expect(input.textContent).toBe('hi');
+		expect(input.textContent).toBe('hi\n');
 		expect(getSelectionRange(input)).toEqual({ selectionStart: 2, selectionEnd: 2 });
 	});
 
@@ -123,10 +119,28 @@ describe('RichText Composer API - insertText', () => {
 
 		composer.insertText('b');
 
-		expect(input.textContent).toBe('abc');
+		expect(input.textContent).toBe('abc\n');
 		expect(getSelectionRange(input)).toEqual({ selectionStart: 2, selectionEnd: 2 });
 	});
 
+	it('renders the markup instead of leaving it raw until the next keystroke', () => {
+		const { composer, input } = setupComposer('', { start: 0, end: 0 });
+
+		composer.insertText('*bold*');
+
+		expect(input.querySelector('strong')).not.toBeNull();
+		expect(input.textContent).toBe('*bold*\n');
+	});
+
+	it('keeps the surrounding markup rendered when inserting at the end', () => {
+		const { composer, input } = setupComposer('*bold*', { start: 6, end: 6 });
+
+		composer.insertText(' 😄');
+
+		expect(input.querySelector('strong')).not.toBeNull();
+		expect(input.textContent).toBe('*bold* 😄\n');
+	});
+
 	it('still inserts when execCommand reports success but changes nothing', () => {
 		const { execCommand } = document as unknown as { execCommand: () => boolean };
 		(document as unknown as { execCommand: () => boolean }).execCommand = () => true;
@@ -144,6 +158,35 @@ describe('RichText Composer API - insertText', () => {
 	});
 });
 
+describe('RichText Composer API - draft restore', () => {
+	afterEach(() => {
+		window.getSelection()?.removeAllRanges();
+		document.body.innerHTML = '';
+	});
+
+	it('renders the restored draft markup without waiting for a keystroke', () => {
+		const input = document.createElement('div');
+		input.contentEditable = 'true';
+		document.body.appendChild(input);
+
+		createRichTextComposerAPI(input, jest.fn(), '*bold*', Number.MAX_SAFE_INTEGER, {}, { current: null }, { rid: 'GENERAL' });
+
+		expect(input.querySelector('strong')).not.toBeNull();
+		expect(input.textContent).toBe('*bold*\n');
+	});
+
+	it('leaves the composer empty when there is no draft', () => {
+		const input = document.createElement('div');
+		input.contentEditable = 'true';
+		document.body.appendChild(input);
+
+		const composer = createRichTextComposerAPI(input, jest.fn(), '', Number.MAX_SAFE_INTEGER, {}, { current: null }, { rid: 'GENERAL' });
+
+		expect(input.textContent).toBe('');
+		expect(composer.text).toBe('');
+	});
+});
+
 describe('RichText Composer API - wrapSelection', () => {
 	afterEach(() => {
 		window.getSelection()?.removeAllRanges();

apps/meteor/app/ui-message/client/messageBox/createRichTextComposerAPI.ts

@@ -53,6 +53,14 @@ export const createRichTextComposerAPI = (
 			input.innerHTML = escapeHTML(text);
 		}
 
+		// The events below are synthetic, so resolveComposerBox ignores them and the markup would stay
+		// unrendered. Skip it while empty: rendering '' yields the renderer's trailing newline, which would
+		// leave `clear()` with a composer that is no longer empty.
+		if (input.innerText !== '') {
+			const { selectionStart: caretStart, selectionEnd: caretEnd } = getSelectionRange(input);
+			renderComposerContent(input, parseOptions, { selectionStart: caretStart, selectionEnd: caretEnd });
+		}
+
 		triggerEvent(input, 'input');
 		triggerEvent(input, 'change');
 

apps/meteor/app/ui-message/client/messageBox/messageStateHandler.spec.ts

@@ -1,15 +1,35 @@
-import { resolveComposerBox } from './messageStateHandler';
+import { renderComposerContent, resolveComposerBox } from './messageStateHandler';
+import { renderComposerMarkup } from './renderComposerMarkup';
 
 jest.mock('./renderComposerMarkup', () => ({
-	renderComposerMarkup: () => '<p>rendered</p>',
+	renderComposerMarkup: jest.fn(),
 }));
 
-describe('resolveComposerBox', () => {
-	afterEach(() => {
-		document.body.innerHTML = '';
-	});
+const renderMock = renderComposerMarkup as jest.MockedFunction<typeof renderComposerMarkup>;
+
+const mountInput = (text: string): HTMLDivElement => {
+	const input = document.createElement('div');
+	// jsdom does not implement innerText, which is what the composer reads.
+	Object.defineProperty(input, 'innerText', { value: text, writable: true, configurable: true });
+	document.body.appendChild(input);
+	return input;
+};
+
+const render = (input: HTMLDivElement): void => renderComposerContent(input, {}, { selectionStart: 0, selectionEnd: 0 });
+
+beforeEach(() => {
+	renderMock.mockReset();
+});
+
+afterEach(() => {
+	window.getSelection()?.removeAllRanges();
+	document.body.innerHTML = '';
+});
 
+describe('resolveComposerBox', () => {
 	it('ignores untrusted events so programmatic changes do not trigger a rerender', () => {
+		renderMock.mockReturnValue('<span>rendered</span>');
+
 		const input = document.createElement('div');
 		input.innerHTML = '<p>original</p>';
 		document.body.appendChild(input);
@@ -23,3 +43,52 @@ describe('resolveComposerBox', () => {
 		expect(input.innerHTML).toBe('<p>original</p>');
 	});
 });
+
+describe('renderComposerContent', () => {
+	it('passes the parsed source to the renderer so nodes without a renderer can recover their markup', () => {
+		renderMock.mockReturnValue('<span>a *b*\n</span>');
+
+		render(mountInput('a *b*'));
+
+		expect(renderMock).toHaveBeenCalledWith(expect.anything(), 'a *b*');
+	});
+
+	it('keeps the rendered markup when it holds the same text as the source', () => {
+		renderMock.mockReturnValue('<span>a <strong>b</strong>\n</span>');
+
+		const input = mountInput('a b');
+		render(input);
+
+		expect(input.querySelector('strong')).not.toBeNull();
+	});
+
+	it('falls back to the raw text when the render loses text', () => {
+		renderMock.mockReturnValue('<span>\n</span>');
+
+		const input = mountInput('- [ ] a task');
+		render(input);
+
+		expect(input.textContent).toBe('- [ ] a task');
+	});
+
+	it('falls back to the raw text when the render adds text', () => {
+		renderMock.mockReturnValue('<span>hello there\n</span>');
+
+		const input = mountInput('hello');
+		render(input);
+
+		expect(input.textContent).toBe('hello');
+	});
+
+	it('escapes the fallback so a lossy render cannot inject elements', () => {
+		renderMock.mockReturnValue('<span>\n</span>');
+
+		const text = 'x.com/<img src=x onerror=alert(1)><script>alert(2)</script>';
+		const input = mountInput(text);
+		render(input);
+
+		expect(input.querySelector('img')).toBeNull();
+		expect(input.querySelector('script')).toBeNull();
+		expect(input.textContent).toBe(text);
+	});
+});

apps/meteor/app/ui-message/client/messageBox/messageStateHandler.ts

@@ -1,44 +1,11 @@
 import { parse, type Options } from '@rocket.chat/message-parser';
+import { escapeHTML } from '@rocket.chat/string-helpers';
 
 import { renderComposerMarkup } from './renderComposerMarkup';
 import { getSelectionRange, setSelectionRange } from './selectionRange';
 
-// TODO: Investigate an issue where Slack style links are not working properly
-// This might have to do with the symbols < and > not resolving into websafe characters
-const protectLinks = (text: string): { output: string; matches: string[] } => {
-	const matches: string[] = [];
-	let idx = 0;
-
-	const patterns = [
-		// Markdown reference links
-		/\[([^\]]*)\]\((https?:\/\/[^\)]+)\)/g,
-
-		// Slack-style links
-		/<([^>|]+)\|([^>]+)>/g,
-
-		// Emails
-		/\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b/g,
-
-		// Bare domains / URLs (not after @ so we don't eat mentions)
-		/(?<!@)\b[a-z0-9-]+(?:\.[a-z0-9-]+)+(?:\/[^\s]*)?/gi,
-	];
-
-	let output = text;
-
-	patterns.forEach((regex) => {
-		output = output.replace(regex, (full) => {
-			const placeholder = `[[[LINK_${idx}]]]`;
-			matches[idx++] = full;
-			return placeholder;
-		});
-	});
-
-	return { output, matches };
-};
-
-const restoreLinks = (html: string, matches: string[]): string => {
-	return html.replace(/\[\[\[LINK_(\d+)\]\]\]/g, (_, i) => matches[parseInt(i, 10)] || '');
-};
+// Paragraphs render with a trailing '\n' the source does not have.
+const sameText = (rendered: string, source: string): boolean => rendered.replace(/\n$/, '') === source.replace(/\n$/, '');
 
 // Parse the composer's raw text into markup and render it into the contenteditable,
 // restoring the caret to the given flat-offset selection afterwards.
@@ -48,21 +15,16 @@ export const renderComposerContent = (
 	{ selectionStart, selectionEnd }: { selectionStart: number; selectionEnd: number },
 ): void => {
 	const text = target.innerText;
+	const source = text === '' ? '\n' : text;
 
-	// Extract the URL and substitue with a safe template
-	const { output: safeText, matches } = protectLinks(text === '' ? '\n' : text);
-
-	// Parse the safetext
-	const ast = parse(safeText, parseOptions);
-
-	// Render the AST through the gazzodown-alt WYSIWYG components
-	const html = renderComposerMarkup(ast);
-
-	// Restore the substituted links
-	const finalHtml = restoreLinks(html, matches);
+	// Parse the raw text and render the AST through the gazzodown-alt WYSIWYG components
+	target.innerHTML = renderComposerMarkup(parse(source, parseOptions), source);
 
-	// Rendering pipeline
-	target.innerHTML = finalHtml;
+	// Caret offsets are flat character counts over the rendered text, so a node without a renderer
+	// would both lose the user's text and shift the caret. Fall back to the raw text instead.
+	if (!sameText(target.textContent ?? '', source)) {
+		target.innerHTML = escapeHTML(text);
+	}
 
 	// Restore the cursor to the correct position
 	setSelectionRange(target, selectionStart, selectionEnd);

apps/meteor/app/ui-message/client/messageBox/renderComposerMarkup.spec.tsx

@@ -1,42 +1,89 @@
 import { parse } from '@rocket.chat/message-parser';
 
+import { renderComposerContent } from './messageStateHandler';
 import { renderComposerMarkup } from './renderComposerMarkup';
 import { getSelectionRange, setSelectionRange } from './selectionRange';
 
-jest.mock('../../../../client/lib/utils/renderEmoji', () => ({
-	getEmojiClassNameAndDataTitle: () => ({}),
-}));
-
 const mountMarkup = (text: string): HTMLDivElement => {
 	const input = document.createElement('div');
-	input.innerHTML = renderComposerMarkup(parse(text, {}));
+	input.innerHTML = renderComposerMarkup(parse(text, {}), text);
 	document.body.appendChild(input);
 	return input;
 };
 
+const mountComposer = (text: string): HTMLDivElement => {
+	const input = document.createElement('div');
+	// jsdom does not implement innerText, which is what the composer reads.
+	Object.defineProperty(input, 'innerText', { value: text, writable: true, configurable: true });
+	document.body.appendChild(input);
+	renderComposerContent(input, {}, { selectionStart: 0, selectionEnd: 0 });
+	return input;
+};
+
+const stripLineEnd = (text: string): string => text.replace(/\n$/, '');
+
 afterEach(() => {
 	window.getSelection()?.removeAllRanges();
 	document.body.innerHTML = '';
 });
 
-describe('caret round-trip on real rendered markup', () => {
+describe('text exactness and caret round-trip on real rendered markup', () => {
 	it.each([
-		['plain text', 'hello world', 'hello world\n'],
-		['bold', 'a *bold* b', 'a *bold* b\n'],
-		['italic', 'a _em_ b', 'a _em_ b\n'],
-		['inline code', 'a `code` b', 'a `code` b\n'],
-		['heading', '# Title', '# Title\n'],
-		['multiline', 'first\nsecond\nthird', 'first\nsecond\nthird\n'],
-	])('preserves every caret offset for %s', (_label, text, rendered) => {
+		['plain text', 'hello world'],
+		['bold', 'a *bold* b'],
+		['italic', 'a _em_ b'],
+		['strike', 'a ~out~ b'],
+		['spoiler', 'a ||hidden|| b'],
+		['inline code', 'a `code` b'],
+		['code block', '```js\nconst a = 1;\n```'],
+		['heading', '# Title'],
+		['heading with a link', '# see rocket.chat'],
+		['quote', '> quoted'],
+		['multiline', 'first\nsecond\nthird'],
+		['user mention', 'hi @rocket.cat'],
+		['channel mention', 'hi #general'],
+		['markdown link', 'see [the docs](https://rocket.chat/docs)'],
+		['bare domain', 'see rocket.chat/docs now'],
+		['email', 'mail me@rocket.chat now'],
+		['link inside bold', 'a *see rocket.chat* b'],
+		['image', 'look ![alt](https://rocket.chat/a.png)'],
+		['timestamp', 'at <t:1700000000:t> ok'],
+		['horizontal rule', '---'],
+		['horizontal rule between paragraphs', 'a\n---\nb'],
+		['table', '|a|b|\n|-|-|\n|1|2|'],
+		['hyphen list', '- one\n- two'],
+		['ordered list', '1. one\n2. two'],
+		['tasks', '- [x] done\n- [ ] todo'],
+		['emoji shortcode', 'hi :smile: there'],
+		['emoji shortcode alone', ':smile:'],
+		['unicode emoji in text', 'hi 😄 there'],
+		['unicode emoji alone', '😄'],
+	])('renders %s as its own text and preserves every caret offset', (_label, text) => {
 		const input = mountMarkup(text);
+		const rendered = input.textContent ?? '';
 
-		expect(input.textContent).toBe(rendered);
+		expect(stripLineEnd(rendered)).toBe(stripLineEnd(text));
 
-		const { length } = rendered;
-
-		for (let n = 0; n <= length; n++) {
+		for (let n = 0; n <= rendered.length; n++) {
 			setSelectionRange(input, n, n);
 			expect(getSelectionRange(input)).toEqual({ selectionStart: n, selectionEnd: n });
 		}
 	});
 });
+
+const lossy: [string, string][] = [
+	['asterisk list', '* one\n* two'],
+	['slack-style link', '<https://rocket.chat|docs>'],
+	['padded horizontal rule', '  ---'],
+	['several big emoji', '😄 😄'],
+];
+
+describe('markup the renderer cannot reproduce', () => {
+	it.each(lossy)('does not reproduce %s', (_label, text) => {
+		expect(stripLineEnd(mountMarkup(text).textContent ?? '')).not.toBe(stripLineEnd(text));
+	});
+
+	it.each(lossy)('keeps every character of %s through the text guard', (_label, text) => {
+		expect(stripLineEnd(mountComposer(text).textContent ?? '')).toBe(stripLineEnd(text));
+	});
+});