From 32f0c7da0a85a380c8b83dab427c527770a07fdf Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 1 Aug 2026 22:02:19 -0700 Subject: [PATCH] fix(ime): stop terminal shortcuts firing on keystrokes the IME owns The window-capture keydown handler resolved and dispatched terminal chords without checking for a live composition. It runs ahead of the pane handler and stops propagation on a match, so the chord was the only thing the keystroke did. Ctrl+Backspace mid-preedit reached the PTY as \x17 and erased a word of the real command line behind the candidate window, leaving the composition untouched. Alt+Backspace, Alt and Ctrl arrows, and the macOS Cmd line-edit chords have the same shape. Separately, the non-Latin fallback re-derives bindings from event.code, so a keystroke reported as Process still matched Ctrl+W and closed the pane out from under a composition. The only composition check was inside the sendInput branch and covered Enter alone, which is the one key that was already safe. Enter and the input-source switch stay exempt: the send path defers Enter until the commit lands, and the switch only hands the key back to the OS. --- .../keyboard-handlers-ime.test.tsx | 78 +++++++++++++++++++ .../terminal-pane/keyboard-handlers.ts | 5 ++ ...erminal-shortcut-composition-guard.test.ts | 47 +++++++++++ .../terminal-shortcut-composition-guard.ts | 35 +++++++++ 4 files changed, 165 insertions(+) create mode 100644 src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.ts diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx index 568f34a92d0..7717dffeb87 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx @@ -31,6 +31,7 @@ function keyboardEvent( function createHarness(): { deps: KeyboardHandlersDeps editable: HTMLInputElement + sendInput: ReturnType startComposition: () => void terminalInput: HTMLTextAreaElement dispose: () => void @@ -97,6 +98,7 @@ function createHarness(): { return { deps, editable, + sendInput, terminalInput, startComposition: () => { terminalElement.dispatchEvent( @@ -275,3 +277,79 @@ describe('Windows IME keyboard ownership', () => { harness.dispose() }) }) + +describe('terminal shortcuts yield to a composition', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue('X11; Linux x86_64') + }) + + afterEach(() => { + cleanup() + vi.clearAllTimers() + vi.useRealTimers() + vi.restoreAllMocks() + }) + + // The chord resolves to a real control sequence, so the only thing standing between a + // preedit keystroke and the PTY is a composition check above the dispatch. + it.each([ + { name: 'Ctrl+Backspace', init: { key: 'Backspace', code: 'Backspace', ctrlKey: true } }, + { name: 'Alt+Backspace', init: { key: 'Backspace', code: 'Backspace', altKey: true } }, + { name: 'Alt+ArrowLeft', init: { key: 'ArrowLeft', code: 'ArrowLeft', altKey: true } } + ])('does not send $name to the PTY while composing', ({ init }) => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.startComposition() + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { ...init, keyCode: 229, timeStamp: 10, isComposing: true }) + ) + + expect(harness.sendInput).not.toHaveBeenCalled() + hook.unmount() + harness.dispose() + }) + + it('still sends Ctrl+Backspace when nothing is composing', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'Backspace', + code: 'Backspace', + keyCode: 8, + timeStamp: 10, + ctrlKey: true + }) + ) + + expect(harness.sendInput).toHaveBeenCalled() + hook.unmount() + harness.dispose() + }) + + // 'Process' means the IME already consumed this keystroke, but the non-Latin fallback + // re-derives the binding from event.code, so the chord matches by physical key anyway. + it('does not close the pane on a Ctrl+W the IME consumed', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.startComposition() + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'Process', + code: 'KeyW', + keyCode: 229, + timeStamp: 10, + isComposing: true, + ctrlKey: true + }) + ) + + expect(harness.deps.onRequestClosePane).not.toHaveBeenCalled() + hook.unmount() + harness.dispose() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 98b3ac50e3c..9c558da4f64 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -9,6 +9,7 @@ import { safeFind } from '../terminal-search-safe-find' import { resolveTerminalShortcutAction } from './terminal-shortcut-policy' import type { MacOptionAsAlt } from './terminal-shortcut-policy' import { createTerminalNativeOnlyShortcutTracker } from './terminal-native-only-shortcut' +import { terminalShortcutIsOwnedByIme } from './terminal-shortcut-composition-guard' import { createTerminalImeDeferredNewlineSender, createTerminalImeModifiedEnterChordOwner, @@ -456,6 +457,10 @@ export function useTerminalKeyboardShortcuts({ return } + if (terminalShortcutIsOwnedByIme(e, resolveShortcutEvent)) { + return + } + if (matchFileSearchShortcut(e, shortcutPlatform, keybindings, terminalShortcutPolicy)) { const pane = manager.getActivePane() ?? manager.getPanes()[0] const selectedText = normalizeSelectedTextForFileSearch(pane?.terminal.getSelection()) diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.test.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.test.ts new file mode 100644 index 00000000000..1323b58ecb4 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.test.ts @@ -0,0 +1,47 @@ +// @vitest-environment happy-dom +import { describe, expect, it } from 'vitest' +import { terminalShortcutIsOwnedByIme } from './terminal-shortcut-composition-guard' + +function keydown(init: KeyboardEventInit & { keyCode?: number }): KeyboardEvent { + const event = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, ...init }) + if (init.keyCode !== undefined) { + Object.defineProperty(event, 'keyCode', { value: init.keyCode }) + } + return event +} + +const sendInput = (): { type: string } => ({ type: 'sendInput' }) +const switchInputSource = (): { type: string } => ({ type: 'switchInputSource' }) +const noAction = (): null => null + +describe('terminalShortcutIsOwnedByIme', () => { + it.each([ + ['isComposing', { key: 'Backspace', code: 'Backspace', ctrlKey: true, isComposing: true }], + ['keyCode 229', { key: 'Backspace', code: 'Backspace', ctrlKey: true, keyCode: 229 }], + ['key Process', { key: 'Process', code: 'KeyW', ctrlKey: true }] + ])('claims a chord marked by %s', (_marker, init) => { + expect(terminalShortcutIsOwnedByIme(keydown(init), sendInput)).toBe(true) + }) + + it('leaves an unmarked chord alone', () => { + const event = keydown({ key: 'Backspace', code: 'Backspace', ctrlKey: true, keyCode: 8 }) + expect(terminalShortcutIsOwnedByIme(event, sendInput)).toBe(false) + }) + + // Both exemptions matter in the same direction: claiming them would break a path that + // already handles composition, rather than merely declining to fix one. + it('releases Enter so the send path can defer it to the commit', () => { + const event = keydown({ key: 'Process', code: 'Enter', keyCode: 229, isComposing: true }) + expect(terminalShortcutIsOwnedByIme(event, sendInput)).toBe(false) + }) + + it('releases the input-source switch so the OS still receives it', () => { + const event = keydown({ key: 'Process', code: 'Space', ctrlKey: true, keyCode: 229 }) + expect(terminalShortcutIsOwnedByIme(event, switchInputSource)).toBe(false) + }) + + it('claims a marked chord that resolves to nothing, so no later matcher sees it', () => { + const event = keydown({ key: 'Process', code: 'KeyF', ctrlKey: true, keyCode: 229 }) + expect(terminalShortcutIsOwnedByIme(event, noAction)).toBe(true) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.ts new file mode 100644 index 00000000000..3cb7cdb957d --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-composition-guard.ts @@ -0,0 +1,35 @@ +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' + +type ResolvedShortcutAction = { readonly type: string } | null + +/** + * True when an IME owns this keydown, so no terminal shortcut may act on it. + * + * The window-capture handler runs before the pane's own key handler and stops + * propagation on a match, so a chord resolved here is the only thing the keystroke + * will ever do. Ctrl+Backspace mid-preedit reaches the PTY as `\x17` and erases a word + * of the real command line behind the candidate window; the IME's buffer is untouched. + * The non-Latin fallback makes it worse by re-deriving bindings from `event.code`, so + * even a keystroke reported as `Process` still matches Ctrl+W and closes the pane. + * + * Two exemptions, both paths that already handle a live composition: + * + * - **Enter** — the send path defers it until the commit lands, so it must get through. + * - **Input-source switch** — it only hands the key back to the OS. Dropping it here + * would let xterm see the chord instead and write its control byte to the PTY, which + * is the very failure this guard exists to prevent. + * + * Resolution is pure, so asking for the action before deciding costs nothing. + */ +export function terminalShortcutIsOwnedByIme( + event: KeyboardEvent, + resolveAction: (event: KeyboardEvent) => ResolvedShortcutAction +): boolean { + if (!isImeCompositionKeyDown(event)) { + return false + } + if (event.code === 'Enter') { + return false + } + return resolveAction(event)?.type !== 'switchInputSource' +}