Desktop: Fixes #16454: In-editor Markdown rendering: Fix markup syntax is visible while clicking and dragging the scrollbar (#16461)

.gitignore

@@ -1177,6 +1177,7 @@ packages/editor/CodeMirror/pluginApi/PluginLoader.js
 packages/editor/CodeMirror/pluginApi/codeMirrorRequire.js
 packages/editor/CodeMirror/pluginApi/customEditorCompletion.test.js
 packages/editor/CodeMirror/pluginApi/customEditorCompletion.js
+packages/editor/CodeMirror/testing/backspaceOnce.js
 packages/editor/CodeMirror/testing/createEditorControl.js
 packages/editor/CodeMirror/testing/createTestEditor.js
 packages/editor/CodeMirror/testing/findNodesWithName.js
@@ -1186,6 +1187,8 @@ packages/editor/CodeMirror/testing/pressReleaseKey.js
 packages/editor/CodeMirror/testing/typeText.js
 packages/editor/CodeMirror/testing/visibleEditorText.js
 packages/editor/CodeMirror/theme.js
+packages/editor/CodeMirror/utils/clampSelectionToDocument.test.js
+packages/editor/CodeMirror/utils/clampSelectionToDocument.js
 packages/editor/CodeMirror/utils/findPositionMatchingLink.test.js
 packages/editor/CodeMirror/utils/findPositionMatchingLink.js
 packages/editor/CodeMirror/utils/formatting/RegionSpec.js

.ignore.eslint

@@ -1203,6 +1203,7 @@ packages/editor/CodeMirror/pluginApi/PluginLoader.js
 packages/editor/CodeMirror/pluginApi/codeMirrorRequire.js
 packages/editor/CodeMirror/pluginApi/customEditorCompletion.test.js
 packages/editor/CodeMirror/pluginApi/customEditorCompletion.js
+packages/editor/CodeMirror/testing/backspaceOnce.js
 packages/editor/CodeMirror/testing/createEditorControl.js
 packages/editor/CodeMirror/testing/createTestEditor.js
 packages/editor/CodeMirror/testing/findNodesWithName.js
@@ -1212,6 +1213,8 @@ packages/editor/CodeMirror/testing/pressReleaseKey.js
 packages/editor/CodeMirror/testing/typeText.js
 packages/editor/CodeMirror/testing/visibleEditorText.js
 packages/editor/CodeMirror/theme.js
+packages/editor/CodeMirror/utils/clampSelectionToDocument.test.js
+packages/editor/CodeMirror/utils/clampSelectionToDocument.js
 packages/editor/CodeMirror/utils/findPositionMatchingLink.test.js
 packages/editor/CodeMirror/utils/findPositionMatchingLink.js
 packages/editor/CodeMirror/utils/formatting/RegionSpec.js

packages/editor/CodeMirror/extensions/rendering/utils/makeInlineReplaceExtension.test.ts

@@ -6,6 +6,7 @@ import replaceFormatCharacters from '../replaceFormatCharacters';
 import replaceInlineHtml from '../replaceInlineHtml';
 import replaceLinks from '../replaceLinks';
 import visibleEditorText from '../../../testing/visibleEditorText';
+import backspaceOnce from '../../../testing/backspaceOnce';
 
 jest.retryTimes(2);
 
@@ -176,4 +177,37 @@ describe('makeInlineReplaceExtension', () => {
 			editor.destroy();
 		}
 	});
+
+	it('should update markdown decorations when the mouse is down and the document changes', async () => {
+		const markdown = '**test**...';
+		const initialSelection = EditorSelection.cursor(markdown.length);
+		const editor = await createTestEditor(
+			'**test**...',
+			initialSelection,
+			['StrongEmphasis'],
+			[replaceFormatCharacters],
+		);
+
+		try {
+			editor.dom.dispatchEvent(new MouseEvent('mousedown', { button: 0 }));
+			editor.dispatch({ selection: initialSelection, userEvent: 'select.pointer' });
+			jest.runAllTimers();
+
+			expectTextContentToBe(visibleEditorText(editor), 'test...');
+
+			backspaceOnce(editor);
+			expectTextContentToBe(visibleEditorText(editor), 'test..');
+			backspaceOnce(editor);
+			expectTextContentToBe(visibleEditorText(editor), 'test.');
+			backspaceOnce(editor);
+			expectTextContentToBe(visibleEditorText(editor), '**test**');
+
+			editor.dom.ownerDocument.dispatchEvent(new MouseEvent('mouseup', { button: 0 }));
+			await jest.runAllTimersAsync();
+
+			expectTextContentToBe(visibleEditorText(editor), '**test**');
+		} finally {
+			editor.destroy();
+		}
+	});
 });

packages/editor/CodeMirror/extensions/rendering/utils/makeInlineReplaceExtension.ts

@@ -4,21 +4,31 @@
 import { EditorView, Decoration, DecorationSet, WidgetType } from '@codemirror/view';
 import { ViewPlugin, ViewUpdate } from '@codemirror/view';
 import { syntaxTree } from '@codemirror/language';
-import { Range, StateEffect } from '@codemirror/state';
+import { EditorSelection, EditorState, Range, StateEffect } from '@codemirror/state';
 import { SyntaxNodeRef } from '@lezer/common';
 import { ReplacementExtension } from '../types';
 import nodeIntersectsSelection from './nodeIntersectsSelection';
+import clampSelectionToDocument from '../../../utils/clampSelectionToDocument';
 
 const updateInlineDecorationsEffect = StateEffect.define();
 
+interface MouseSelectionState {
+	initialSelection: EditorSelection;
+}
+
+interface VisibleRange {
+	from: number;
+	to: number;
+}
+
 export const makeInlineReplaceExtension = (extensionSpec: ReplacementExtension) => ViewPlugin.fromClass(class {
 	public decorations: DecorationSet = Decoration.set([]);
-	private mouseSelectionInProgress = false;
+	private mouseSelectionBefore_: MouseSelectionState|null = null;
 
 	public constructor(private view: EditorView) {
 		view.dom.addEventListener('mousedown', this.onMouseDown, true);
 		view.dom.ownerDocument.addEventListener('mouseup', this.onMouseUp);
-		this.updateDecorations(view);
+		this.updateDecorations(view.state, view.visibleRanges);
 	}
 
 	public destroy() {
@@ -28,16 +38,16 @@ export const makeInlineReplaceExtension = (extensionSpec: ReplacementExtension)
 
 	private onMouseDown = (event: MouseEvent) => {
 		if (event.button === 0) {
-			this.mouseSelectionInProgress = true;
+			this.mouseSelectionBefore_ = { initialSelection: this.view.state.selection };
 		}
 	};
 
 	private onMouseUp = () => {
-		if (this.mouseSelectionInProgress) {
+		if (this.mouseSelectionBefore_) {
 			// To prevent unnecessary scroll on iOS, decoration changes need to
 			// happen *after* the gesture ends.
 			requestAnimationFrame(() => {
-				this.mouseSelectionInProgress = false;
+				this.mouseSelectionBefore_ = null;
 				this.view.dispatch({
 					effects: updateInlineDecorationsEffect.of(null),
 				});
@@ -45,14 +55,20 @@ export const makeInlineReplaceExtension = (extensionSpec: ReplacementExtension)
 		}
 	};
 
-	private updateDecorations(view: EditorView) {
-		const doc = view.state.doc;
-		const cursorLine = doc.lineAt(view.state.selection.main.anchor);
-		const selection = view.state.selection;
+	private updateDecorations(state: EditorState, visibleRanges: readonly VisibleRange[]) {
+		const doc = state.doc;
+		let selection = state.selection;
+		if (this.mouseSelectionBefore_?.initialSelection) {
+			selection = clampSelectionToDocument(this.mouseSelectionBefore_.initialSelection, doc);
+		}
+		if (this.mouseSelectionBefore_) {
+			state = state.update({ selection }).state;
+		}
+		const cursorLine = doc.lineAt(selection.main.anchor);
 
 		const parentTagCounts = new Map<string, number>();
 		const decorateNode = (node: SyntaxNodeRef) => {
-			const widgetOrDecoration = extensionSpec.createDecoration(node, view.state, parentTagCounts);
+			const widgetOrDecoration = extensionSpec.createDecoration(node, state, parentTagCounts);
 			let decoration;
 			if (widgetOrDecoration instanceof WidgetType) {
 				decoration = Decoration.replace({
@@ -63,7 +79,7 @@ export const makeInlineReplaceExtension = (extensionSpec: ReplacementExtension)
 			}
 
 			if (decoration) {
-				const range = extensionSpec.getDecorationRange?.(node, view.state, parentTagCounts) ?? [node.from, node.to];
+				const range = extensionSpec.getDecorationRange?.(node, state, parentTagCounts) ?? [node.from, node.to];
 				const rangeLineFrom = doc.lineAt(range[0]);
 				const rangeLineTo = range.length === 2 ? doc.lineAt(range[1]) : rangeLineFrom;
 
@@ -79,14 +95,14 @@ export const makeInlineReplaceExtension = (extensionSpec: ReplacementExtension)
 		};
 
 		let widgets: Range<Decoration>[] = [];
-		for (const { from, to } of view.visibleRanges) {
+		for (const { from, to } of visibleRanges) {
 			parentTagCounts.clear();
-			syntaxTree(view.state).iterate({
+			syntaxTree(state).iterate({
 				from, to,
 				enter: node => {
 					parentTagCounts.set(node.name, (parentTagCounts.get(node.name) ?? 0) + 1);
 
-					const strategy = extensionSpec.getRevealStrategy?.(node, view.state, parentTagCounts) ?? 'line';
+					const strategy = extensionSpec.getRevealStrategy?.(node, state, parentTagCounts) ?? 'line';
 
 					let isSelected = false;
 					if (typeof strategy === 'boolean') {
@@ -151,12 +167,15 @@ export const makeInlineReplaceExtension = (extensionSpec: ReplacementExtension)
 			transaction.effects.some(effect => effect.is(updateInlineDecorationsEffect))
 			|| extensionSpec.shouldFullReRender?.(transaction)
 		));
-		if (this.mouseSelectionInProgress && !update.docChanged && !forceUpdate) {
-			return;
+
+		// Document changes move the selection, so the original selection may no longer
+		// be valid:
+		if (update.docChanged) {
+			this.mouseSelectionBefore_ = null;
 		}
 
 		if (update.docChanged || update.viewportChanged || update.selectionSet || forceUpdate) {
-			this.updateDecorations(update.view);
+			this.updateDecorations(update.state, update.view.visibleRanges);
 		}
 	}
 }, {

packages/editor/CodeMirror/testing/backspaceOnce.ts

@@ -0,0 +1,8 @@
+import pressReleaseKey from './pressReleaseKey';
+import { EditorView } from '@codemirror/view';
+
+const backspaceOnce = (view: EditorView) => {
+	pressReleaseKey(view, { key: 'Backspace', code: 'Backspace', typesText: '' });
+};
+
+export default backspaceOnce;

packages/editor/CodeMirror/testing/pressReleaseKey.ts

@@ -1,7 +1,7 @@
 import { EditorView } from '@codemirror/view';
 import typeText from './typeText';
 
-interface KeyInfo {
+export interface KeyInfo {
 	key: string;
 	code: string;
 	// Text to type if the event was not processed
@@ -21,8 +21,8 @@ const pressReleaseKey = (editor: EditorView, key: KeyInfo) => {
 
 	editor.contentDOM.dispatchEvent(keyDownEvent);
 
-	if (key.typesText && !keyDownPrevented) {
-		typeText(editor, key.typesText);
+	if (key.typesText !== undefined && !keyDownPrevented) {
+		typeText(editor, key.typesText, key);
 	}
 
 	editor.contentDOM.dispatchEvent(new KeyboardEvent('keyup', key));

packages/editor/CodeMirror/testing/typeText.ts

@@ -1,15 +1,26 @@
 import { EditorView } from '@codemirror/view';
+import { KeyInfo } from './pressReleaseKey';
 
-const typeText = (editor: EditorView, text: string) => {
+const typeText = (editor: EditorView, text: string, key?: KeyInfo) => {
 	const selection = editor.state.selection;
 	const inputHandlers = editor.state.facet(EditorView.inputHandler);
+	const isBackspace = !!key && key.key === 'Backspace' && text === '';
+
+	let backspaceOffset = 0;
+	if (isBackspace && selection.main.empty && selection.main.from > 0) {
+		backspaceOffset = -1;
+	}
 
 	// See how the upstream CodeMirror tests simulate user input:
 	//    https://github.com/codemirror/autocomplete/blob/fb1c899464df4d36528331412cdd316548134cb2/test/webtest-autocomplete.ts#L116
 	// The important part is the userEvent: input.type.
 	const defaultTransaction = editor.state.update({
-		changes: [{ from: selection.main.head, insert: text }],
-		selection: { anchor: selection.main.head + text.length },
+		changes: [{
+			from: selection.main.from + backspaceOffset,
+			to: selection.main.to,
+			insert: text,
+		}],
+		selection: { anchor: selection.main.head + text.length + backspaceOffset },
 		userEvent: 'input.type',
 	});
 

packages/editor/CodeMirror/utils/clampSelectionToDocument.test.ts

@@ -0,0 +1,39 @@
+import { EditorSelection, Text } from '@codemirror/state';
+import clampSelectionToDocument from './clampSelectionToDocument';
+
+describe('clampSelectionToDocument', () => {
+	it('should clamp a selection to the document', () => {
+		const doc = Text.of(['Testing...']);
+		const end = 'Testing...'.length;
+
+		expect(clampSelectionToDocument(
+			EditorSelection.single(end + 1), doc,
+		)).toMatchObject({
+			main: { from: end, to: end },
+		});
+
+		// Should map multiple at once
+		expect(clampSelectionToDocument(
+			EditorSelection.create([
+				EditorSelection.range(-1, 0),
+				EditorSelection.cursor(1),
+				EditorSelection.cursor(100),
+			], 2), doc,
+		)).toMatchObject({
+			ranges: [
+				{ from: 0, to: 0 },
+				{ from: 1, to: 1 },
+				{ from: end, to: end },
+			],
+			mainIndex: 2,
+		});
+	});
+
+	it('should not change a selection within the document', () => {
+		const doc = Text.of(['Test']);
+
+		// Should return the exact same selection
+		const selection = EditorSelection.single('Te'.length);
+		expect(clampSelectionToDocument(selection, doc)).toBe(selection);
+	});
+});

packages/editor/CodeMirror/utils/clampSelectionToDocument.ts

@@ -0,0 +1,27 @@
+import { EditorSelection, Text } from '@codemirror/state';
+
+const clampSelectionToDocument = (selection: EditorSelection, doc: Text) => {
+	const newRanges = [];
+
+	const clampDocumentPosition = (pos: number) => Math.max(0, Math.min(pos, doc.length));
+
+	let changed = false;
+	for (const range of selection.ranges) {
+		const newAnchor = clampDocumentPosition(range.anchor);
+		const newHead = clampDocumentPosition(range.head);
+		if (newAnchor !== range.anchor || newHead !== range.head) {
+			newRanges.push(EditorSelection.range(newAnchor, newHead));
+			changed = true;
+		} else {
+			newRanges.push(range);
+		}
+	}
+
+	if (changed) {
+		return EditorSelection.create(newRanges, selection.mainIndex);
+	} else {
+		return selection;
+	}
+};
+
+export default clampSelectionToDocument;