Desktop, Mobile: Fix table editor losing and mis-rendering inline markdown (#16632)

packages/editor/CodeMirror/extensions/rendering/renderTables.test.ts

@@ -39,6 +39,13 @@ describe('renderTables', () => {
 		{ input: 'a **b** c', expected: 'a b c', inner: 'a <strong>b</strong> c' },
 		// Escaped pipes are unescaped for display.
 		{ input: 'a \\| b', expected: 'a | b', inner: 'a | b' },
+		// Nested markup renders both layers.
+		{ input: '**[label](https://example.com)**', expected: 'label', inner: '<strong><a href="https://example.com">label</a></strong>' },
+		{ input: '[**label**](https://example.com)', expected: 'label', inner: '<a href="https://example.com"><strong>label</strong></a>' },
+		{ input: '*[label](https://example.com)*', expected: 'label', inner: '<em><a href="https://example.com">label</a></em>' },
+		{ input: '**~~strike~~**', expected: 'strike', inner: '<strong><del>strike</del></strong>' },
+		// Code spans stay literal.
+		{ input: '`**not bold**`', expected: '**not bold**', inner: '<code>**not bold**</code>' },
 	])('renderInlineMarkdown should render $input', ({ input, expected, inner }) => {
 		const div = document.createElement('div');
 		renderInlineMarkdown(div, input);
@@ -202,4 +209,63 @@ describe('renderTables', () => {
 		}
 	});
 
+	test.each([
+		'**[label](https://example.com)**',
+		'**bold**',
+		'*italic*',
+		'`code`',
+	])('focus/blur cycles should not degrade cell markdown: %s', async (cellSource) => {
+		jest.useFakeTimers();
+		let editor: EditorView | null = null;
+		try {
+			const markdown = `| a | b |\n|---|---|\n| ${cellSource} | y |`;
+			editor = await createEditor(markdown);
+			document.body.appendChild(editor.dom);
+
+			// Each cycle re-renders the cell without editing it.
+			for (let i = 0; i < 3; i++) {
+				const cell = findCellTextDivs(editor)[2];
+				focusCell(cell);
+				cell.dispatchEvent(new Event('blur'));
+				jest.advanceTimersByTime(200);
+				jest.runOnlyPendingTimers();
+
+				const rendered = findCellTextDivs(editor)[2];
+				expect(rendered.classList.contains('cm-tw-raw')).toBe(false);
+			}
+
+			expect(editor.state.doc.toString()).toContain(cellSource);
+		} finally {
+			editor?.destroy();
+			jest.useRealTimers();
+		}
+	});
+
+	test('a blurred cell should not write its rendered text back to the document', async () => {
+		jest.useFakeTimers();
+		let editor: EditorView | null = null;
+		try {
+			editor = await createEditor('| a | b |\n|---|---|\n| **[x](http://e.com)** | y |');
+			document.body.appendChild(editor.dom);
+
+			const cell = findCellTextDivs(editor)[2];
+			focusCell(cell);
+			cell.dispatchEvent(new Event('blur'));
+			jest.advanceTimersByTime(200);
+			jest.runOnlyPendingTimers();
+
+			// An input event on the now-rendered cell, as a stray mutation
+			// would fire, must not harvest its stripped text.
+			const rendered = findCellTextDivs(editor)[2];
+			rendered.dispatchEvent(new Event('input'));
+			jest.advanceTimersByTime(700);
+			jest.runOnlyPendingTimers();
+
+			expect(editor.state.doc.toString()).toContain('**[x](http://e.com)**');
+		} finally {
+			editor?.destroy();
+			jest.useRealTimers();
+		}
+	});
+
 });

packages/editor/CodeMirror/extensions/rendering/renderTables.ts

@@ -27,6 +27,8 @@ const W = 'cm-tw';
 const CELL = 'cm-tw-c';
 const HDR = 'cm-tw-h';
 const CTX = 'cm-tw-ctx';
+// Marks a cell holding raw source. Only these may be read back into the model.
+const RAW = 'cm-tw-raw';
 
 // Cache for rendered table widget heights so CodeMirror can estimate
 // heights correctly for scroll position and coordinate mapping.
@@ -59,45 +61,54 @@ const escapeHtml = (s: string): string => {
 // shown as plain |. The assembled HTML is run through DOMPurify before
 // insertion, so unsafe URL schemes (javascript:, data:, ...) and any tags
 // or attributes that slipped through the regex are removed.
+// Wrapper contents recurse so nested markup works (**[label](url)** is a bold
+// link). Code spans do not: their contents are literal in markdown.
+const inlineMarkdownToHtml = (segment: string): string => {
+	// Single regex with alternatives, scanned left-to-right. Each branch
+	// captures its inner content. Single * and _ emphasis use word-
+	// boundary guards so identifiers like `foo_bar_baz` or `a*b*c` are
+	// not rendered as emphasis.
+	const re = /\*\*([\s\S]+?)\*\*|__([\s\S]+?)__|(?<![A-Za-z0-9])\*([^*]+)\*(?![A-Za-z0-9])|(?<![A-Za-z0-9])_([^_]+)_(?![A-Za-z0-9])|`([^`]+)`|~~([\s\S]+?)~~|\[([^\]]*)\]\(([^)\s]+)\)/g;
+	const parts: string[] = [];
+	let lastIdx = 0;
+	let m: RegExpExecArray | null;
+	while ((m = re.exec(segment)) !== null) {
+		if (m.index > lastIdx) {
+			parts.push(escapeHtml(segment.slice(lastIdx, m.index)));
+		}
+		if (m[1] !== undefined || m[2] !== undefined) {
+			parts.push(`<strong>${inlineMarkdownToHtml((m[1] ?? m[2])!)}</strong>`);
+		} else if (m[3] !== undefined || m[4] !== undefined) {
+			parts.push(`<em>${inlineMarkdownToHtml((m[3] ?? m[4])!)}</em>`);
+		} else if (m[5] !== undefined) {
+			parts.push(`<code>${escapeHtml(m[5])}</code>`);
+		} else if (m[6] !== undefined) {
+			parts.push(`<del>${inlineMarkdownToHtml(m[6])}</del>`);
+		} else {
+			parts.push(`<a href="${escapeHtml(m[8]!)}">${inlineMarkdownToHtml(m[7]!)}</a>`);
+		}
+		lastIdx = m.index + m[0].length;
+	}
+	if (lastIdx < segment.length) {
+		parts.push(escapeHtml(segment.slice(lastIdx)));
+	}
+	return parts.join('');
+};
+
 export const renderInlineMarkdown = (parent: HTMLElement, text: string) => {
 	// Normalise: escaped pipes → |, and split on literal <br> for soft breaks.
 	const normalised = text.replace(/\\\|/g, '|');
-	const segments = normalised.split(/<br\s*\/?>/i);
-	const parts: string[] = [];
-	for (let s = 0; s < segments.length; s++) {
-		if (s > 0) parts.push('<br>');
-		const segment = segments[s];
-		// Single regex with alternatives, scanned left-to-right. Each branch
-		// captures its inner content. Single * and _ emphasis use word-
-		// boundary guards so identifiers like `foo_bar_baz` or `a*b*c` are
-		// not rendered as emphasis.
-		const re = /\*\*([^*]+)\*\*|__([^_]+)__|(?<![A-Za-z0-9])\*([^*]+)\*(?![A-Za-z0-9])|(?<![A-Za-z0-9])_([^_]+)_(?![A-Za-z0-9])|`([^`]+)`|~~([^~]+)~~|\[([^\]]+)\]\(([^)\s]+)\)/g;
-		let lastIdx = 0;
-		let m: RegExpExecArray | null;
-		while ((m = re.exec(segment)) !== null) {
-			if (m.index > lastIdx) {
-				parts.push(escapeHtml(segment.slice(lastIdx, m.index)));
-			}
-			if (m[1] !== undefined || m[2] !== undefined) {
-				parts.push(`<strong>${escapeHtml((m[1] ?? m[2])!)}</strong>`);
-			} else if (m[3] !== undefined || m[4] !== undefined) {
-				parts.push(`<em>${escapeHtml((m[3] ?? m[4])!)}</em>`);
-			} else if (m[5] !== undefined) {
-				parts.push(`<code>${escapeHtml(m[5])}</code>`);
-			} else if (m[6] !== undefined) {
-				parts.push(`<del>${escapeHtml(m[6])}</del>`);
-			} else {
-				parts.push(`<a href="${escapeHtml(m[8]!)}">${escapeHtml(m[7]!)}</a>`);
-			}
-			lastIdx = m.index + m[0].length;
-		}
-		if (lastIdx < segment.length) {
-			parts.push(escapeHtml(segment.slice(lastIdx)));
-		}
-	}
-	parent.innerHTML = sanitizeHtml(parts.join(''));
+	const html = normalised
+		.split(/<br\s*\/?>/i)
+		.map(inlineMarkdownToHtml)
+		.join('<br>');
+	parent.innerHTML = sanitizeHtml(html);
 };
 
+// Stashed on the container so destroy() can reach toDOM()'s closure state.
+const teardownKey = Symbol('tableWidgetTeardown');
+type TableWidgetContainer = HTMLElement & { [teardownKey]?: ()=> void };
+
 class TableWidget extends WidgetType {
 	public constructor(
 		private tableText: string,
@@ -202,6 +213,9 @@ class TableWidget extends WidgetType {
 		let scrollbarDragging = false;
 		let lastFocusedTextDiv: HTMLElement | null = null;
 
+		// Disconnected on destroy so a detached DOM cannot still schedule syncs.
+		const cellObservers: MutationObserver[] = [];
+
 		// Debounced dispatch so the document source stays in sync with cell
 		// edits — important so the preview pane reflects in-cell changes
 		// (e.g. deleting an image) without waiting for blur or a structural
@@ -214,19 +228,14 @@ class TableWidget extends WidgetType {
 			}
 		};
 
-		// Sync the focused cell's current text into the table model. Other
-		// cells are kept in sync continuously via their oninput handler, so
-		// this is just a final read of whichever cell is being edited right
-		// now. Reading textContent of an unfocused cell would be wrong —
-		// rendered cells have stripped markdown markers (** etc).
+		// Sync the raw-mode cell's text into the model. Reading a rendered cell
+		// would drop one layer of markup per round-trip, since its textContent
+		// has the markers stripped (#16498).
 		const syncDirtyCells = () => {
-			const active = doc.activeElement as HTMLElement | null;
-			if (!active || !active.classList.contains('cm-tw-text')) return;
-			if (!container.contains(active)) return;
 			for (let ri = 0; ri < allCells.length; ri++) {
 				for (let ci = 0; ci < allCells[ri].length; ci++) {
 					const td = allCells[ri][ci].querySelector('.cm-tw-text') as HTMLElement;
-					if (td !== active) continue;
+					if (!td || !td.classList.contains(RAW)) continue;
 					const v = (td.textContent || '').trim().replace(/\n/g, '<br>').replace(/\|/g, '\\|');
 					const isH = ri === 0;
 					if (isH) table.header.cells[ci].content = v;
@@ -250,6 +259,37 @@ class TableWidget extends WidgetType {
 			// swap to the raw source so the user edits the markdown text.
 			renderInlineMarkdown(textDiv, text);
 
+			// Track IME composition so we don't rebuild the cell DOM
+			// mid-composition — rebuilding would cancel the IME and drop
+			// any in-progress candidates.
+			let isComposing = false;
+
+			// Some browsers do not fire `input` reliably when non-text nodes
+			// (e.g. <img>) are removed via Backspace inside contentEditable.
+			// A MutationObserver catches DOM-level changes that `input` misses.
+			const mo = new win.MutationObserver(() => {
+				if (isComposing) return;
+				if (textDiv.classList.contains(RAW)) scheduleLiveSync();
+			});
+			mo.observe(textDiv, { childList: true, characterData: true, subtree: true });
+			cellObservers.push(mo);
+
+			// Observer is suspended while swapping: the render is itself a
+			// childList mutation, which would otherwise schedule a sync.
+			const showRendered = (src: string) => {
+				mo.disconnect();
+				textDiv.classList.remove(RAW);
+				renderInlineMarkdown(textDiv, src);
+				mo.observe(textDiv, { childList: true, characterData: true, subtree: true });
+			};
+
+			const showRaw = (src: string) => {
+				mo.disconnect();
+				textDiv.textContent = src.replace(/\\\|/g, '|');
+				textDiv.classList.add(RAW);
+				mo.observe(textDiv, { childList: true, characterData: true, subtree: true });
+			};
+
 			// Push this cell's current edit-mode text into the table model.
 			// Called on every input so the model stays in sync even if a
 			// rebuild is triggered by an external event (image paste, toolbar
@@ -257,6 +297,7 @@ class TableWidget extends WidgetType {
 			// Not trimmed: an edge space the user just typed is real content
 			// and must stay visible while editing (see #15918).
 			const pushToModel = () => {
+				if (!textDiv.classList.contains(RAW)) return;
 				const v = (textDiv.textContent || '')
 					.replace(/\n/g, '<br>').replace(/\|/g, '\\|');
 				if (isHdr) table.header.cells[c].content = v;
@@ -277,6 +318,7 @@ class TableWidget extends WidgetType {
 			};
 
 			const scheduleLiveSync = () => {
+				if (!textDiv.classList.contains(RAW)) return;
 				pushToModel();
 				cancelLiveSync();
 				const offset = caretOffset();
@@ -330,10 +372,6 @@ class TableWidget extends WidgetType {
 				}, 500);
 			};
 
-			// Track IME composition so we don't rebuild the cell DOM
-			// mid-composition — rebuilding would cancel the IME and drop
-			// any in-progress candidates.
-			let isComposing = false;
 			textDiv.addEventListener('compositionstart', () => {
 				isComposing = true;
 				cancelLiveSync();
@@ -347,14 +385,6 @@ class TableWidget extends WidgetType {
 				if (isComposing) return;
 				scheduleLiveSync();
 			};
-			// Some browsers do not fire `input` reliably when non-text nodes
-			// (e.g. <img>) are removed via Backspace inside contentEditable.
-			// A MutationObserver catches DOM-level changes that `input` misses.
-			const mo = new win.MutationObserver(() => {
-				if (isComposing) return;
-				if (doc.activeElement === textDiv) scheduleLiveSync();
-			});
-			mo.observe(textDiv, { childList: true, characterData: true, subtree: true });
 
 			// Sync CM cursor to this cell so toolbar commands work, and
 			// swap the rendered DOM for the raw markdown source for editing.
@@ -367,7 +397,7 @@ class TableWidget extends WidgetType {
 				const src = isHdr
 					? table.header.cells[c]?.content ?? ''
 					: table.body[r - 1]?.cells[c]?.content ?? '';
-				textDiv.textContent = src.replace(/\\\|/g, '|');
+				showRaw(src);
 				// Place caret at end so typing appends (matches prior behaviour
 				// where cells started empty of selection).
 				const sel = doc.defaultView!.getSelection();
@@ -401,6 +431,9 @@ class TableWidget extends WidgetType {
 					// context menu action), the old container is detached.
 					// Do nothing — the rebuild already has the latest data.
 					if (!container.isConnected) return;
+					// Already re-rendered (second blur, or a race with this
+					// timer) — its textContent is no longer source.
+					if (!textDiv.classList.contains(RAW)) return;
 					const v = (textDiv.textContent || '').trim();
 					const orig = isHdr
 						? table.header.cells[c]?.content
@@ -426,8 +459,7 @@ class TableWidget extends WidgetType {
 					const src = isHdr
 						? table.header.cells[c]?.content ?? ''
 						: table.body[r - 1]?.cells[c]?.content ?? '';
-					textDiv.textContent = '';
-					renderInlineMarkdown(textDiv, src);
+					showRendered(src);
 				}, 80);
 			};
 
@@ -861,9 +893,21 @@ class TableWidget extends WidgetType {
 			container.classList.toggle('cm-tw-mod-link', overLink);
 		});
 
+		// Runs on every rebuild and on teardown. No dispatch here: destroy()
+		// runs inside CodeMirror's update cycle, where it is not allowed.
+		(container as TableWidgetContainer)[teardownKey] = () => {
+			for (const observer of cellObservers) observer.disconnect();
+			cellObservers.length = 0;
+			cancelLiveSync();
+		};
+
 		return container;
 	}
 
+	public destroy(dom: HTMLElement) {
+		(dom as TableWidgetContainer)[teardownKey]?.();
+	}
+
 	public ignoreEvent() { return true; }
 }