Desktop: Accessibility: Prevent overwrite mode toggle shortcut from conflicting with screen reader shortcuts (#11447)

.eslintignore

@@ -918,6 +918,7 @@ packages/editor/CodeMirror/utils/growSelectionToNode.js
 packages/editor/CodeMirror/utils/handlePasteEvent.js
 packages/editor/CodeMirror/utils/isCursorAtBeginning.js
 packages/editor/CodeMirror/utils/isInSyntaxNode.js
+packages/editor/CodeMirror/utils/keyUpHandlerExtension.js
 packages/editor/CodeMirror/utils/overwriteModeExtension.test.js
 packages/editor/CodeMirror/utils/overwriteModeExtension.js
 packages/editor/CodeMirror/utils/searchExtension.js

.gitignore

@@ -894,6 +894,7 @@ packages/editor/CodeMirror/utils/growSelectionToNode.js
 packages/editor/CodeMirror/utils/handlePasteEvent.js
 packages/editor/CodeMirror/utils/isCursorAtBeginning.js
 packages/editor/CodeMirror/utils/isInSyntaxNode.js
+packages/editor/CodeMirror/utils/keyUpHandlerExtension.js
 packages/editor/CodeMirror/utils/overwriteModeExtension.test.js
 packages/editor/CodeMirror/utils/overwriteModeExtension.js
 packages/editor/CodeMirror/utils/searchExtension.js

packages/editor/CodeMirror/utils/keyUpHandlerExtension.ts

@@ -0,0 +1,66 @@
+import { EditorView, ViewPlugin } from '@codemirror/view';
+
+interface OnKeyUpEvent {
+	domEvent: KeyboardEvent;
+	view: EditorView;
+	// true if other keys were pressed (and possibly released) during the time
+	// that this key was down (and before it was released).
+	otherKeysWerePressed: boolean;
+}
+
+// Should return true if the event was handled
+type OnKeyUpHandler = (event: OnKeyUpEvent)=> boolean;
+// Should return true if the given event matches the key that should
+// be handled by the extension:
+type IsTargetKeyCallback = (event: KeyboardEvent)=> boolean;
+
+// CodeMirror's built-in keyboard event handlers trigger on key down, rather than on
+// key up. In some cases, it's useful to trigger keyboard shortcuts on key up instead.
+// For example, if a shortcut should only activate if it isn't pressed at the same time
+// as other keys.
+const keyUpHandlerExtension = (isTargetKey: IsTargetKeyCallback, onKeyUp: OnKeyUpHandler) => {
+	return ViewPlugin.fromClass(class {
+		private otherKeysWerePressed = false;
+		private targetKeyDown = false;
+
+		public constructor(private view: EditorView) {
+			view.contentDOM.addEventListener('keydown', this.onKeyDown);
+			view.contentDOM.addEventListener('keyup', this.onKeyUp);
+		}
+
+		public destroy() {
+			this.view?.contentDOM?.removeEventListener('keyup', this.onKeyUp);
+		}
+
+		private onKeyDown = (event: KeyboardEvent) => {
+			if (isTargetKey(event)) {
+				this.targetKeyDown = true;
+			} else if (this.targetKeyDown) {
+				this.otherKeysWerePressed = true;
+			}
+		};
+
+		private onKeyUp = (event: KeyboardEvent) => {
+			if (isTargetKey(event)) {
+				if (this.targetKeyDown && !event.defaultPrevented) {
+					const handled = onKeyUp({
+						domEvent: event,
+						view: this.view,
+						otherKeysWerePressed: this.otherKeysWerePressed,
+					});
+
+					if (handled) {
+						event.preventDefault();
+					}
+				}
+
+				this.targetKeyDown = false;
+				this.otherKeysWerePressed = false;
+			} else if (this.targetKeyDown) {
+				this.otherKeysWerePressed = true;
+			}
+		};
+	});
+};
+
+export default keyUpHandlerExtension;

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

@@ -1,6 +1,6 @@
 import { EditorSelection } from '@codemirror/state';
 import createTestEditor from '../testUtil/createTestEditor';
-import overwriteModeExtension, { toggleOverwrite } from './overwriteModeExtension';
+import overwriteModeExtension, { overwriteModeEnabled, toggleOverwrite } from './overwriteModeExtension';
 import typeText from '../testUtil/typeText';
 import pressReleaseKey from '../testUtil/pressReleaseKey';
 
@@ -36,6 +36,8 @@ describe('overwriteModeExtension', () => {
 		const editor = await createEditor('Test!');
 
 		pressReleaseKey(editor, { key: 'Insert', code: 'Insert' });
+		expect(overwriteModeEnabled(editor)).toBe(true);
+
 		typeText(editor, 'Exam');
 		expect(editor.state.doc.toString()).toBe('Exam!');
 
@@ -44,6 +46,35 @@ describe('overwriteModeExtension', () => {
 		expect(editor.state.doc.toString()).toBe('Example!');
 	});
 
+	test('should be disabled by pressing <escape>', async () => {
+		const editor = await createEditor('Test!', true);
+
+		expect(overwriteModeEnabled(editor)).toBe(true);
+		pressReleaseKey(editor, { key: 'Escape', code: 'Escape' });
+		expect(overwriteModeEnabled(editor)).toBe(false);
+		// Escape should not re-enable the extension
+		pressReleaseKey(editor, { key: 'Escape', code: 'Escape' });
+		expect(overwriteModeEnabled(editor)).toBe(false);
+	});
+
+	test('<insert> should not toggle overwrite mode if other keys are pressed', async () => {
+		// On Linux, the Orca screen reader's default "do screen reader action" key is
+		// <insert>. To avoid toggling insert mode when users perform screen reader actions,
+		// pressing <insert> should only toggle insert mode in certain cases.
+		const editor = await createEditor('Test');
+		const insertKey = { code: 'Insert', key: 'Insert ' };
+
+		editor.contentDOM.dispatchEvent(new KeyboardEvent('keydown', insertKey));
+		expect(overwriteModeEnabled(editor)).toBe(false);
+
+		// Pressing & releasing the space key should prevent insert mode from being enabled
+		pressReleaseKey(editor, { code: 'Space', key: 'Space' });
+
+		editor.contentDOM.dispatchEvent(new KeyboardEvent('keyup', insertKey));
+
+		expect(overwriteModeEnabled(editor)).toBe(false);
+	});
+
 	test('should insert text if the cursor is at the end of a line', async () => {
 		const editor = await createEditor('\nTest', true);
 		typeText(editor, 'Test! This is a test! ');

packages/editor/CodeMirror/utils/overwriteModeExtension.ts

@@ -1,5 +1,6 @@
-import { keymap, EditorView } from '@codemirror/view';
+import { EditorView, keymap } from '@codemirror/view';
 import { StateField, Facet, StateEffect } from '@codemirror/state';
+import keyUpHandlerExtension from './keyUpHandlerExtension';
 
 const overwriteModeFacet = Facet.define({
 	combine: values => values[0] ?? false,
@@ -62,17 +63,50 @@ const overwriteModeState = StateField.define({
 	],
 });
 
+export const overwriteModeEnabled = (view: EditorView) => {
+	return view.state.field(overwriteModeState);
+};
+
+const setOverwriteModeEnabled = (enabled: boolean, view: EditorView) => {
+	view.dispatch({
+		effects: [
+			toggleOverwrite.of(enabled),
+			EditorView.announce.of(
+				// TODO: Localize:
+				enabled ? 'Overwrite mode enabled' : 'Overwrite mode disabled',
+			),
+		],
+	});
+};
+
 const overwriteModeExtension = [
 	overwriteModeState,
-	keymap.of([{
-		key: 'Insert',
-		run: (view) => {
-			view.dispatch({
-				effects: toggleOverwrite.of(!view.state.field(overwriteModeState)),
-			});
-			return false;
+	keymap.of([
+		{
+			// The <escape> keyboard shortcut may be more easily discoverable for users
+			// who enter overwrite mode unintentionally.
+			key: 'Escape',
+			run: (view) => {
+				if (overwriteModeEnabled(view)) {
+					setOverwriteModeEnabled(false, view);
+					return true;
+				}
+				return false;
+			},
 		},
-	}]),
+	]),
+	keyUpHandlerExtension(
+		(event) => (
+			event.code === 'Insert' && !event.shiftKey && !event.altKey && !event.metaKey && !event.ctrlKey
+		),
+		({ view, otherKeysWerePressed }) => {
+			if (otherKeysWerePressed) {
+				return false;
+			}
+			setOverwriteModeEnabled(!overwriteModeEnabled(view), view);
+			return true;
+		},
+	),
 ];
 
 export default overwriteModeExtension;