From 359fd2e0959e43fbe0fdb3eb2f22e8e425dadd05 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:41:02 +0900 Subject: [PATCH 01/15] fix(terminal): sequence shortcut bytes behind a pending IME composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing "가나" and pressing Cmd+Backspace leaves "나" on the line. "가" has reached the pty while "나" is still in the preedit, so the line-kill byte goes out synchronously, erases what the shell has, and only then does xterm flush "나" from its timer. xterm guards this in CompositionHelper.keydown by flushing the preedit before a non-composition key runs, but Orca's window-level capture handler calls stopImmediatePropagation, so xterm never sees the keydown. The compositionend path sends the committed glyph from a timer, so it is asynchronous regardless. keyboard-handlers already defers for this reason, but only for Enter. Widen it to every sendInput action while a composition is pending. Enter keeps deferredNewlineSender, which also owns the Windows redispatch ledger; other keys call sendTerminalInputAfterComposition directly so they do not leave absorb credits nothing will consume. Also fixes Cmd+Delete, Cmd+Arrow, Ctrl+Backspace, and the Alt meta sequences, which shared the same unsequenced path on all platforms. Co-Authored-By: Claude Opus 5 (1M context) --- ...board-handlers-ime-deferred-input.test.tsx | 229 ++++++++++++++++++ .../terminal-pane/keyboard-handlers.ts | 23 +- 2 files changed, 245 insertions(+), 7 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers-ime-deferred-input.test.tsx diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-deferred-input.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-deferred-input.test.tsx new file mode 100644 index 000000000000..b9bcb20b573f --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-deferred-input.test.tsx @@ -0,0 +1,229 @@ +// @vitest-environment happy-dom +import { cleanup, renderHook } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import type { PtyTransport } from './pty-transport' +import { useTerminalKeyboardShortcuts } from './keyboard-handlers' +import { + installTerminalImeCompositionRoute, + XTERM_COMPOSITION_SESSION_END_EVENT, + XTERM_COMPOSITION_SESSION_START_EVENT +} from './terminal-ime-composition-route' + +type KeyboardHandlersDeps = Parameters[0] + +function keyboardEvent( + type: 'keydown' | 'keyup', + overrides: KeyboardEventInit & { keyCode: number; timeStamp: number; isComposing?: boolean } +): KeyboardEvent { + const event = new KeyboardEvent(type, { bubbles: true, cancelable: true, ...overrides }) + Object.defineProperties(event, { + isComposing: { value: overrides.isComposing ?? false }, + keyCode: { value: overrides.keyCode }, + timeStamp: { value: overrides.timeStamp } + }) + return event +} + +function createHarness(): { + deps: KeyboardHandlersDeps + order: string[] + startComposition: () => void + commitComposition: (data: string) => void + terminalInput: HTMLTextAreaElement + dispose: () => void +} { + const scope = document.createElement('div') + const terminalElement = document.createElement('div') + const terminalInput = document.createElement('textarea') + terminalInput.className = 'xterm-helper-textarea' + terminalElement.append(terminalInput) + scope.append(terminalElement) + document.body.append(scope) + + const order: string[] = [] + const transport = { + getPtyId: () => 'pty-1', + sendInput: vi.fn((data: string) => { + order.push(`pty:${JSON.stringify(data)}`) + return true + }) + } as unknown as PtyTransport + const pane = { + id: 1, + leafId: '00000000-0000-4000-8000-000000000001', + terminal: { + element: terminalElement, + focus: vi.fn(), + getSelection: vi.fn(() => '') + } + } + const manager = { + getActivePane: () => pane, + getPanes: () => [pane] + } as unknown as PaneManager + const route = installTerminalImeCompositionRoute({ + terminalElement, + terminal: { + input: vi.fn((data: string) => { + order.push(`commit:${data}`) + }) + }, + capturedTransport: transport, + getCurrentTransport: () => transport + }) + const deps: KeyboardHandlersDeps = { + tabId: 'tab-1', + worktreeId: 'worktree-1', + isActive: true, + keyboardScopeRef: { current: scope }, + managerRef: { current: manager }, + paneTransportsRef: { current: new Map([[pane.id, transport]]) }, + panePtyBindingsRef: { current: new Map() }, + paneCwdRef: { current: new Map() }, + fallbackCwd: '', + expandedPaneIdRef: { current: null }, + setExpandedPane: vi.fn(), + restoreExpandedLayout: vi.fn(), + refreshPaneSizes: vi.fn(), + persistLayoutSnapshot: vi.fn(), + toggleExpandPane: vi.fn(), + setSearchOpen: vi.fn(), + onSearchSelectedText: vi.fn(), + onRequestClosePane: vi.fn(), + onClearPaneScrollback: vi.fn(), + onSetTitle: vi.fn(), + onClearPaneTitle: vi.fn(), + searchOpenRef: { current: false }, + searchStateRef: { current: { query: '', caseSensitive: false, regex: false } }, + macOptionAsAltRef: { current: 'false' } + } + return { + deps, + order, + terminalInput, + startComposition: () => { + terminalElement.dispatchEvent( + new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1 } }) + ) + }, + commitComposition: (data) => { + terminalElement.dispatchEvent( + new CustomEvent(XTERM_COMPOSITION_SESSION_END_EVENT, { + cancelable: true, + detail: { id: 1, data } + }) + ) + }, + dispose: () => { + route.dispose() + scope.remove() + } + } +} + +describe('terminal shortcut bytes during an IME composition', () => { + afterEach(() => { + cleanup() + vi.clearAllTimers() + vi.useRealTimers() + vi.restoreAllMocks() + }) + + function usePlatform(userAgent: string): void { + vi.useFakeTimers() + vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue(userAgent) + } + + // "가" already reached the pty; "나" is still in the preedit. The shell reads + // these bytes against the line it has, so they must not overtake the commit. + it.each([ + { + name: 'Cmd+Backspace line kill', + key: 'Backspace', + keyCode: 8, + mods: { metaKey: true }, + sent: '\x15' + }, + { + name: 'Cmd+Delete kill to end', + key: 'Delete', + keyCode: 46, + mods: { metaKey: true }, + sent: '\x0b' + }, + { + name: 'Cmd+ArrowLeft line start', + key: 'ArrowLeft', + keyCode: 37, + mods: { metaKey: true }, + sent: '\x01' + } + ])('defers a macOS $name until the trailing syllable commits', ({ key, keyCode, mods, sent }) => { + usePlatform('Macintosh') + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.startComposition() + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key, + code: key, + keyCode, + timeStamp: 10, + isComposing: true, + ...mods + }) + ) + harness.commitComposition('나') + vi.runAllTimers() + + expect(harness.order).toEqual(['commit:나', `pty:${JSON.stringify(sent)}`]) + hook.unmount() + harness.dispose() + }) + + it('defers Ctrl+Backspace word kill on Linux too', () => { + usePlatform('X11; Linux x86_64') + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.startComposition() + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'Backspace', + code: 'Backspace', + keyCode: 8, + timeStamp: 10, + isComposing: true, + ctrlKey: true + }) + ) + harness.commitComposition('나') + vi.runAllTimers() + + expect(harness.order).toEqual(['commit:나', 'pty:"\\u0017"']) + hook.unmount() + harness.dispose() + }) + + it('still sends immediately when no composition is pending', () => { + usePlatform('Macintosh') + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'Backspace', + code: 'Backspace', + keyCode: 8, + timeStamp: 10, + metaKey: true + }) + ) + + expect(harness.order).toEqual(['pty:"\\u0015"']) + 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 6519c5f86551..7553e440963e 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -15,7 +15,8 @@ import { getTerminalImeModifiedEnterKind, isTerminalImeConsumedKey, isTerminalImeEnterKeyUp, - isTerminalImeProcessEnter + isTerminalImeProcessEnter, + sendTerminalInputAfterComposition } from './terminal-ime-deferred-newline' import { hasPendingTerminalImeComposition } from './terminal-ime-composition-route' import { @@ -589,14 +590,22 @@ export function useTerminalKeyboardShortcuts({ return } const sendResolvedInput = createCapturedInputSender(pane, action.data) - if ((e.isComposing || hasPendingImeComposition) && (e.key === 'Enter' || imeProcessEnter)) { - if (isWindows) { - const chord = getModifiedEnterChord(e) - if (chord && !modifiedEnterChordOwner.claim(chord)) { - return + if (e.isComposing || hasPendingImeComposition) { + if (e.key === 'Enter' || imeProcessEnter) { + if (isWindows) { + const chord = getModifiedEnterChord(e) + if (chord && !modifiedEnterChordOwner.claim(chord)) { + return + } } + deferredNewlineSender.defer(e, pane.terminal.element, sendResolvedInput) + return } - deferredNewlineSender.defer(e, pane.terminal.element, sendResolvedInput) + // Why: xterm sends the committed glyph on a timer while these bytes go out + // synchronously, and the shell reads them against the line it already has — + // an unsequenced kill or cursor move lands before the trailing syllable. + // The Enter path keeps its own sender for the Windows redispatch ledger. + sendTerminalInputAfterComposition(pane.terminal.element, sendResolvedInput) return } sendResolvedInput() From 8aec6254af9c1254b4e9e2b5a514ab8c5644d826 Mon Sep 17 00:00:00 2001 From: hyeonho Date: Thu, 6 Aug 2026 15:11:29 +0900 Subject: [PATCH 02/15] test(terminal): pin cursor-chord relocation regressions during IME composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With committed text on the line and a syllable still composing, a cursor-movement chord relocated the composing character to wherever the cursor landed: Option+Left word-jump turned "가나 다라 마바[사]" into "가나 [사]다라 마바", and Cmd+Left moved the syllable to the line start. With a Japanese preedit the relocated text also overwrote the glyph at the destination cell. Reported with reproduction steps in #12871. The ordering fix already covers these chords; this pins the previously untested resolver branches so the symptom cannot quietly return: - Option+ArrowLeft / ArrowRight word jumps (\eb / \ef) - Cmd+ArrowRight line-end jump (\x05) - a multi-character Japanese preedit committing before the chord byte - Ctrl+ArrowLeft word jump on Windows (\eb) Verified non-vacuous: against the pre-fix keyboard-handlers.ts all five new cases fail with the chord byte recorded ahead of the commit. --- ...board-handlers-ime-deferred-input.test.tsx | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-deferred-input.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-deferred-input.test.tsx index b9bcb20b573f..d7b646dce3c0 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-deferred-input.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-deferred-input.test.tsx @@ -226,4 +226,108 @@ describe('terminal shortcut bytes during an IME composition', () => { hook.unmount() harness.dispose() }) + + // Issue #12871: with "가나 다라 마바" committed and "사" still composing, a + // cursor-movement chord relocated the composing syllable to wherever the + // cursor landed — Option+← produced "가나 '사'다라 마바" and Cmd+← produced + // "'사'가나 다라 마바". The shell applies the movement bytes to the line it + // already has, so the commit must land before the chord travels. + describe('cursor-movement chords do not relocate the composing character', () => { + it.each([ + { + name: 'Option+ArrowLeft word jump', + key: 'ArrowLeft', + keyCode: 37, + mods: { altKey: true }, + sent: '\x1bb' + }, + { + name: 'Option+ArrowRight word jump', + key: 'ArrowRight', + keyCode: 39, + mods: { altKey: true }, + sent: '\x1bf' + }, + { + name: 'Cmd+ArrowRight line-end jump', + key: 'ArrowRight', + keyCode: 39, + mods: { metaKey: true }, + sent: '\x05' + } + ])('sequences a macOS $name behind the composing syllable', ({ key, keyCode, mods, sent }) => { + usePlatform('Macintosh') + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.startComposition() + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key, + code: key, + keyCode, + timeStamp: 10, + isComposing: true, + ...mods + }) + ) + harness.commitComposition('사') + vi.runAllTimers() + + expect(harness.order).toEqual([`commit:사`, `pty:${JSON.stringify(sent)}`]) + hook.unmount() + harness.dispose() + }) + + // A Japanese preedit spans several characters before it commits; the + // relocated preedit overwrote the glyph at the destination cell. The + // whole preedit must commit in place before the cursor moves. + it('sequences a Cmd+ArrowLeft line-start jump behind a multi-character Japanese preedit', () => { + usePlatform('Macintosh') + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.startComposition() + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 37, + timeStamp: 10, + isComposing: true, + metaKey: true + }) + ) + harness.commitComposition('かな') + vi.runAllTimers() + + expect(harness.order).toEqual(['commit:かな', 'pty:"\\u0001"']) + hook.unmount() + harness.dispose() + }) + + it('sequences a Ctrl+ArrowLeft word jump behind the composing syllable on Windows', () => { + usePlatform('Windows NT 10.0; Win64; x64') + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + harness.startComposition() + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 37, + timeStamp: 10, + isComposing: true, + ctrlKey: true + }) + ) + harness.commitComposition('사') + vi.runAllTimers() + + expect(harness.order).toEqual(['commit:사', 'pty:"\\u001bb"']) + hook.unmount() + harness.dispose() + }) + }) }) From 07f72331f9c72ff3ae6c508b0f8a9e41e4a1b59a Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 8 Aug 2026 19:41:34 +0900 Subject: [PATCH 03/15] fix(terminal): send IME-swallowed chords once, from recorded traces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A modifier chord pressed during a composition arrives identically on both input sources tested — code='ArrowLeft', keyCode=229, isComposing=true — but they answer it in opposite ways, recorded on stock macOS: Korean 2-Set commits the syllable, emits compositionend, and the platform replays the chord unmarked after keyup. That replay already reached the shell, so main is correct here today. Japanese conversion swallows it: no commit, no replay, still composing at keyup. Cmd/Option+Arrow and Cmd+Backspace produced no bytes at all. Exempting the marked keydown alone fixes Japanese and breaks Korean, where Option+Left's \x1bb would go out twice and jump two words. So the exemption is paired with a redispatch ledger that retires the replayed copy, mirroring useImeEnterGestureOwnership — including its animation-frame expiry, since macOS delivers keyup before the replay. Chord matching for these four physical keys now reads event.code, and the keybinding lookup reads the same resolved key, so a remapped chord cannot lose to the built-in byte while an input source rewrites event.key. Ordering needs no timer: xterm's patched input() queues the bytes behind the preedit and flushes them with the commit. --- ...ndlers.issue-12871-composing-chord.test.ts | 94 +++ ....issue-12871-recorded-chord-traces.test.ts | 766 ++++++++++++++++++ .../terminal-pane/keyboard-handlers.ts | 38 +- .../terminal-ime-chord-redispatch.ts | 82 ++ .../terminal-pane/terminal-shortcut-policy.ts | 75 +- 5 files changed, 1029 insertions(+), 26 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts new file mode 100644 index 000000000000..4c361d5b501b --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts @@ -0,0 +1,94 @@ +// Issue #12871. Unit half: which events the shortcut layer still resolves while a composition +// is live. The end-to-end half lives in keyboard-handlers.issue-12871-recorded-chord-traces.test.ts, +// which replays captured macOS Korean and Japanese traces through the real handler and xterm. +// +// The exemption exists because a Japanese conversion swallows a modifier chord outright — no +// commit, no platform replay, nothing reaches the shell. Its counterweight is the redispatch +// ledger, since Korean's input source *does* replay the chord and would otherwise fire twice. +// +// Scope: shapes here are constructed from the DOM contract, not captured. The captured file is +// where hardware fidelity lives; this file pins the decision boundary, including the negatives. +import { describe, expect, it } from 'vitest' +import { resolveTerminalKeyboardShortcutAction } from './keyboard-handlers' +import type { TerminalShortcutEvent } from './terminal-shortcut-policy' + +// `isComposing` alone decides ownership; `keyCode` is carried so each fixture reads as the +// shape hardware actually produces, matching the recorded traces in the sibling IME tests. +type ComposingKeyEvent = TerminalShortcutEvent & { isComposing: boolean; keyCode: number } + +function keyEvent(overrides: Partial): ComposingKeyEvent { + return { + key: '', + code: '', + metaKey: false, + ctrlKey: false, + altKey: false, + shiftKey: false, + repeat: false, + isComposing: false, + keyCode: 0, + ...overrides + } +} + +function resolveOnMac(event: ComposingKeyEvent): ReturnType { + return resolveTerminalKeyboardShortcutAction( + event, + true, + 'false', + 0, + false, + undefined, + undefined, + undefined, + undefined, + undefined, + () => false + ) +} + +describe('terminal chords stay live during an IME composition', () => { + it.each([ + ['Cmd+ArrowLeft', keyEvent({ key: 'ArrowLeft', code: 'ArrowLeft', metaKey: true }), '\x01'], + ['Cmd+ArrowRight', keyEvent({ key: 'ArrowRight', code: 'ArrowRight', metaKey: true }), '\x05'], + ['Cmd+Backspace', keyEvent({ key: 'Backspace', code: 'Backspace', metaKey: true }), '\x15'], + ['Cmd+Delete', keyEvent({ key: 'Delete', code: 'Delete', metaKey: true }), '\x0b'], + ['Option+ArrowLeft', keyEvent({ key: 'ArrowLeft', code: 'ArrowLeft', altKey: true }), '\x1bb'], + ['Option+ArrowRight', keyEvent({ key: 'ArrowRight', code: 'ArrowRight', altKey: true }), '\x1bf'] + ])('resolves %s while a composition is live', (_label, event, expected) => { + // The idle case is asserted too because it is not covered elsewhere with a populated + // `code`: terminal-shortcut-policy.test.ts builds its fixtures with `code: ''`, which + // takes physicalChordKey's fallback branch rather than the production one. + expect(resolveOnMac(event)).toEqual({ type: 'sendInput', data: expected }) + + expect(resolveOnMac({ ...event, isComposing: true })).toEqual({ + type: 'sendInput', + data: expected + }) + }) + + // A CJK input source rewrites `key` while `code` keeps the physical key, which is what + // #12171 and #13033 turned on. Matching `key` here would drop the chord again. + it('matches the physical code when the input source has rewritten key', () => { + const event = keyEvent({ + key: 'Process', + code: 'ArrowLeft', + metaKey: true, + isComposing: true, + keyCode: 229 + }) + + expect(resolveOnMac(event)).toEqual({ type: 'sendInput', data: '\x01' }) + }) + + // The exemption is for chords over a physical key, not for the IME's own gesture keys. + // A composing Ctrl+Space mode switch reaches the policy as a lone Control keydown. + it.each([ + ['lone Control', keyEvent({ key: 'Control', code: 'ControlLeft', keyCode: 17, ctrlKey: true })], + ['lone Meta', keyEvent({ key: 'Meta', code: 'MetaLeft', keyCode: 91, metaKey: true })], + ['bare ArrowLeft', keyEvent({ key: 'ArrowLeft', code: 'ArrowLeft' })], + ['Cmd+A', keyEvent({ key: 'a', code: 'KeyA', metaKey: true })] + ])('still yields %s to the IME while composing', (_label, event) => { + expect(resolveOnMac({ ...event, isComposing: true })).toBeNull() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts new file mode 100644 index 000000000000..5e11aee8f9a2 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -0,0 +1,766 @@ +// @vitest-environment happy-dom +// Recorded on stock macOS (Chrome 151) by logging keydown/keyup/composition*/input on a +// textarea, then replayed here through the production handler and a real patched Terminal. +// Every row below is captured, not authored. +// +// The two input sources answer a modifier chord in opposite ways, and the marked keydown is +// identical in both — code='ArrowLeft', keyCode=229, isComposing=true — so nothing tells them +// apart at the moment the handler has to decide: +// +// - Korean 2-Set commits the syllable, emits compositionend, and the platform replays the +// chord unmarked after keyup. Acting on both copies sends Option+Left's \x1bb twice, which +// jumps two words instead of one. +// - Japanese conversion swallows the chord outright: no compositionend, no replay, still +// composing at keyup. Before the exemption these three chords produced no bytes at all. +// +// So both halves are pinned together: the fix is only correct if Korean stays at one firing +// while Japanese goes from zero to one each. +import { Terminal } from '@xterm/xterm' +import { cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import type { PtyTransport } from './pty-transport' +import { useTerminalKeyboardShortcuts } from './keyboard-handlers' + +type RecordedRow = { + t: string + key?: string + code?: string + keyCode?: number + isComposing?: boolean + meta?: boolean + alt?: boolean + data?: string + inputType?: string + value?: string +} + +type RecordedCase = { + name: string + expectCalls: string[] + expectEmitted: string[] + rows: RecordedRow[] +} + +const CASES: RecordedCase[] = [ + { + "name": "Korean 2-Set, Cmd+ArrowLeft", + "expectCalls": [ + "\u0001" + ], + "expectEmitted": [ + "사\u0001" + ], + "rows": [ + { + "t": "keydown", + "key": "ㅅ", + "code": "KeyT", + "keyCode": 229 + }, + { + "t": "compositionstart", + "data": "" + }, + { + "t": "compositionupdate", + "data": "ㅅ" + }, + { + "t": "input", + "data": "ㅅ", + "inputType": "insertCompositionText", + "value": "ㅅ" + }, + { + "t": "keyup", + "key": "ㅅ", + "code": "KeyT", + "keyCode": 84, + "isComposing": true + }, + { + "t": "keydown", + "key": "ㅏ", + "code": "KeyK", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "사" + }, + { + "t": "input", + "data": "사", + "inputType": "insertCompositionText", + "value": "사" + }, + { + "t": "keyup", + "key": "ㅏ", + "code": "KeyK", + "keyCode": 75, + "isComposing": true + }, + { + "t": "keydown", + "key": "Meta", + "code": "MetaLeft", + "keyCode": 91, + "isComposing": true, + "meta": true + }, + { + "t": "keydown", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 229, + "isComposing": true, + "meta": true + }, + { + "t": "compositionupdate", + "data": "사" + }, + { + "t": "input", + "data": "사", + "inputType": "insertCompositionText", + "value": "사" + }, + { + "t": "compositionend", + "data": "사" + }, + { + "t": "keyup", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 37, + "meta": true + }, + { + "t": "keydown", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 37, + "meta": true + }, + { + "t": "keyup", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 37, + "meta": true + }, + { + "t": "keyup", + "key": "Meta", + "code": "MetaLeft", + "keyCode": 91 + } + ] + }, + { + "name": "Korean 2-Set, Option+ArrowLeft", + "expectCalls": [ + "\u001bb" + ], + "expectEmitted": [ + "사\u001bb" + ], + "rows": [ + { + "t": "keydown", + "key": "ㅅ", + "code": "KeyT", + "keyCode": 229 + }, + { + "t": "compositionstart", + "data": "" + }, + { + "t": "compositionupdate", + "data": "ㅅ" + }, + { + "t": "input", + "data": "ㅅ", + "inputType": "insertCompositionText", + "value": "ㅅ" + }, + { + "t": "keyup", + "key": "ㅅ", + "code": "KeyT", + "keyCode": 84, + "isComposing": true + }, + { + "t": "keydown", + "key": "ㅏ", + "code": "KeyK", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "사" + }, + { + "t": "input", + "data": "사", + "inputType": "insertCompositionText", + "value": "사" + }, + { + "t": "keyup", + "key": "ㅏ", + "code": "KeyK", + "keyCode": 75, + "isComposing": true + }, + { + "t": "keydown", + "key": "Alt", + "code": "AltLeft", + "keyCode": 18, + "isComposing": true, + "alt": true + }, + { + "t": "keydown", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 229, + "isComposing": true, + "alt": true + }, + { + "t": "compositionupdate", + "data": "사" + }, + { + "t": "input", + "data": "사", + "inputType": "insertCompositionText", + "value": "사" + }, + { + "t": "compositionend", + "data": "사" + }, + { + "t": "keyup", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 37, + "alt": true + }, + { + "t": "keydown", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 37, + "alt": true + }, + { + "t": "keyup", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 37, + "alt": true + }, + { + "t": "keyup", + "key": "Alt", + "code": "AltLeft", + "keyCode": 18 + } + ] + }, + { + "name": "Korean 2-Set, Cmd+Backspace", + "expectCalls": [ + "\u0015" + ], + "expectEmitted": [ + "사\u0015" + ], + "rows": [ + { + "t": "keydown", + "key": "ㅅ", + "code": "KeyT", + "keyCode": 229 + }, + { + "t": "compositionstart", + "data": "" + }, + { + "t": "compositionupdate", + "data": "ㅅ" + }, + { + "t": "input", + "data": "ㅅ", + "inputType": "insertCompositionText", + "value": "ㅅ" + }, + { + "t": "keydown", + "key": "ㅏ", + "code": "KeyK", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "사" + }, + { + "t": "input", + "data": "사", + "inputType": "insertCompositionText", + "value": "사" + }, + { + "t": "keyup", + "key": "ㅅ", + "code": "KeyT", + "keyCode": 84, + "isComposing": true + }, + { + "t": "keyup", + "key": "ㅏ", + "code": "KeyK", + "keyCode": 75, + "isComposing": true + }, + { + "t": "keydown", + "key": "Meta", + "code": "MetaLeft", + "keyCode": 91, + "isComposing": true, + "meta": true + }, + { + "t": "keydown", + "key": "Backspace", + "code": "Backspace", + "keyCode": 229, + "isComposing": true, + "meta": true + }, + { + "t": "compositionupdate", + "data": "사" + }, + { + "t": "input", + "data": "사", + "inputType": "insertCompositionText", + "value": "사" + }, + { + "t": "compositionend", + "data": "사" + }, + { + "t": "keyup", + "key": "Backspace", + "code": "Backspace", + "keyCode": 8, + "meta": true + }, + { + "t": "keydown", + "key": "Backspace", + "code": "Backspace", + "keyCode": 8, + "meta": true + }, + { + "t": "keyup", + "key": "Backspace", + "code": "Backspace", + "keyCode": 8, + "meta": true + }, + { + "t": "keyup", + "key": "Meta", + "code": "MetaLeft", + "keyCode": 91 + } + ] + }, + { + "name": "Japanese, three chords across one live preedit", + "expectCalls": [ + "\u0001", + "\u001bb", + "\u0015" + ], + "expectEmitted": [ + "日本語\u0001\u001bb\u0015" + ], + "rows": [ + { + "t": "keydown", + "key": "n", + "code": "KeyN", + "keyCode": 229 + }, + { + "t": "compositionstart", + "data": "" + }, + { + "t": "compositionupdate", + "data": "n" + }, + { + "t": "input", + "data": "n", + "inputType": "insertCompositionText", + "value": "n" + }, + { + "t": "keydown", + "key": "i", + "code": "KeyI", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "に" + }, + { + "t": "input", + "data": "に", + "inputType": "insertCompositionText", + "value": "に" + }, + { + "t": "keydown", + "key": "h", + "code": "KeyH", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "にh" + }, + { + "t": "input", + "data": "にh", + "inputType": "insertCompositionText", + "value": "にh" + }, + { + "t": "keydown", + "key": "o", + "code": "KeyO", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "にほ" + }, + { + "t": "input", + "data": "にほ", + "inputType": "insertCompositionText", + "value": "にほ" + }, + { + "t": "keydown", + "key": "n", + "code": "KeyN", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "にほn" + }, + { + "t": "input", + "data": "にほn", + "inputType": "insertCompositionText", + "value": "にほn" + }, + { + "t": "keydown", + "key": "g", + "code": "KeyG", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "にほんg" + }, + { + "t": "input", + "data": "にほんg", + "inputType": "insertCompositionText", + "value": "にほんg" + }, + { + "t": "keydown", + "key": "o", + "code": "KeyO", + "keyCode": 229, + "isComposing": true + }, + { + "t": "compositionupdate", + "data": "日本語" + }, + { + "t": "input", + "data": "日本語", + "inputType": "insertCompositionText", + "value": "日本語" + }, + { + "t": "keydown", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 229, + "isComposing": true, + "meta": true + }, + { + "t": "keyup", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 37, + "isComposing": true, + "meta": true + }, + { + "t": "keydown", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 229, + "isComposing": true, + "alt": true + }, + { + "t": "keyup", + "key": "ArrowLeft", + "code": "ArrowLeft", + "keyCode": 37, + "isComposing": true, + "alt": true + }, + { + "t": "keydown", + "key": "Backspace", + "code": "Backspace", + "keyCode": 229, + "isComposing": true, + "meta": true + }, + { + "t": "keyup", + "key": "Backspace", + "code": "Backspace", + "keyCode": 8, + "isComposing": true, + "meta": true + }, + { + "t": "compositionend", + "data": "日本語" + } + ] + } +] + +const MAC_USER_AGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)' + +function macrotask(): Promise { + return new Promise((resolve) => window.setTimeout(resolve, 0)) +} + +type Rig = { + textarea: HTMLTextAreaElement + inputCalls: string[] + emitted: string[] + unmount: () => void +} + +function openRig(): Rig { + const scope = document.createElement('div') + document.body.append(scope) + const container = document.createElement('div') + scope.append(container) + const terminal = new Terminal() + terminal.open(container) + const textarea = terminal.textarea + if (!textarea) { + throw new Error('xterm helper textarea was not created') + } + + const emitted: string[] = [] + terminal.onData((data) => emitted.push(data)) + const inputCalls: string[] = [] + const passThrough = terminal.input.bind(terminal) + vi.spyOn(terminal, 'input').mockImplementation((data: string, wasUserInput?: boolean) => { + inputCalls.push(data) + passThrough(data, wasUserInput) + }) + + const transport = { + getPtyId: () => 'pty-1', + sendInput: vi.fn(() => true) + } as unknown as PtyTransport + const pane = { id: 1, leafId: '00000000-0000-4000-8000-000000000001', terminal } + const manager = { + getActivePane: () => pane, + getPanes: () => [pane] + } as unknown as PaneManager + const deps = { + tabId: 'tab-1', + worktreeId: 'worktree-1', + isActive: true, + keyboardScopeRef: { current: scope }, + managerRef: { current: manager }, + paneTransportsRef: { current: new Map([[pane.id, transport]]) }, + panePtyBindingsRef: { current: new Map() }, + paneCwdRef: { current: new Map() }, + fallbackCwd: '', + expandedPaneIdRef: { current: null }, + setExpandedPane: vi.fn(), + restoreExpandedLayout: vi.fn(), + refreshPaneSizes: vi.fn(), + persistLayoutSnapshot: vi.fn(), + toggleExpandPane: vi.fn(), + setSearchOpen: vi.fn(), + onSearchSelectedText: vi.fn(), + onRequestClosePane: vi.fn(), + onClearPaneScrollback: vi.fn(), + onSetTitle: vi.fn(), + onClearPaneTitle: vi.fn(), + searchOpenRef: { current: false }, + searchStateRef: { current: { query: '', caseSensitive: false, regex: false } }, + macOptionAsAltRef: { current: 'false' } + } as unknown as Parameters[0] + + const hook = renderHook(() => useTerminalKeyboardShortcuts(deps)) + return { + textarea, + inputCalls, + emitted, + unmount: () => { + hook.unmount() + scope.remove() + } + } +} + +function dispatchRow(textarea: HTMLTextAreaElement, row: RecordedRow): void { + if (row.t === 'keydown' || row.t === 'keyup') { + const event = new KeyboardEvent(row.t, { + key: row.key, + code: row.code, + metaKey: row.meta === true, + altKey: row.alt === true, + bubbles: true, + cancelable: true + }) + Object.defineProperties(event, { + isComposing: { value: row.isComposing === true }, + keyCode: { value: row.keyCode } + }) + textarea.dispatchEvent(event) + return + } + if (row.t.startsWith('composition')) { + const event = new CompositionEvent(row.t, { bubbles: true }) + // happy-dom ignores CompositionEventInit.data, but Chromium supplies it. + Object.defineProperty(event, 'data', { value: row.data ?? '' }) + textarea.dispatchEvent(event) + return + } + if (row.t === 'input') { + textarea.value = row.value ?? '' + textarea.selectionStart = textarea.value.length + textarea.selectionEnd = textarea.value.length + textarea.dispatchEvent( + new InputEvent('input', { data: row.data, inputType: row.inputType, bubbles: true }) + ) + } +} + +async function replay(textarea: HTMLTextAreaElement, rows: RecordedRow[]): Promise { + for (const row of rows) { + dispatchRow(textarea, row) + // Why: rows are spaced by a microtask, not a task. Chromium delivers the replayed keydown + // in the same input task as the marked one, so a task gap here would let the carry's + // animation-frame expiry run in between and the replay would no longer be recognised — + // a timing the hardware never produces. + await Promise.resolve() + } + await macrotask() + await macrotask() +} + +describe('recorded macOS chord traces during an IME composition', () => { + beforeEach(() => { + vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({ + measureText: () => ({ width: 10 }) + } as unknown as CanvasRenderingContext2D) + vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue(MAC_USER_AGENT) + }) + + afterEach(() => { + cleanup() + vi.restoreAllMocks() + document.body.replaceChildren() + }) + + it.each(CASES.map((testCase) => [testCase.name, testCase] as const))( + '%s', + async (_name, testCase) => { + const rig = openRig() + await replay(rig.textarea, testCase.rows) + + expect(rig.inputCalls).toEqual(testCase.expectCalls) + expect(rig.emitted).toEqual(testCase.expectEmitted) + rig.unmount() + } + ) + + // The carry must not outlive its gesture: an ordinary chord pressed later, with no + // composition anywhere near it, still has to reach the shell. + it('releases the carry so a later ordinary chord still sends', async () => { + const rig = openRig() + await replay(rig.textarea, CASES[0].rows) + expect(rig.inputCalls).toEqual(CASES[0].expectCalls) + + dispatchRow(rig.textarea, { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 37, + meta: true + }) + await macrotask() + + expect(rig.inputCalls).toEqual([...CASES[0].expectCalls, CASES[0].expectCalls[0]]) + rig.unmount() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index e80afdae9faa..224210a81f99 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -6,9 +6,13 @@ import type { IDisposable } from '@xterm/xterm' import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager' import type { PtyTransport } from './pty-transport' import { safeFind } from '../terminal-search-safe-find' -import { resolveTerminalShortcutAction } from './terminal-shortcut-policy' +import { + isImeExemptTerminalChord, + resolveTerminalShortcutAction +} from './terminal-shortcut-policy' import type { MacOptionAsAlt } from './terminal-shortcut-policy' import { createTerminalNativeOnlyShortcutTracker } from './terminal-native-only-shortcut' +import { createTerminalImeChordRedispatchLedger } from './terminal-ime-chord-redispatch' import { installTerminalNativeInputListeners } from './terminal-native-input-listeners' import { keybindingMatchesAction, @@ -43,6 +47,16 @@ import { syncTerminalScrollIntentFromViewport } from '@/lib/pane-manager/terminal-scroll-intent' +/** + * True when a live composition owns the keydown outright. A chord over an arrow or + * Backspace/Delete is excluded: the IME never claims those, and dropping them makes + * Cmd+← do nothing mid-syllable (#12871). xterm queues the resulting bytes behind the + * preedit, so letting them through cannot reorder them ahead of the commit. + */ +function isImeOwnedTerminalKeyEvent(event: Parameters[0]): boolean { + return isImeOwnedKeyboardEvent(event) && !isImeExemptTerminalChord(event) +} + export function resolveTerminalKeyboardShortcutAction( event: Parameters[0], isMac: Parameters[1], @@ -57,7 +71,7 @@ export function resolveTerminalKeyboardShortcutAction( isWindowsTerminalHost: NonNullable[10]>, terminalShortcutPolicy: Parameters[11] = 'orca-first' ): ReturnType { - if (isImeOwnedKeyboardEvent(event)) { + if (isImeOwnedTerminalKeyEvent(event)) { return null } // Why: keep the host callback required at the production boundary so a @@ -271,6 +285,7 @@ export function useTerminalKeyboardShortcuts({ // location from its own keydown event and clear it on keyup. let optionKeyLocation = 0 const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker() + const imeChordRedispatchLedger = createTerminalImeChordRedispatchLedger() const disposeNativeInputListeners = installTerminalNativeInputListeners( window, nativeOnlyShortcutTracker, @@ -372,7 +387,14 @@ export function useTerminalKeyboardShortcuts({ if (keyboardScope && !keyboardEventBelongsToScope(e, keyboardScope)) { return } - if (isImeOwnedKeyboardEvent(e)) { + if (isImeOwnedTerminalKeyEvent(e)) { + return + } + // Korean's input source replays the chord unmarked after committing; it was already + // sent from the marked keydown, so let the replay pass without acting on it again. + if (imeChordRedispatchLedger.isRedispatchOfSentChord(e)) { + e.preventDefault() + e.stopImmediatePropagation() return } // Why: replace stale state only for this physical key so rollover cannot @@ -439,6 +461,9 @@ export function useTerminalKeyboardShortcuts({ return } pane.terminal.input(action.data) + if (isImeOwnedKeyboardEvent(e)) { + imeChordRedispatchLedger.claimSentChord(e) + } recordTerminalUserInputForLeaf(tabId, pane.leafId) if (action.data === '\x1b[13;2u') { const binding = panePtyBindingsRef.current.get(pane.id) as @@ -638,10 +663,17 @@ export function useTerminalKeyboardShortcuts({ } } + const onImeChordKeyUp = (e: KeyboardEvent): void => { + imeChordRedispatchLedger.onKeyUp(e) + } + window.addEventListener('keydown', onKeyDown, { capture: true }) + window.addEventListener('keyup', onImeChordKeyUp, { capture: true }) return () => { disposeNativeInputListeners() + imeChordRedispatchLedger.reset() window.removeEventListener('keydown', onKeyDown, { capture: true }) + window.removeEventListener('keyup', onImeChordKeyUp, { capture: true }) } }, [ isActive, diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts b/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts new file mode 100644 index 000000000000..ef9cdc41d373 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts @@ -0,0 +1,82 @@ +import { isImeOwnedKeyboardEvent } from '@/lib/ime-composition-keyboard-event' + +export type ImeChordRedispatchEvent = { + code?: string + key: string + keyCode?: number + isComposing?: boolean + metaKey: boolean + ctrlKey: boolean + altKey: boolean +} + +export type TerminalImeChordRedispatchLedger = { + /** Record that a chord was sent from its IME-marked keydown. */ + claimSentChord: (event: ImeChordRedispatchEvent) => void + /** True when this keydown is the platform's replay of a chord already sent. */ + isRedispatchOfSentChord: (event: ImeChordRedispatchEvent) => boolean + onKeyUp: (event: ImeChordRedispatchEvent) => void + reset: () => void +} + +function hasChordModifier(event: ImeChordRedispatchEvent): boolean { + return event.metaKey || event.ctrlKey || event.altKey +} + +/** + * Why: whether a modifier chord pressed mid-composition is replayed depends on the input + * source, and the two cases are indistinguishable at the marked keydown. Recorded on stock + * macOS, both `code='ArrowLeft'` `keyCode=229` `isComposing=true`: + * + * - Korean 2-Set commits the syllable, emits `compositionend`, and the platform then + * replays the chord unmarked (`keyCode=37`, `isComposing=false`) after keyup. + * - Japanese conversion swallows it: no `compositionend`, no replay, `isComposing` still + * true at keyup, and the chord never reaches the shell at all. + * + * Sending on the marked keydown covers Japanese; retiring the replay keeps Korean at one + * firing instead of two, which for `Option+←` is the difference between one word and two. + * + * The carry expires on the next frame rather than on keyup, because macOS delivers keyup + * *before* the replay — the same ordering `useImeEnterGestureOwnership` documents for Enter. + */ +export function createTerminalImeChordRedispatchLedger(): TerminalImeChordRedispatchLedger { + let pending: { code: string; token: object } | null = null + + const reset = (): void => { + pending = null + } + + return { + claimSentChord: (event) => { + if (!event.code) { + return + } + pending = { code: event.code, token: {} } + }, + isRedispatchOfSentChord: (event) => { + if (!pending || !event.code || event.code !== pending.code) { + return false + } + // The replay is the unmarked copy of the same physical chord; a still-marked event is + // the IME's own, and a bare press is the user reaching the shell on purpose. + if (isImeOwnedKeyboardEvent(event) || !hasChordModifier(event)) { + return false + } + pending = null + return true + }, + onKeyUp: (event) => { + const carried = pending + if (!carried || event.code !== carried.code) { + return + } + // Identity-scoped so a stale expiry cannot clear a newer chord's carry. + requestAnimationFrame(() => { + if (pending === carried) { + pending = null + } + }) + }, + reset + } +} diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts index 6ab00a138065..93d790d71814 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -19,6 +19,28 @@ export type TerminalShortcutEvent = { export type MacOptionAsAlt = 'true' | 'false' | 'left' | 'right' +// Why: an IME owns every keydown while a composition is live, but it never claims a +// modifier chord over one of these physical keys — so Orca must still translate them +// instead of dropping the press (#12871). Read `code`, not `key`: a CJK input source +// rewrites `key` mid-composition while `code` keeps the physical key (#12171, #13033). +const IME_EXEMPT_CHORD_CODES = new Set(['ArrowLeft', 'ArrowRight', 'Backspace', 'Delete']) + +function physicalChordKey(event: TerminalShortcutEvent): string { + return event.code && IME_EXEMPT_CHORD_CODES.has(event.code) ? event.code : event.key +} + +/** + * True when an IME-owned keydown is nonetheless an Orca terminal chord. A lone modifier + * is excluded, so a mode-switch gesture such as composing Ctrl+Space never reaches the + * shortcut policy. + */ +export function isImeExemptTerminalChord(event: TerminalShortcutEvent): boolean { + return ( + IME_EXEMPT_CHORD_CODES.has(event.code ?? '') && + (event.metaKey || event.ctrlKey || event.altKey) + ) +} + // Shared close-chord predicate: the terminal pane (L3) and the floating panel's focused-terminal // branch (L2) both treat terminal.closePane OR a terminal-scope tab.close as "close the active // pane," so the two layers can't diverge. Callers pass the options each binding needs — @@ -112,53 +134,60 @@ export function resolveTerminalShortcutAction( hasCtrlEnterCsiUAuthority?: () => boolean ): TerminalShortcutAction | null { const platform: NodeJS.Platform = isMac ? 'darwin' : isWindows ? 'win32' : 'linux' + const chordKey = physicalChordKey(event) + // Why: a composition rewrites `key` to 'Process', which normalizes to no token at all, and + // macOS has no non-Latin physical fallback (shared/keybindings.ts). Without substituting the + // physical key a user's remap onto Cmd+Backspace would miss and fall through to the built-in + // kill-line byte — silence before the IME exemption, the wrong action after it. + const chordEvent: TerminalShortcutEvent = + chordKey === event.key ? event : { ...event, key: chordKey } // Why: capture this chord even on repeat without blocking the OS default input-source switch. - if (keybindingMatchesAction('terminal.switchInputSource', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.switchInputSource', chordEvent, platform, keybindings)) { return { type: 'switchInputSource' } } if (!event.repeat) { - if (keybindingMatchesAction('terminal.copySelection', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.copySelection', chordEvent, platform, keybindings)) { return { type: 'copySelection' } } - if (keybindingMatchesAction('terminal.search', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.search', chordEvent, platform, keybindings)) { return { type: 'toggleSearch' } } - if (keybindingMatchesAction('terminal.clear', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.clear', chordEvent, platform, keybindings)) { return { type: 'clearActivePane' } } - if (keybindingMatchesAction('terminal.focusPreviousPane', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.focusPreviousPane', chordEvent, platform, keybindings)) { return { type: 'focusPane', direction: 'previous' } } - if (keybindingMatchesAction('terminal.focusNextPane', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.focusNextPane', chordEvent, platform, keybindings)) { return { type: 'focusPane', direction: 'next' } } - if (keybindingMatchesAction('terminal.equalizePaneSizes', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.equalizePaneSizes', chordEvent, platform, keybindings)) { return { type: 'equalizePaneSizes' } } - if (keybindingMatchesAction('terminal.expandPane', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.expandPane', chordEvent, platform, keybindings)) { return { type: 'toggleExpandActivePane' } } - if (keybindingMatchesAction('terminal.setTitle', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.setTitle', chordEvent, platform, keybindings)) { return { type: 'setTitle' } } - if (keybindingMatchesAction('terminal.clearPaneTitle', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.clearPaneTitle', chordEvent, platform, keybindings)) { return { type: 'clearPaneTitle' } } // Why: recognize the active tab.close binding as a pane-close alias too, so a user who remaps // tab.close alone still closes the focused split pane (never the whole tab); L2 always defers to us. if ( - isTerminalPaneCloseChord(event, platform, keybindings, undefined, { + isTerminalPaneCloseChord(chordEvent, platform, keybindings, undefined, { context: 'terminal', terminalShortcutPolicy }) @@ -166,11 +195,11 @@ export function resolveTerminalShortcutAction( return { type: 'closeActivePane' } } - if (keybindingMatchesAction('terminal.splitRight', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.splitRight', chordEvent, platform, keybindings)) { return { type: 'splitActivePane', direction: 'vertical' } } - if (keybindingMatchesAction('terminal.splitDown', event, platform, keybindings)) { + if (keybindingMatchesAction('terminal.splitDown', chordEvent, platform, keybindings)) { return { type: 'splitActivePane', direction: 'horizontal' } } } @@ -214,23 +243,23 @@ export function resolveTerminalShortcutAction( !event.metaKey && !event.altKey && !event.shiftKey && - event.key === 'Backspace' + chordKey === 'Backspace' ) { return { type: 'sendInput', data: '\x17' } } if (isMac && event.metaKey && !event.ctrlKey && !event.altKey && !event.shiftKey) { - if (event.key === 'Backspace') { + if (chordKey === 'Backspace') { return { type: 'sendInput', data: '\x15' } } - if (event.key === 'Delete') { + if (chordKey === 'Delete') { return { type: 'sendInput', data: '\x0b' } } // Why: xterm.js has no Cmd+Arrow mapping; translate Cmd+←/→ to readline Ctrl+A/Ctrl+E for line start/end (iTerm2/Ghostty). - if (event.key === 'ArrowLeft') { + if (chordKey === 'ArrowLeft') { return { type: 'sendInput', data: '\x01' } } - if (event.key === 'ArrowRight') { + if (chordKey === 'ArrowRight') { return { type: 'sendInput', data: '\x05' } } // Why: macOS users expect Cmd+↑/↓ to scroll scrollback, not write escape bytes to the shell. @@ -247,7 +276,7 @@ export function resolveTerminalShortcutAction( !event.ctrlKey && event.altKey && !event.shiftKey && - event.key === 'Backspace' + chordKey === 'Backspace' ) { // Why: a kitty-protocol TUI binds the CSI 127;3u xterm emits natively; the legacy \x1b\x7f fallback would bypass it. if (isKittyKeyboardActivePane?.()) { @@ -261,14 +290,14 @@ export function resolveTerminalShortcutAction( !event.ctrlKey && event.altKey && !event.shiftKey && - (event.key === 'ArrowLeft' || event.key === 'ArrowRight') + (chordKey === 'ArrowLeft' || chordKey === 'ArrowRight') ) { // Why: a kitty-protocol TUI binds alt+arrow via xterm's native CSI 1;3D/C; \eb/\ef would reach it as alt+b/f. if (isKittyKeyboardActivePane?.()) { return null } // Why: readline doesn't bind xterm's \e[1;3D/C for alt+←/→, so translate to \eb/\ef for word-nav (iTerm2 "Esc+" behavior). - return { type: 'sendInput', data: event.key === 'ArrowLeft' ? '\x1bb' : '\x1bf' } + return { type: 'sendInput', data: chordKey === 'ArrowLeft' ? '\x1bb' : '\x1bf' } } if ( @@ -277,14 +306,14 @@ export function resolveTerminalShortcutAction( event.ctrlKey && !event.altKey && !event.shiftKey && - (event.key === 'ArrowLeft' || event.key === 'ArrowRight') + (chordKey === 'ArrowLeft' || chordKey === 'ArrowRight') ) { // Why: local Windows ConPTY (PSReadLine) binds Ctrl+←/→ itself; sending \eb/\ef prints stray b/f. Remote/WSL run readline. if (isLocalWindowsConptyPane?.()) { return null } // Why: readline ignores xterm's \e[1;5D/C, so translate Ctrl+←/→ to \eb/\ef for word-nav; !isMac since Mac reserves Ctrl+Arrow. - return { type: 'sendInput', data: event.key === 'ArrowLeft' ? '\x1bb' : '\x1bf' } + return { type: 'sendInput', data: chordKey === 'ArrowLeft' ? '\x1bb' : '\x1bf' } } // Why: macOptionIsMeta stays off so non-US layouts can compose @/€; match event.code since composition rewrites event.key. From 396c274d71b35b85e607f2cc41159730c3112898 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 8 Aug 2026 19:57:14 +0900 Subject: [PATCH 04/15] fix(terminal): close the redispatch ledger's gaps found in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, two of them introduced by the previous commit. Claiming happened only on the byte path, but widening the keybinding lookup is what let a remapped chord resolve mid-composition — and a remap produces a pane command, not bytes. The replay then ran it again: two panes closed, or two clears, from one press. Claim now covers every resolved action. Pinned by replaying the recorded Korean trace with terminal.clear remapped onto Mod+Backspace, which counts 2 without the fix. The carry expired on an animation frame, so any slow task landing between keyup and the replay let the chord fire twice. It now expires when the last chord modifier comes up, which is the gesture's own boundary in both traces and does not depend on timing at all. The trace replay now spaces rows by a full task rather than a microtask, so a regression back to a timer would fail it. A gesture interrupted by focus loss never releases its modifier, leaving the carry armed to swallow an ordinary chord later; blur now resets it, as both siblings in this effect already do. The keyup listener is scoped like the keydown one so another pane's release cannot retire this pane's carry. Also narrows the physical-key substitution to IME-owned events — an X11 keysym remap preserves code while changing key, and reading code there would have overridden it on ordinary presses. --- ....issue-12871-recorded-chord-traces.test.ts | 31 ++++++-- .../terminal-pane/keyboard-handlers.ts | 19 ++++- .../terminal-ime-chord-redispatch.test.ts | 77 +++++++++++++++++++ .../terminal-ime-chord-redispatch.ts | 47 +++++------ .../terminal-pane/terminal-shortcut-policy.ts | 22 ++++-- 5 files changed, 154 insertions(+), 42 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.test.ts diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index 5e11aee8f9a2..99456198cbc0 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -599,10 +599,11 @@ type Rig = { textarea: HTMLTextAreaElement inputCalls: string[] emitted: string[] + clearPaneCalls: number unmount: () => void } -function openRig(): Rig { +function openRig(overrides: Record = {}): Rig { const scope = document.createElement('div') document.body.append(scope) const container = document.createElement('div') @@ -656,7 +657,8 @@ function openRig(): Rig { onClearPaneTitle: vi.fn(), searchOpenRef: { current: false }, searchStateRef: { current: { query: '', caseSensitive: false, regex: false } }, - macOptionAsAltRef: { current: 'false' } + macOptionAsAltRef: { current: 'false' }, + ...overrides } as unknown as Parameters[0] const hook = renderHook(() => useTerminalKeyboardShortcuts(deps)) @@ -664,6 +666,9 @@ function openRig(): Rig { textarea, inputCalls, emitted, + get clearPaneCalls() { + return (deps.onClearPaneScrollback as ReturnType).mock.calls.length + }, unmount: () => { hook.unmount() scope.remove() @@ -708,11 +713,10 @@ function dispatchRow(textarea: HTMLTextAreaElement, row: RecordedRow): void { async function replay(textarea: HTMLTextAreaElement, rows: RecordedRow[]): Promise { for (const row of rows) { dispatchRow(textarea, row) - // Why: rows are spaced by a microtask, not a task. Chromium delivers the replayed keydown - // in the same input task as the marked one, so a task gap here would let the carry's - // animation-frame expiry run in between and the replay would no longer be recognised — - // a timing the hardware never produces. - await Promise.resolve() + // A full task between rows, deliberately: the carry is released by the modifier coming up + // rather than by a timer, so no amount of delay between the marked press and the replay + // may change the outcome. Spacing rows more tightly would hide a regression to a timer. + await macrotask() } await macrotask() await macrotask() @@ -744,6 +748,19 @@ describe('recorded macOS chord traces during an IME composition', () => { } ) + // A chord remapped onto one of these keys resolves to a pane command rather than bytes. + // The carry has to cover that too, or the replay runs the command a second time — which is + // two panes closed, or two clears, from one press. + it('runs a remapped pane command once across the replay', async () => { + const rig = openRig({ keybindings: { 'terminal.clear': ['Mod+Backspace'] } }) + await replay(rig.textarea, CASES[2].rows) + + expect(rig.clearPaneCalls).toBe(1) + // The remap wins outright: no kill-line byte is sent alongside it. + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + // The carry must not outlive its gesture: an ordinary chord pressed later, with no // composition anywhere near it, still has to reach the shell. it('releases the carry so a later ordinary chord still sends', async () => { diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 224210a81f99..9bca09ad3139 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -444,6 +444,12 @@ export function useTerminalKeyboardShortcuts({ if (!action) { return } + // Claim before acting, and for every action rather than the byte path alone: a chord + // remapped onto one of these keys resolves to a pane command, and the replay would run + // it a second time — two panes closed from one press. + if (isImeOwnedKeyboardEvent(e)) { + imeChordRedispatchLedger.claimSentChord(e) + } if (action.type === 'switchInputSource') { // Why: the OS must receive its default action, while xterm must receive @@ -461,9 +467,6 @@ export function useTerminalKeyboardShortcuts({ return } pane.terminal.input(action.data) - if (isImeOwnedKeyboardEvent(e)) { - imeChordRedispatchLedger.claimSentChord(e) - } recordTerminalUserInputForLeaf(tabId, pane.leafId) if (action.data === '\x1b[13;2u') { const binding = panePtyBindingsRef.current.get(pane.id) as @@ -664,16 +667,26 @@ export function useTerminalKeyboardShortcuts({ } const onImeChordKeyUp = (e: KeyboardEvent): void => { + // Scoped like onKeyDown: another pane's release must not retire this pane's carry. + const keyboardScope = keyboardScopeRef.current + if (keyboardScope && !keyboardEventBelongsToScope(e, keyboardScope)) { + return + } imeChordRedispatchLedger.onKeyUp(e) } + const onImeChordBlur = (): void => { + imeChordRedispatchLedger.reset() + } window.addEventListener('keydown', onKeyDown, { capture: true }) window.addEventListener('keyup', onImeChordKeyUp, { capture: true }) + window.addEventListener('blur', onImeChordBlur) return () => { disposeNativeInputListeners() imeChordRedispatchLedger.reset() window.removeEventListener('keydown', onKeyDown, { capture: true }) window.removeEventListener('keyup', onImeChordKeyUp, { capture: true }) + window.removeEventListener('blur', onImeChordBlur) } }, [ isActive, diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.test.ts new file mode 100644 index 000000000000..34a0e1f7f7a8 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.test.ts @@ -0,0 +1,77 @@ +// @vitest-environment happy-dom +// The recorded traces cover the ledger end to end, but they all hold their modifier through the +// replay and always carry a `code`, so these guards are unreachable from them. Pinned here +// because deleting either would leave every recorded case green while a bare press or a +// codeless event started being swallowed. +import { describe, expect, it } from 'vitest' +import { + createTerminalImeChordRedispatchLedger, + type ImeChordRedispatchEvent +} from './terminal-ime-chord-redispatch' + +function event(overrides: Partial): ImeChordRedispatchEvent { + return { + code: 'ArrowLeft', + keyCode: 37, + isComposing: false, + metaKey: false, + ctrlKey: false, + altKey: false, + ...overrides + } +} + +const MARKED_CHORD = event({ keyCode: 229, isComposing: true, metaKey: true }) + +describe('ime chord redispatch ledger', () => { + it('retires the unmarked replay of a chord it sent', () => { + const ledger = createTerminalImeChordRedispatchLedger() + ledger.claimSentChord(MARKED_CHORD) + + expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(true) + // Spent: a second press of the same chord is the user's own, not a replay. + expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(false) + }) + + it.each([ + ['a different physical key', event({ code: 'ArrowRight', metaKey: true })], + ['a bare press with no modifier', event({})], + ['a still-marked event, which is the IME’s own', event({ keyCode: 229, isComposing: true, metaKey: true })] + ])('does not retire %s', (_label, replay) => { + const ledger = createTerminalImeChordRedispatchLedger() + ledger.claimSentChord(MARKED_CHORD) + + expect(ledger.isRedispatchOfSentChord(replay)).toBe(false) + }) + + it('claims nothing from an event with no physical code', () => { + const ledger = createTerminalImeChordRedispatchLedger() + ledger.claimSentChord(event({ code: undefined, keyCode: 229, isComposing: true, metaKey: true })) + + expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(false) + }) + + // macOS delivers the key's own keyup before the replay, with the modifier still held, so + // that release must not end the gesture. Releasing the modifier must. + it('survives the chord key coming up and ends when the modifier does', () => { + const ledger = createTerminalImeChordRedispatchLedger() + ledger.claimSentChord(MARKED_CHORD) + + ledger.onKeyUp(event({ metaKey: true })) + expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(true) + + ledger.claimSentChord(MARKED_CHORD) + ledger.onKeyUp(event({ code: 'MetaLeft', keyCode: 91 })) + expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(false) + }) + + // A gesture interrupted by focus loss never releases its modifier, so without this the + // carry would sit armed and eat an ordinary chord later. + it('drops the carry on reset', () => { + const ledger = createTerminalImeChordRedispatchLedger() + ledger.claimSentChord(MARKED_CHORD) + ledger.reset() + + expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(false) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts b/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts index ef9cdc41d373..75199ee790be 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts @@ -2,7 +2,6 @@ import { isImeOwnedKeyboardEvent } from '@/lib/ime-composition-keyboard-event' export type ImeChordRedispatchEvent = { code?: string - key: string keyCode?: number isComposing?: boolean metaKey: boolean @@ -11,9 +10,9 @@ export type ImeChordRedispatchEvent = { } export type TerminalImeChordRedispatchLedger = { - /** Record that a chord was sent from its IME-marked keydown. */ + /** Record that a chord was acted on from its IME-marked keydown. */ claimSentChord: (event: ImeChordRedispatchEvent) => void - /** True when this keydown is the platform's replay of a chord already sent. */ + /** True when this keydown is the platform's replay of a chord already acted on. */ isRedispatchOfSentChord: (event: ImeChordRedispatchEvent) => boolean onKeyUp: (event: ImeChordRedispatchEvent) => void reset: () => void @@ -28,33 +27,31 @@ function hasChordModifier(event: ImeChordRedispatchEvent): boolean { * source, and the two cases are indistinguishable at the marked keydown. Recorded on stock * macOS, both `code='ArrowLeft'` `keyCode=229` `isComposing=true`: * - * - Korean 2-Set commits the syllable, emits `compositionend`, and the platform then - * replays the chord unmarked (`keyCode=37`, `isComposing=false`) after keyup. + * - Korean 2-Set commits the syllable, emits `compositionend`, and the platform replays the + * chord unmarked (`keyCode=37`, `isComposing=false`) after keyup. * - Japanese conversion swallows it: no `compositionend`, no replay, `isComposing` still * true at keyup, and the chord never reaches the shell at all. * - * Sending on the marked keydown covers Japanese; retiring the replay keeps Korean at one + * Acting on the marked keydown covers Japanese; retiring the replay keeps Korean at one * firing instead of two, which for `Option+←` is the difference between one word and two. * - * The carry expires on the next frame rather than on keyup, because macOS delivers keyup - * *before* the replay — the same ordering `useImeEnterGestureOwnership` documents for Enter. + * The carry is released when the last chord modifier comes up, not on a timer. Both traces + * hold the modifier across the whole gesture and release it after the replay, so that edge is + * the gesture's own boundary — where an animation-frame expiry would race any slow task + * landing between keyup and the replay and let the chord through twice. */ export function createTerminalImeChordRedispatchLedger(): TerminalImeChordRedispatchLedger { - let pending: { code: string; token: object } | null = null - - const reset = (): void => { - pending = null - } + let pendingCode: string | null = null return { claimSentChord: (event) => { if (!event.code) { return } - pending = { code: event.code, token: {} } + pendingCode = event.code }, isRedispatchOfSentChord: (event) => { - if (!pending || !event.code || event.code !== pending.code) { + if (pendingCode === null || event.code !== pendingCode) { return false } // The replay is the unmarked copy of the same physical chord; a still-marked event is @@ -62,21 +59,19 @@ export function createTerminalImeChordRedispatchLedger(): TerminalImeChordRedisp if (isImeOwnedKeyboardEvent(event) || !hasChordModifier(event)) { return false } - pending = null + pendingCode = null return true }, onKeyUp: (event) => { - const carried = pending - if (!carried || event.code !== carried.code) { - return + if (!hasChordModifier(event)) { + pendingCode = null } - // Identity-scoped so a stale expiry cannot clear a newer chord's carry. - requestAnimationFrame(() => { - if (pending === carried) { - pending = null - } - }) }, - reset + // Why: a gesture interrupted by focus loss never releases its modifier, and a carry left + // armed would swallow an ordinary chord later. Both siblings in this effect drop their + // state on blur for the same reason. + reset: () => { + pendingCode = null + } } } diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts index 93d790d71814..d5e7df3d7f7b 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -6,6 +6,7 @@ import { type TerminalShortcutPolicy } from '../../../../shared/keybindings' import type { WindowsShiftEnterEncoding } from './terminal-windows-shift-enter' +import { isImeOwnedKeyboardEvent } from '@/lib/ime-composition-keyboard-event' export type TerminalShortcutEvent = { key: string @@ -19,20 +20,29 @@ export type TerminalShortcutEvent = { export type MacOptionAsAlt = 'true' | 'false' | 'left' | 'right' -// Why: an IME owns every keydown while a composition is live, but it never claims a -// modifier chord over one of these physical keys — so Orca must still translate them -// instead of dropping the press (#12871). Read `code`, not `key`: a CJK input source -// rewrites `key` mid-composition while `code` keeps the physical key (#12171, #13033). +// Why: a live composition owns every keydown, but a Japanese conversion swallows a modifier +// chord over one of these keys outright — no commit, no platform replay, nothing reaching the +// shell (#12871). Exempting them costs a double firing on input sources that DO replay, which +// terminal-ime-chord-redispatch.ts retires. Read `code`, not `key`: Windows reports an +// IME-consumed key as `key: 'Process'` (#12171, #13033). +// +// ArrowUp/ArrowDown are left out because no trace covers them, not because the reasoning +// stops there — Cmd+↑/↓ scrolls the viewport and writes no bytes, so it is the safer half. const IME_EXEMPT_CHORD_CODES = new Set(['ArrowLeft', 'ArrowRight', 'Backspace', 'Delete']) +// Gated on IME ownership so an ordinary press keeps following `key`: an X11 keysym remap +// preserves `code` while changing `key`, and reading `code` there would silently override it. function physicalChordKey(event: TerminalShortcutEvent): string { - return event.code && IME_EXEMPT_CHORD_CODES.has(event.code) ? event.code : event.key + if (!isImeOwnedKeyboardEvent(event) || !event.code) { + return event.key + } + return IME_EXEMPT_CHORD_CODES.has(event.code) ? event.code : event.key } /** * True when an IME-owned keydown is nonetheless an Orca terminal chord. A lone modifier * is excluded, so a mode-switch gesture such as composing Ctrl+Space never reaches the - * shortcut policy. + * shortcut policy, and so is Shift, which Japanese conversion binds to resize a segment. */ export function isImeExemptTerminalChord(event: TerminalShortcutEvent): boolean { return ( From dcb89489e57f3103e276ab6c6030893a2252b1d7 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 8 Aug 2026 20:36:15 +0900 Subject: [PATCH 05/15] fix(terminal): decide a swallowed IME chord from its release The two input sources are indistinguishable while the key is down and separate by the time it comes up: Korean has ended its composition and a platform replay is on the way, Japanese is still composing and nothing else will ever deliver the chord. So yield every IME-owned keydown as main does, and resolve the chord from its release only when the composition is still live. Nothing is swallowed and no state is carried between events, which retires the redispatch ledger and the whole class of gaps it had to cover. Also stop rebuilding the chord event with a spread. A real KeyboardEvent keeps its properties as prototype accessors, so `{ ...event }` produced an empty object and every keybinding lookup silently missed in production while the plain-object test doubles passed. The Japanese trace is replaced with the full capture, including the modifier presses and releases that hand transcription had dropped. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Mu6j33LNsd4Mvv8a1NuFoo --- ...ndlers.issue-12871-composing-chord.test.ts | 15 +- ....issue-12871-recorded-chord-traces.test.ts | 801 ++++++------------ .../terminal-pane/keyboard-handlers.ts | 101 ++- .../terminal-ime-chord-redispatch.test.ts | 77 -- .../terminal-ime-chord-redispatch.ts | 77 -- .../terminal-pane/terminal-shortcut-policy.ts | 32 +- 6 files changed, 339 insertions(+), 764 deletions(-) delete mode 100644 src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.test.ts delete mode 100644 src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts index 4c361d5b501b..fec5ff012701 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts @@ -3,8 +3,9 @@ // which replays captured macOS Korean and Japanese traces through the real handler and xterm. // // The exemption exists because a Japanese conversion swallows a modifier chord outright — no -// commit, no platform replay, nothing reaches the shell. Its counterweight is the redispatch -// ledger, since Korean's input source *does* replay the chord and would otherwise fire twice. +// commit, no platform replay, nothing reaches the shell. It is what lets the pane resolve such +// a chord from its *release*; on the keydown the pane still yields, because Korean's input +// source replays the chord instead and acting on both copies would fire it twice. // // Scope: shapes here are constructed from the DOM contract, not captured. The captured file is // where hardware fidelity lives; this file pins the decision boundary, including the negatives. @@ -31,7 +32,9 @@ function keyEvent(overrides: Partial): ComposingKeyEvent { } } -function resolveOnMac(event: ComposingKeyEvent): ReturnType { +function resolveOnMac( + event: ComposingKeyEvent +): ReturnType { return resolveTerminalKeyboardShortcutAction( event, true, @@ -54,7 +57,11 @@ describe('terminal chords stay live during an IME composition', () => { ['Cmd+Backspace', keyEvent({ key: 'Backspace', code: 'Backspace', metaKey: true }), '\x15'], ['Cmd+Delete', keyEvent({ key: 'Delete', code: 'Delete', metaKey: true }), '\x0b'], ['Option+ArrowLeft', keyEvent({ key: 'ArrowLeft', code: 'ArrowLeft', altKey: true }), '\x1bb'], - ['Option+ArrowRight', keyEvent({ key: 'ArrowRight', code: 'ArrowRight', altKey: true }), '\x1bf'] + [ + 'Option+ArrowRight', + keyEvent({ key: 'ArrowRight', code: 'ArrowRight', altKey: true }), + '\x1bf' + ] ])('resolves %s while a composition is live', (_label, event, expected) => { // The idle case is asserted too because it is not covered elsewhere with a populated // `code`: terminal-shortcut-policy.test.ts builds its fixtures with `code: ''`, which diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index 99456198cbc0..078c1cca61f9 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -1,20 +1,20 @@ // @vitest-environment happy-dom // Recorded on stock macOS (Chrome 151) by logging keydown/keyup/composition*/input on a // textarea, then replayed here through the production handler and a real patched Terminal. -// Every row below is captured, not authored. +// Every row below is captured, not authored, including the modifier presses and releases. // // The two input sources answer a modifier chord in opposite ways, and the marked keydown is // identical in both — code='ArrowLeft', keyCode=229, isComposing=true — so nothing tells them -// apart at the moment the handler has to decide: +// apart while the key is down. The release does: // // - Korean 2-Set commits the syllable, emits compositionend, and the platform replays the -// chord unmarked after keyup. Acting on both copies sends Option+Left's \x1bb twice, which -// jumps two words instead of one. +// chord unmarked. isComposing is false by its keyup, and that replay resolves on its own. // - Japanese conversion swallows the chord outright: no compositionend, no replay, still -// composing at keyup. Before the exemption these three chords produced no bytes at all. +// composing at keyup. Before this fix its chords produced no bytes at all. // -// So both halves are pinned together: the fix is only correct if Korean stays at one firing -// while Japanese goes from zero to one each. +// Both directions are pinned together, because the failure modes point opposite ways: Korean +// must stay at one firing per chord (a second \x1bb jumps two words), Japanese must go from +// zero to one. A change that only reads the keydown cannot satisfy both. import { Terminal } from '@xterm/xterm' import { cleanup, renderHook } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -44,547 +44,205 @@ type RecordedCase = { const CASES: RecordedCase[] = [ { - "name": "Korean 2-Set, Cmd+ArrowLeft", - "expectCalls": [ - "\u0001" - ], - "expectEmitted": [ - "사\u0001" - ], - "rows": [ - { - "t": "keydown", - "key": "ㅅ", - "code": "KeyT", - "keyCode": 229 - }, - { - "t": "compositionstart", - "data": "" - }, - { - "t": "compositionupdate", - "data": "ㅅ" - }, - { - "t": "input", - "data": "ㅅ", - "inputType": "insertCompositionText", - "value": "ㅅ" - }, - { - "t": "keyup", - "key": "ㅅ", - "code": "KeyT", - "keyCode": 84, - "isComposing": true - }, - { - "t": "keydown", - "key": "ㅏ", - "code": "KeyK", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "사" - }, - { - "t": "input", - "data": "사", - "inputType": "insertCompositionText", - "value": "사" - }, - { - "t": "keyup", - "key": "ㅏ", - "code": "KeyK", - "keyCode": 75, - "isComposing": true - }, - { - "t": "keydown", - "key": "Meta", - "code": "MetaLeft", - "keyCode": 91, - "isComposing": true, - "meta": true - }, - { - "t": "keydown", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 229, - "isComposing": true, - "meta": true - }, - { - "t": "compositionupdate", - "data": "사" - }, - { - "t": "input", - "data": "사", - "inputType": "insertCompositionText", - "value": "사" - }, - { - "t": "compositionend", - "data": "사" - }, - { - "t": "keyup", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 37, - "meta": true - }, - { - "t": "keydown", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 37, - "meta": true - }, - { - "t": "keyup", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 37, - "meta": true - }, - { - "t": "keyup", - "key": "Meta", - "code": "MetaLeft", - "keyCode": 91 - } + name: 'Korean 2-Set, Cmd+ArrowLeft', + expectCalls: ['\x01'], + expectEmitted: ['사', '\x01'], + rows: [ + { t: 'keydown', key: 'ㅅ', code: 'KeyT', keyCode: 229 }, + { t: 'compositionstart', data: '' }, + { t: 'compositionupdate', data: 'ㅅ' }, + { t: 'input', data: 'ㅅ', inputType: 'insertCompositionText', value: 'ㅅ' }, + { t: 'keyup', key: 'ㅅ', code: 'KeyT', keyCode: 84, isComposing: true }, + { t: 'keydown', key: 'ㅏ', code: 'KeyK', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: '사' }, + { t: 'input', data: '사', inputType: 'insertCompositionText', value: '사' }, + { t: 'keyup', key: 'ㅏ', code: 'KeyK', keyCode: 75, isComposing: true }, + { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + meta: true + }, + { t: 'compositionupdate', data: '사' }, + { t: 'input', data: '사', inputType: 'insertCompositionText', value: '사' }, + { t: 'compositionend', data: '사' }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, meta: true }, + { t: 'keydown', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, meta: true }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, meta: true }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91 } ] }, { - "name": "Korean 2-Set, Option+ArrowLeft", - "expectCalls": [ - "\u001bb" - ], - "expectEmitted": [ - "사\u001bb" - ], - "rows": [ - { - "t": "keydown", - "key": "ㅅ", - "code": "KeyT", - "keyCode": 229 - }, - { - "t": "compositionstart", - "data": "" - }, - { - "t": "compositionupdate", - "data": "ㅅ" - }, - { - "t": "input", - "data": "ㅅ", - "inputType": "insertCompositionText", - "value": "ㅅ" - }, - { - "t": "keyup", - "key": "ㅅ", - "code": "KeyT", - "keyCode": 84, - "isComposing": true - }, - { - "t": "keydown", - "key": "ㅏ", - "code": "KeyK", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "사" - }, - { - "t": "input", - "data": "사", - "inputType": "insertCompositionText", - "value": "사" - }, - { - "t": "keyup", - "key": "ㅏ", - "code": "KeyK", - "keyCode": 75, - "isComposing": true - }, - { - "t": "keydown", - "key": "Alt", - "code": "AltLeft", - "keyCode": 18, - "isComposing": true, - "alt": true - }, - { - "t": "keydown", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 229, - "isComposing": true, - "alt": true - }, - { - "t": "compositionupdate", - "data": "사" - }, - { - "t": "input", - "data": "사", - "inputType": "insertCompositionText", - "value": "사" - }, - { - "t": "compositionend", - "data": "사" - }, - { - "t": "keyup", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 37, - "alt": true - }, - { - "t": "keydown", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 37, - "alt": true - }, - { - "t": "keyup", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 37, - "alt": true - }, - { - "t": "keyup", - "key": "Alt", - "code": "AltLeft", - "keyCode": 18 - } + name: 'Korean 2-Set, Option+ArrowLeft', + expectCalls: ['\x1bb'], + expectEmitted: ['사', '\x1bb'], + rows: [ + { t: 'keydown', key: 'ㅅ', code: 'KeyT', keyCode: 229 }, + { t: 'compositionstart', data: '' }, + { t: 'compositionupdate', data: 'ㅅ' }, + { t: 'input', data: 'ㅅ', inputType: 'insertCompositionText', value: 'ㅅ' }, + { t: 'keyup', key: 'ㅅ', code: 'KeyT', keyCode: 84, isComposing: true }, + { t: 'keydown', key: 'ㅏ', code: 'KeyK', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: '사' }, + { t: 'input', data: '사', inputType: 'insertCompositionText', value: '사' }, + { t: 'keyup', key: 'ㅏ', code: 'KeyK', keyCode: 75, isComposing: true }, + { t: 'keydown', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true, alt: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + alt: true + }, + { t: 'compositionupdate', data: '사' }, + { t: 'input', data: '사', inputType: 'insertCompositionText', value: '사' }, + { t: 'compositionend', data: '사' }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, alt: true }, + { t: 'keydown', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, alt: true }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, alt: true }, + { t: 'keyup', key: 'Alt', code: 'AltLeft', keyCode: 18 } ] }, { - "name": "Korean 2-Set, Cmd+Backspace", - "expectCalls": [ - "\u0015" - ], - "expectEmitted": [ - "사\u0015" - ], - "rows": [ - { - "t": "keydown", - "key": "ㅅ", - "code": "KeyT", - "keyCode": 229 - }, - { - "t": "compositionstart", - "data": "" - }, - { - "t": "compositionupdate", - "data": "ㅅ" - }, - { - "t": "input", - "data": "ㅅ", - "inputType": "insertCompositionText", - "value": "ㅅ" - }, - { - "t": "keydown", - "key": "ㅏ", - "code": "KeyK", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "사" - }, - { - "t": "input", - "data": "사", - "inputType": "insertCompositionText", - "value": "사" - }, - { - "t": "keyup", - "key": "ㅅ", - "code": "KeyT", - "keyCode": 84, - "isComposing": true - }, - { - "t": "keyup", - "key": "ㅏ", - "code": "KeyK", - "keyCode": 75, - "isComposing": true - }, - { - "t": "keydown", - "key": "Meta", - "code": "MetaLeft", - "keyCode": 91, - "isComposing": true, - "meta": true - }, - { - "t": "keydown", - "key": "Backspace", - "code": "Backspace", - "keyCode": 229, - "isComposing": true, - "meta": true - }, - { - "t": "compositionupdate", - "data": "사" - }, - { - "t": "input", - "data": "사", - "inputType": "insertCompositionText", - "value": "사" - }, - { - "t": "compositionend", - "data": "사" - }, - { - "t": "keyup", - "key": "Backspace", - "code": "Backspace", - "keyCode": 8, - "meta": true - }, - { - "t": "keydown", - "key": "Backspace", - "code": "Backspace", - "keyCode": 8, - "meta": true - }, - { - "t": "keyup", - "key": "Backspace", - "code": "Backspace", - "keyCode": 8, - "meta": true - }, - { - "t": "keyup", - "key": "Meta", - "code": "MetaLeft", - "keyCode": 91 - } + name: 'Korean 2-Set, Cmd+Backspace', + expectCalls: ['\x15'], + expectEmitted: ['사', '\x15'], + rows: [ + { t: 'keydown', key: 'ㅅ', code: 'KeyT', keyCode: 229 }, + { t: 'compositionstart', data: '' }, + { t: 'compositionupdate', data: 'ㅅ' }, + { t: 'input', data: 'ㅅ', inputType: 'insertCompositionText', value: 'ㅅ' }, + { t: 'keydown', key: 'ㅏ', code: 'KeyK', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: '사' }, + { t: 'input', data: '사', inputType: 'insertCompositionText', value: '사' }, + { t: 'keyup', key: 'ㅅ', code: 'KeyT', keyCode: 84, isComposing: true }, + { t: 'keyup', key: 'ㅏ', code: 'KeyK', keyCode: 75, isComposing: true }, + { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, + { + t: 'keydown', + key: 'Backspace', + code: 'Backspace', + keyCode: 229, + isComposing: true, + meta: true + }, + { t: 'compositionupdate', data: '사' }, + { t: 'input', data: '사', inputType: 'insertCompositionText', value: '사' }, + { t: 'compositionend', data: '사' }, + { t: 'keyup', key: 'Backspace', code: 'Backspace', keyCode: 8, meta: true }, + { t: 'keydown', key: 'Backspace', code: 'Backspace', keyCode: 8, meta: true }, + { t: 'keyup', key: 'Backspace', code: 'Backspace', keyCode: 8, meta: true }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91 } ] }, { - "name": "Japanese, three chords across one live preedit", - "expectCalls": [ - "\u0001", - "\u001bb", - "\u0015" - ], - "expectEmitted": [ - "日本語\u0001\u001bb\u0015" - ], - "rows": [ - { - "t": "keydown", - "key": "n", - "code": "KeyN", - "keyCode": 229 - }, - { - "t": "compositionstart", - "data": "" - }, - { - "t": "compositionupdate", - "data": "n" - }, - { - "t": "input", - "data": "n", - "inputType": "insertCompositionText", - "value": "n" - }, - { - "t": "keydown", - "key": "i", - "code": "KeyI", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "に" - }, - { - "t": "input", - "data": "に", - "inputType": "insertCompositionText", - "value": "に" - }, - { - "t": "keydown", - "key": "h", - "code": "KeyH", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "にh" - }, - { - "t": "input", - "data": "にh", - "inputType": "insertCompositionText", - "value": "にh" - }, - { - "t": "keydown", - "key": "o", - "code": "KeyO", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "にほ" - }, - { - "t": "input", - "data": "にほ", - "inputType": "insertCompositionText", - "value": "にほ" - }, - { - "t": "keydown", - "key": "n", - "code": "KeyN", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "にほn" - }, - { - "t": "input", - "data": "にほn", - "inputType": "insertCompositionText", - "value": "にほn" - }, - { - "t": "keydown", - "key": "g", - "code": "KeyG", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "にほんg" - }, - { - "t": "input", - "data": "にほんg", - "inputType": "insertCompositionText", - "value": "にほんg" - }, - { - "t": "keydown", - "key": "o", - "code": "KeyO", - "keyCode": 229, - "isComposing": true - }, - { - "t": "compositionupdate", - "data": "日本語" - }, - { - "t": "input", - "data": "日本語", - "inputType": "insertCompositionText", - "value": "日本語" - }, - { - "t": "keydown", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 229, - "isComposing": true, - "meta": true - }, - { - "t": "keyup", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 37, - "isComposing": true, - "meta": true - }, - { - "t": "keydown", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 229, - "isComposing": true, - "alt": true - }, - { - "t": "keyup", - "key": "ArrowLeft", - "code": "ArrowLeft", - "keyCode": 37, - "isComposing": true, - "alt": true - }, - { - "t": "keydown", - "key": "Backspace", - "code": "Backspace", - "keyCode": 229, - "isComposing": true, - "meta": true - }, - { - "t": "keyup", - "key": "Backspace", - "code": "Backspace", - "keyCode": 8, - "isComposing": true, - "meta": true - }, - { - "t": "compositionend", - "data": "日本語" - } + name: 'Japanese, a bare arrow and four chords across one live preedit', + expectCalls: ['\x01', '\x1bb', '\x15', '\x1b\x7f'], + expectEmitted: ['日本語\x01\x1bb\x15\x1b\x7f'], + rows: [ + { t: 'keydown', key: 'n', code: 'KeyN', keyCode: 229 }, + { t: 'compositionstart', data: '' }, + { t: 'compositionupdate', data: 'n' }, + { t: 'input', data: 'n', inputType: 'insertCompositionText', value: 'n' }, + { t: 'keydown', key: 'i', code: 'KeyI', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: 'に' }, + { t: 'input', data: 'に', inputType: 'insertCompositionText', value: 'に' }, + { t: 'keyup', key: 'n', code: 'KeyN', keyCode: 78, isComposing: true }, + { t: 'keyup', key: 'i', code: 'KeyI', keyCode: 73, isComposing: true }, + { t: 'keydown', key: 'h', code: 'KeyH', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: 'にh' }, + { t: 'input', data: 'にh', inputType: 'insertCompositionText', value: 'にh' }, + { t: 'keydown', key: 'o', code: 'KeyO', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: 'にほ' }, + { t: 'input', data: 'にほ', inputType: 'insertCompositionText', value: 'にほ' }, + { t: 'keyup', key: 'h', code: 'KeyH', keyCode: 72, isComposing: true }, + { t: 'keyup', key: 'o', code: 'KeyO', keyCode: 79, isComposing: true }, + { t: 'keydown', key: 'n', code: 'KeyN', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: 'にほn' }, + { t: 'input', data: 'にほn', inputType: 'insertCompositionText', value: 'にほn' }, + { t: 'keyup', key: 'n', code: 'KeyN', keyCode: 78, isComposing: true }, + { t: 'keydown', key: 'g', code: 'KeyG', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: 'にほんg' }, + { t: 'input', data: 'にほんg', inputType: 'insertCompositionText', value: 'にほんg' }, + { t: 'keydown', key: 'o', code: 'KeyO', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: '日本語' }, + { t: 'input', data: '日本語', inputType: 'insertCompositionText', value: '日本語' }, + { t: 'keyup', key: 'g', code: 'KeyG', keyCode: 71, isComposing: true }, + { t: 'keyup', key: 'o', code: 'KeyO', keyCode: 79, isComposing: true }, + { t: 'keydown', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 229, isComposing: true }, + { t: 'compositionupdate', data: '日本語' }, + { t: 'input', data: '日本語', inputType: 'insertCompositionText', value: '日本語' }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true }, + { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + meta: true + }, + { + t: 'keyup', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 37, + isComposing: true, + meta: true + }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true }, + { t: 'keydown', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true, alt: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + alt: true + }, + { + t: 'keyup', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 37, + isComposing: true, + alt: true + }, + { t: 'keyup', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true }, + { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, + { + t: 'keydown', + key: 'Backspace', + code: 'Backspace', + keyCode: 229, + isComposing: true, + meta: true + }, + { + t: 'keyup', + key: 'Backspace', + code: 'Backspace', + keyCode: 8, + isComposing: true, + meta: true + }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true }, + { t: 'keydown', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true, alt: true }, + { + t: 'keydown', + key: 'Backspace', + code: 'Backspace', + keyCode: 229, + isComposing: true, + alt: true + }, + { t: 'keyup', key: 'Backspace', code: 'Backspace', keyCode: 8, isComposing: true, alt: true }, + { t: 'keyup', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true }, + { t: 'compositionend', data: '日本語' } ] } ] @@ -713,9 +371,10 @@ function dispatchRow(textarea: HTMLTextAreaElement, row: RecordedRow): void { async function replay(textarea: HTMLTextAreaElement, rows: RecordedRow[]): Promise { for (const row of rows) { dispatchRow(textarea, row) - // A full task between rows, deliberately: the carry is released by the modifier coming up - // rather than by a timer, so no amount of delay between the marked press and the replay - // may change the outcome. Spacing rows more tightly would hide a regression to a timer. + // A full task between rows, deliberately: nothing here may depend on how fast the rows + // arrive. Each event is decided from its own fields, so an arbitrary delay between the + // marked press and the platform's replay must not change the outcome — spacing the rows + // tightly would hide a regression back to a timed carry. await macrotask() } await macrotask() @@ -748,36 +407,56 @@ describe('recorded macOS chord traces during an IME composition', () => { } ) - // A chord remapped onto one of these keys resolves to a pane command rather than bytes. - // The carry has to cover that too, or the replay runs the command a second time — which is - // two panes closed, or two clears, from one press. - it('runs a remapped pane command once across the replay', async () => { + // A chord remapped onto one of these keys resolves to a pane command rather than bytes, so + // the swallowed-chord release has to reach every action, not just the byte path. Recovering + // only bytes would leave a remapped Cmd+Backspace dead during a Japanese preedit. + it('runs a remapped pane command from a swallowed release', async () => { + const rig = openRig({ keybindings: { 'terminal.clear': ['Mod+Backspace'] } }) + await replay(rig.textarea, CASES[3].rows) + + expect(rig.clearPaneCalls).toBe(1) + // The remap wins outright: no kill-line byte alongside it, and the other chords are + // untouched. + expect(rig.inputCalls).toEqual(['\x01', '\x1bb', '\x1b\x7f']) + rig.unmount() + }) + + // The mirror of the above on the input source that does replay: the release sends nothing, + // so the remap must run exactly once, from the replay alone. + it('runs a remapped pane command once when the platform replays instead', async () => { const rig = openRig({ keybindings: { 'terminal.clear': ['Mod+Backspace'] } }) await replay(rig.textarea, CASES[2].rows) expect(rig.clearPaneCalls).toBe(1) - // The remap wins outright: no kill-line byte is sent alongside it. expect(rig.inputCalls).toEqual([]) rig.unmount() }) - // The carry must not outlive its gesture: an ordinary chord pressed later, with no - // composition anywhere near it, still has to reach the shell. - it('releases the carry so a later ordinary chord still sends', async () => { + // The release path reads a live composition, and a rename field or the search input can hold + // one too. Those keystrokes belong to the field, and routing them to the shell would both + // move the wrong cursor and write bytes the user never aimed at the terminal. + it('leaves a swallowed chord alone when the composition is in a text field', async () => { const rig = openRig() - await replay(rig.textarea, CASES[0].rows) - expect(rig.inputCalls).toEqual(CASES[0].expectCalls) + const field = document.createElement('input') + rig.textarea.parentElement?.append(field) - dispatchRow(rig.textarea, { - t: 'keydown', - key: 'ArrowLeft', - code: 'ArrowLeft', - keyCode: 37, - meta: true - }) - await macrotask() + for (const t of ['keydown', 'keyup'] as const) { + const event = new KeyboardEvent(t, { + key: 'ArrowLeft', + code: 'ArrowLeft', + metaKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperties(event, { + isComposing: { value: true }, + keyCode: { value: t === 'keydown' ? 229 : 37 } + }) + field.dispatchEvent(event) + await macrotask() + } - expect(rig.inputCalls).toEqual([...CASES[0].expectCalls, CASES[0].expectCalls[0]]) + expect(rig.inputCalls).toEqual([]) rig.unmount() }) }) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 9bca09ad3139..83d64d189633 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -6,13 +6,9 @@ import type { IDisposable } from '@xterm/xterm' import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager' import type { PtyTransport } from './pty-transport' import { safeFind } from '../terminal-search-safe-find' -import { - isImeExemptTerminalChord, - resolveTerminalShortcutAction -} from './terminal-shortcut-policy' +import { isImeExemptTerminalChord, resolveTerminalShortcutAction } from './terminal-shortcut-policy' import type { MacOptionAsAlt } from './terminal-shortcut-policy' import { createTerminalNativeOnlyShortcutTracker } from './terminal-native-only-shortcut' -import { createTerminalImeChordRedispatchLedger } from './terminal-ime-chord-redispatch' import { installTerminalNativeInputListeners } from './terminal-native-input-listeners' import { keybindingMatchesAction, @@ -48,15 +44,42 @@ import { } from '@/lib/pane-manager/terminal-scroll-intent' /** - * True when a live composition owns the keydown outright. A chord over an arrow or - * Backspace/Delete is excluded: the IME never claims those, and dropping them makes - * Cmd+← do nothing mid-syllable (#12871). xterm queues the resulting bytes behind the - * preedit, so letting them through cannot reorder them ahead of the commit. + * True when a live composition owns the event outright. A modifier chord over an arrow or + * Backspace/Delete is excluded so the terminal pane can resolve one from the key's *release* + * — see `isSwallowedImeChordRelease`. On a keydown the caller must still yield to the IME, + * because at that moment the two cases are indistinguishable (#12871). */ -function isImeOwnedTerminalKeyEvent(event: Parameters[0]): boolean { +function isImeOwnedTerminalKeyEvent( + event: Parameters[0] +): boolean { return isImeOwnedKeyboardEvent(event) && !isImeExemptTerminalChord(event) } +/** + * Whether a chord the IME took on keydown must be sent from this release instead. + * + * Recorded on stock macOS, and the two input sources answer in opposite ways. Both marked + * keydowns are identical (`code='ArrowLeft'`, `keyCode=229`, `isComposing=true`), so nothing + * can be decided when the key goes down. By the time it comes up they have separated: + * + * - Korean 2-Set committed the syllable and ended the composition, then the platform + * replays the chord unmarked. `isComposing` is false at release, and that replay resolves + * on its own — acting here too would send it twice, which for `Option+←` is two words. + * - Japanese conversion swallowed the chord whole: no commit, no replay, and the + * composition is still live at release. Nothing else will ever deliver it. + * + * So a still-composing release means the chord has no other route to the shell. xterm queues + * the bytes behind the preedit and flushes them on commit, so sending them cannot reorder + * them ahead of the text being composed. + * + * TODO: one release is one action, so holding a swallowed chord down repeats nothing. Acting + * on the auto-repeat instead would have to run before the two cases separate, so this waits + * for a trace showing a user holds a chord mid-preedit. + */ +function isSwallowedImeChordRelease(event: KeyboardEvent): boolean { + return isImeOwnedKeyboardEvent(event) && isImeExemptTerminalChord(event) +} + export function resolveTerminalKeyboardShortcutAction( event: Parameters[0], isMac: Parameters[1], @@ -285,7 +308,6 @@ export function useTerminalKeyboardShortcuts({ // location from its own keydown event and clear it on keyup. let optionKeyLocation = 0 const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker() - const imeChordRedispatchLedger = createTerminalImeChordRedispatchLedger() const disposeNativeInputListeners = installTerminalNativeInputListeners( window, nativeOnlyShortcutTracker, @@ -387,14 +409,9 @@ export function useTerminalKeyboardShortcuts({ if (keyboardScope && !keyboardEventBelongsToScope(e, keyboardScope)) { return } - if (isImeOwnedTerminalKeyEvent(e)) { - return - } - // Korean's input source replays the chord unmarked after committing; it was already - // sent from the marked keydown, so let the replay pass without acting on it again. - if (imeChordRedispatchLedger.isRedispatchOfSentChord(e)) { - e.preventDefault() - e.stopImmediatePropagation() + // Every IME-owned keydown yields, chord or not. A chord the IME then swallows is + // recovered from its release; one it merely delays arrives as an unmarked replay. + if (isImeOwnedKeyboardEvent(e)) { return } // Why: replace stale state only for this physical key so rollover cannot @@ -444,13 +461,16 @@ export function useTerminalKeyboardShortcuts({ if (!action) { return } - // Claim before acting, and for every action rather than the byte path alone: a chord - // remapped onto one of these keys resolves to a pane command, and the replay would run - // it a second time — two panes closed from one press. - if (isImeOwnedKeyboardEvent(e)) { - imeChordRedispatchLedger.claimSentChord(e) - } + runShortcutAction(e, action, manager) + } + // Shared by the keydown path and the swallowed-chord release below, so a chord recovered + // from a release runs the same action a press would — including remaps onto pane commands. + function runShortcutAction( + e: KeyboardEvent, + action: NonNullable>, + manager: PaneManager + ): void { if (action.type === 'switchInputSource') { // Why: the OS must receive its default action, while xterm must receive // none of the keydown, keypress, or keyup sequence. @@ -666,27 +686,36 @@ export function useTerminalKeyboardShortcuts({ } } - const onImeChordKeyUp = (e: KeyboardEvent): void => { - // Scoped like onKeyDown: another pane's release must not retire this pane's carry. + const onSwallowedImeChordRelease = (e: KeyboardEvent): void => { + const manager = managerRef.current + if (!manager) { + return + } const keyboardScope = keyboardScopeRef.current if (keyboardScope && !keyboardEventBelongsToScope(e, keyboardScope)) { return } - imeChordRedispatchLedger.onKeyUp(e) - } - const onImeChordBlur = (): void => { - imeChordRedispatchLedger.reset() + if (!isSwallowedImeChordRelease(e)) { + return + } + if (isEditableTarget(e.target)) { + return + } + const action = resolveShortcutEvent(e) + // A native-only chord arms from its press so the OS still sees the gesture; arming it + // from a release would leave the tracker holding a key that is already up. + if (!action || action.type === 'switchInputSource') { + return + } + runShortcutAction(e, action, manager) } window.addEventListener('keydown', onKeyDown, { capture: true }) - window.addEventListener('keyup', onImeChordKeyUp, { capture: true }) - window.addEventListener('blur', onImeChordBlur) + window.addEventListener('keyup', onSwallowedImeChordRelease, { capture: true }) return () => { disposeNativeInputListeners() - imeChordRedispatchLedger.reset() window.removeEventListener('keydown', onKeyDown, { capture: true }) - window.removeEventListener('keyup', onImeChordKeyUp, { capture: true }) - window.removeEventListener('blur', onImeChordBlur) + window.removeEventListener('keyup', onSwallowedImeChordRelease, { capture: true }) } }, [ isActive, diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.test.ts deleted file mode 100644 index 34a0e1f7f7a8..000000000000 --- a/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.test.ts +++ /dev/null @@ -1,77 +0,0 @@ -// @vitest-environment happy-dom -// The recorded traces cover the ledger end to end, but they all hold their modifier through the -// replay and always carry a `code`, so these guards are unreachable from them. Pinned here -// because deleting either would leave every recorded case green while a bare press or a -// codeless event started being swallowed. -import { describe, expect, it } from 'vitest' -import { - createTerminalImeChordRedispatchLedger, - type ImeChordRedispatchEvent -} from './terminal-ime-chord-redispatch' - -function event(overrides: Partial): ImeChordRedispatchEvent { - return { - code: 'ArrowLeft', - keyCode: 37, - isComposing: false, - metaKey: false, - ctrlKey: false, - altKey: false, - ...overrides - } -} - -const MARKED_CHORD = event({ keyCode: 229, isComposing: true, metaKey: true }) - -describe('ime chord redispatch ledger', () => { - it('retires the unmarked replay of a chord it sent', () => { - const ledger = createTerminalImeChordRedispatchLedger() - ledger.claimSentChord(MARKED_CHORD) - - expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(true) - // Spent: a second press of the same chord is the user's own, not a replay. - expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(false) - }) - - it.each([ - ['a different physical key', event({ code: 'ArrowRight', metaKey: true })], - ['a bare press with no modifier', event({})], - ['a still-marked event, which is the IME’s own', event({ keyCode: 229, isComposing: true, metaKey: true })] - ])('does not retire %s', (_label, replay) => { - const ledger = createTerminalImeChordRedispatchLedger() - ledger.claimSentChord(MARKED_CHORD) - - expect(ledger.isRedispatchOfSentChord(replay)).toBe(false) - }) - - it('claims nothing from an event with no physical code', () => { - const ledger = createTerminalImeChordRedispatchLedger() - ledger.claimSentChord(event({ code: undefined, keyCode: 229, isComposing: true, metaKey: true })) - - expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(false) - }) - - // macOS delivers the key's own keyup before the replay, with the modifier still held, so - // that release must not end the gesture. Releasing the modifier must. - it('survives the chord key coming up and ends when the modifier does', () => { - const ledger = createTerminalImeChordRedispatchLedger() - ledger.claimSentChord(MARKED_CHORD) - - ledger.onKeyUp(event({ metaKey: true })) - expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(true) - - ledger.claimSentChord(MARKED_CHORD) - ledger.onKeyUp(event({ code: 'MetaLeft', keyCode: 91 })) - expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(false) - }) - - // A gesture interrupted by focus loss never releases its modifier, so without this the - // carry would sit armed and eat an ordinary chord later. - it('drops the carry on reset', () => { - const ledger = createTerminalImeChordRedispatchLedger() - ledger.claimSentChord(MARKED_CHORD) - ledger.reset() - - expect(ledger.isRedispatchOfSentChord(event({ metaKey: true }))).toBe(false) - }) -}) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts b/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts deleted file mode 100644 index 75199ee790be..000000000000 --- a/src/renderer/src/components/terminal-pane/terminal-ime-chord-redispatch.ts +++ /dev/null @@ -1,77 +0,0 @@ -import { isImeOwnedKeyboardEvent } from '@/lib/ime-composition-keyboard-event' - -export type ImeChordRedispatchEvent = { - code?: string - keyCode?: number - isComposing?: boolean - metaKey: boolean - ctrlKey: boolean - altKey: boolean -} - -export type TerminalImeChordRedispatchLedger = { - /** Record that a chord was acted on from its IME-marked keydown. */ - claimSentChord: (event: ImeChordRedispatchEvent) => void - /** True when this keydown is the platform's replay of a chord already acted on. */ - isRedispatchOfSentChord: (event: ImeChordRedispatchEvent) => boolean - onKeyUp: (event: ImeChordRedispatchEvent) => void - reset: () => void -} - -function hasChordModifier(event: ImeChordRedispatchEvent): boolean { - return event.metaKey || event.ctrlKey || event.altKey -} - -/** - * Why: whether a modifier chord pressed mid-composition is replayed depends on the input - * source, and the two cases are indistinguishable at the marked keydown. Recorded on stock - * macOS, both `code='ArrowLeft'` `keyCode=229` `isComposing=true`: - * - * - Korean 2-Set commits the syllable, emits `compositionend`, and the platform replays the - * chord unmarked (`keyCode=37`, `isComposing=false`) after keyup. - * - Japanese conversion swallows it: no `compositionend`, no replay, `isComposing` still - * true at keyup, and the chord never reaches the shell at all. - * - * Acting on the marked keydown covers Japanese; retiring the replay keeps Korean at one - * firing instead of two, which for `Option+←` is the difference between one word and two. - * - * The carry is released when the last chord modifier comes up, not on a timer. Both traces - * hold the modifier across the whole gesture and release it after the replay, so that edge is - * the gesture's own boundary — where an animation-frame expiry would race any slow task - * landing between keyup and the replay and let the chord through twice. - */ -export function createTerminalImeChordRedispatchLedger(): TerminalImeChordRedispatchLedger { - let pendingCode: string | null = null - - return { - claimSentChord: (event) => { - if (!event.code) { - return - } - pendingCode = event.code - }, - isRedispatchOfSentChord: (event) => { - if (pendingCode === null || event.code !== pendingCode) { - return false - } - // The replay is the unmarked copy of the same physical chord; a still-marked event is - // the IME's own, and a bare press is the user reaching the shell on purpose. - if (isImeOwnedKeyboardEvent(event) || !hasChordModifier(event)) { - return false - } - pendingCode = null - return true - }, - onKeyUp: (event) => { - if (!hasChordModifier(event)) { - pendingCode = null - } - }, - // Why: a gesture interrupted by focus loss never releases its modifier, and a carry left - // armed would swallow an ordinary chord later. Both siblings in this effect drop their - // state on blur for the same reason. - reset: () => { - pendingCode = null - } - } -} diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts index d5e7df3d7f7b..ed6f8c48edb7 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -22,9 +22,10 @@ export type MacOptionAsAlt = 'true' | 'false' | 'left' | 'right' // Why: a live composition owns every keydown, but a Japanese conversion swallows a modifier // chord over one of these keys outright — no commit, no platform replay, nothing reaching the -// shell (#12871). Exempting them costs a double firing on input sources that DO replay, which -// terminal-ime-chord-redispatch.ts retires. Read `code`, not `key`: Windows reports an -// IME-consumed key as `key: 'Process'` (#12171, #13033). +// shell (#12871). Exempting them lets the terminal pane recover the chord from the key's +// release, which is the first moment a swallowed chord is distinguishable from a delayed one +// (keyboard-handlers.ts, `isSwallowedImeChordRelease`). Read `code`, not `key`: Windows +// reports an IME-consumed key as `key: 'Process'` (#12171, #13033). // // ArrowUp/ArrowDown are left out because no trace covers them, not because the reasoning // stops there — Cmd+↑/↓ scrolls the viewport and writes no bytes, so it is the safer half. @@ -40,14 +41,13 @@ function physicalChordKey(event: TerminalShortcutEvent): string { } /** - * True when an IME-owned keydown is nonetheless an Orca terminal chord. A lone modifier - * is excluded, so a mode-switch gesture such as composing Ctrl+Space never reaches the - * shortcut policy, and so is Shift, which Japanese conversion binds to resize a segment. + * True when an IME-owned event is nonetheless an Orca terminal chord. A lone modifier is + * excluded, so a mode-switch gesture such as composing Ctrl+Space never reaches the shortcut + * policy, and so is Shift, which Japanese conversion binds to resize a segment. */ export function isImeExemptTerminalChord(event: TerminalShortcutEvent): boolean { return ( - IME_EXEMPT_CHORD_CODES.has(event.code ?? '') && - (event.metaKey || event.ctrlKey || event.altKey) + IME_EXEMPT_CHORD_CODES.has(event.code ?? '') && (event.metaKey || event.ctrlKey || event.altKey) ) } @@ -149,8 +149,22 @@ export function resolveTerminalShortcutAction( // macOS has no non-Latin physical fallback (shared/keybindings.ts). Without substituting the // physical key a user's remap onto Cmd+Backspace would miss and fall through to the built-in // kill-line byte — silence before the IME exemption, the wrong action after it. + // + // Field by field rather than a spread: a real KeyboardEvent keeps `key`/`metaKey`/... as + // prototype accessors, so `{ ...event }` yields an empty object and every lookup below + // silently misses. Test doubles are plain objects and hide that. const chordEvent: TerminalShortcutEvent = - chordKey === event.key ? event : { ...event, key: chordKey } + chordKey === event.key + ? event + : { + key: chordKey, + code: event.code, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, + altKey: event.altKey, + shiftKey: event.shiftKey, + repeat: event.repeat + } // Why: capture this chord even on repeat without blocking the OS default input-source switch. if (keybindingMatchesAction('terminal.switchInputSource', chordEvent, platform, keybindings)) { From 3543cff63c5be419476b37d3160a9ba57bc96d65 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 8 Aug 2026 20:55:41 +0900 Subject: [PATCH 06/15] test(terminal): pin the IME chord branches the macOS captures miss Review found four gaps, none of which the recorded traces can reach. A browser keeps KeyboardEvent's fields on the prototype and happy-dom keeps them as own properties, so neither a plain-object fixture nor a real event under this runner can see a copy that enumerates them. Modelling the browser's shape explicitly does, and fails on the old spread exactly as production did. Windows rewrites `key` to 'Process' and macOS does not, so no capture reached the physical-code substitution through the real handler. Escape mid-preedit was likewise uncovered: the chord still reaches the shell and only the composed text is dropped, which is now pinned rather than left to be discovered in a shell. The release path also skipped the floating-panel close guard the keydown path applies, so a chord remapped onto tab.close diverged between them. Recorded cases are selected by name rather than array index, and Cmd+Up/Down reads the same resolved key as its siblings instead of `event.key`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Mu6j33LNsd4Mvv8a1NuFoo --- .../src/components/terminal-pane/AGENTS.md | 2 +- ...ndlers.issue-12871-composing-chord.test.ts | 39 ++++++++ ....issue-12871-recorded-chord-traces.test.ts | 95 ++++++++++++++++++- .../terminal-pane/keyboard-handlers.ts | 12 +++ .../terminal-pane/terminal-shortcut-policy.ts | 6 +- 5 files changed, 149 insertions(+), 5 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/AGENTS.md b/src/renderer/src/components/terminal-pane/AGENTS.md index a96b26d74766..a4ec0da9dcef 100644 --- a/src/renderer/src/components/terminal-pane/AGENTS.md +++ b/src/renderer/src/components/terminal-pane/AGENTS.md @@ -36,6 +36,6 @@ Each rule below was paid for by a shipped defect. The parenthetical is the issue event shape. Real IMEs produce sequences nobody guesses correctly; see `tests/e2e/terminal-ime-observed-event-sequences.ts`. - **A paired negative** proving ordinary non-IME input is unchanged. It must be a genuine control: - an arm that dispatches `compositionupdate` is a Latin *preedit*, not ordinary input. + an arm that dispatches `compositionupdate` is a Latin _preedit_, not ordinary input. - **A mutation check.** Removing or bypassing the fix must make the positive fail while the negative passes. A test that survives deleting the code it guards is guarding nothing. diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts index fec5ff012701..61cf04d6b858 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts @@ -74,6 +74,45 @@ describe('terminal chords stay live during an IME composition', () => { }) }) + // Chromium defines KeyboardEvent's fields as accessors on the prototype, not as own + // properties, so anything that copies an event by enumerating it gets nothing. happy-dom + // does the opposite, which means a plain-object fixture cannot see that class of bug and a + // real `new KeyboardEvent(...)` cannot either under this runner. Modelling the browser's + // shape explicitly is the only form of it that runs here. + it('resolves an event whose fields live on the prototype, as in a browser', () => { + const fields = keyEvent({ + key: 'Process', + code: 'Backspace', + metaKey: true, + isComposing: true, + keyCode: 229 + }) + const prototype = Object.create( + null, + Object.fromEntries( + Object.entries(fields).map(([name, value]) => [name, { get: () => value }]) + ) + ) as ComposingKeyEvent + const event = Object.create(prototype) as ComposingKeyEvent + expect(Object.keys(event)).toEqual([]) + + expect( + resolveTerminalKeyboardShortcutAction( + event, + true, + 'false', + 0, + false, + { 'terminal.clear': ['Mod+Backspace'] }, + undefined, + undefined, + undefined, + undefined, + () => false + ) + ).toEqual({ type: 'clearActivePane' }) + }) + // A CJK input source rewrites `key` while `code` keeps the physical key, which is what // #12171 and #13033 turned on. Matching `key` here would drop the chord again. it('matches the physical code when the input source has rewritten key', () => { diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index 078c1cca61f9..6247d59a3f36 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -381,6 +381,18 @@ async function replay(textarea: HTMLTextAreaElement, rows: RecordedRow[]): Promi await macrotask() } +const JAPANESE_CASE = 'Japanese, a bare arrow and four chords across one live preedit' + +// By name, not by index: each test below needs a particular gesture, and inserting a case +// would otherwise silently repoint them at the wrong trace while still passing. +function caseNamed(name: string): RecordedCase { + const found = CASES.find((testCase) => testCase.name === name) + if (!found) { + throw new Error(`recorded case not found: ${name}`) + } + return found +} + describe('recorded macOS chord traces during an IME composition', () => { beforeEach(() => { vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({ @@ -412,7 +424,7 @@ describe('recorded macOS chord traces during an IME composition', () => { // only bytes would leave a remapped Cmd+Backspace dead during a Japanese preedit. it('runs a remapped pane command from a swallowed release', async () => { const rig = openRig({ keybindings: { 'terminal.clear': ['Mod+Backspace'] } }) - await replay(rig.textarea, CASES[3].rows) + await replay(rig.textarea, caseNamed(JAPANESE_CASE).rows) expect(rig.clearPaneCalls).toBe(1) // The remap wins outright: no kill-line byte alongside it, and the other chords are @@ -425,7 +437,7 @@ describe('recorded macOS chord traces during an IME composition', () => { // so the remap must run exactly once, from the replay alone. it('runs a remapped pane command once when the platform replays instead', async () => { const rig = openRig({ keybindings: { 'terminal.clear': ['Mod+Backspace'] } }) - await replay(rig.textarea, CASES[2].rows) + await replay(rig.textarea, caseNamed('Korean 2-Set, Cmd+Backspace').rows) expect(rig.clearPaneCalls).toBe(1) expect(rig.inputCalls).toEqual([]) @@ -460,3 +472,82 @@ describe('recorded macOS chord traces during an IME composition', () => { rig.unmount() }) }) + +// Constructed, not recorded. Both cases below need a shape the two captures happen not to +// contain, and the file above is kept to captured rows only so its fidelity claim stays true. +describe('constructed shapes the macOS captures do not contain', () => { + beforeEach(() => { + vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({ + measureText: () => ({ width: 10 }) + } as unknown as CanvasRenderingContext2D) + vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue(MAC_USER_AGENT) + }) + + afterEach(() => { + cleanup() + vi.restoreAllMocks() + document.body.replaceChildren() + }) + + // The decision is the release's alone. Pinned directly rather than left to be inferred from + // the call counts above, because `resolveTerminalKeyboardShortcutAction` does resolve a + // composing exempt chord — it is the pane that declines to act on it until the key comes up. + it('does nothing while the chord is still held down', async () => { + const rig = openRig() + const japanese = caseNamed(JAPANESE_CASE) + const throughFirstChordPress = japanese.rows.slice( + 0, + japanese.rows.findIndex( + (row) => row.keyCode === 229 && row.code === 'ArrowLeft' && row.meta + ) + 1 + ) + await replay(rig.textarea, throughFirstChordPress) + + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + + // Windows reports an IME-consumed key as `key: 'Process'` (#12171), which normalizes to no + // token at all, so the keybinding lookup has to run on the physical `code` instead. macOS + // leaves `key` alone, so no capture reaches that substitution — without this the branch is + // only ever exercised by plain-object fixtures, never by a real event through the handler. + it('honours a remap when the input source has rewritten key to Process', async () => { + const rig = openRig({ keybindings: { 'terminal.clear': ['Mod+Backspace'] } }) + const japanese = caseNamed(JAPANESE_CASE) + const rows = japanese.rows.map((row) => + row.code === 'Backspace' ? { ...row, key: 'Process' } : row + ) + await replay(rig.textarea, rows) + + // Without the physical-code substitution this falls through to the built-in kill-line + // byte: the wrong action, silently, for anyone who remapped the chord. + expect(rig.clearPaneCalls).toBe(1) + expect(rig.inputCalls).toEqual(['\x01', '\x1bb', '\x1b\x7f']) + rig.unmount() + }) + + // Escape during a preedit ends the composition with an empty commit. xterm flushes whatever + // the chord queued at that point, so this pins where those bytes go rather than leaving it + // to be discovered in a shell. + it('still delivers a chord when the composition is cancelled instead of committed', async () => { + const rig = openRig() + const japanese = caseNamed(JAPANESE_CASE) + const upToFirstChord = japanese.rows.slice( + 0, + japanese.rows.findIndex((row) => row.t === 'keyup' && row.code === 'MetaLeft') + 1 + ) + // Escape clears the preedit out of the helper textarea before the composition ends; xterm + // reads the commit from that textarea, so omitting this row would commit 日本語 after all. + await replay(rig.textarea, [ + ...upToFirstChord, + { t: 'input', inputType: 'deleteCompositionText', value: '' }, + { t: 'compositionend', data: '' } + ]) + + // The chord was aimed at the shell's line, not at the preedit, so cancelling the text does + // not retract it. Only the composed characters are dropped. + expect(rig.inputCalls).toEqual(['\x01']) + expect(rig.emitted).toEqual(['\x01']) + rig.unmount() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 83d64d189633..c310fb099017 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -80,6 +80,13 @@ function isSwallowedImeChordRelease(event: KeyboardEvent): boolean { return isImeOwnedKeyboardEvent(event) && isImeExemptTerminalChord(event) } +/** + * Note the asymmetry with the pane's own keydown handler, which yields on *every* IME-owned + * event. This resolver deliberately lets an exempt chord through, because the pane also calls + * it for such a chord's release, where resolving is the whole point. A composing exempt chord + * therefore resolves here while producing nothing from a keydown — asserting a keydown outcome + * against this function alone would pin something the pane does not do. + */ export function resolveTerminalKeyboardShortcutAction( event: Parameters[0], isMac: Parameters[1], @@ -701,6 +708,11 @@ export function useTerminalKeyboardShortcuts({ if (isEditableTarget(e.target)) { return } + // Same precedence as the keydown path: a chord remapped onto tab.close closes an empty + // floating panel there, and skipping it here would close the pane instead. + if (handleEmptyFloatingWorkspacePanelCloseShortcut(e, shortcutPlatform, keybindings)) { + return + } const action = resolveShortcutEvent(e) // A native-only chord arms from its press so the OS still sees the gesture; arming it // from a release would leave the tracker holding a key that is already up. diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts index ed6f8c48edb7..b43200aa68df 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -287,10 +287,12 @@ export function resolveTerminalShortcutAction( return { type: 'sendInput', data: '\x05' } } // Why: macOS users expect Cmd+↑/↓ to scroll scrollback, not write escape bytes to the shell. - if (event.key === 'ArrowUp') { + // `chordKey` like its siblings, though it resolves to `key` today: ArrowUp/ArrowDown are not + // exempt codes, and reading `event.key` here would silently opt out if they ever became so. + if (chordKey === 'ArrowUp') { return { type: 'scrollViewport', position: 'top' } } - if (event.key === 'ArrowDown') { + if (chordKey === 'ArrowDown') { return { type: 'scrollViewport', position: 'bottom' } } } From 81cd563122b290dffecd1921ac2d1eefa35019d6 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:08:51 +0900 Subject: [PATCH 07/15] fix(terminal): answer a swallowed chord from its press, not its release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the release's own fields are the wrong ones to read, in both directions, and both reproduce. A KeyboardEvent's modifier flags describe the moment it fired, so letting Cmd up before the arrow leaves the arrow's keyup with metaKey: false. The exemption then misses and the chord is lost outright — the bug this change exists to fix, surviving in whichever order the user happens to let go. The mirror case: a key that went down with no composition in sight has already resolved from its own keydown, but a composition starting while it is held makes its release look swallowed and sends it a second time. Measured as \x1bb twice, which is two words instead of one. So keep the chord from the keydown that yielded, and let the release answer for that press: same key, composition still live, modifiers as they were when it went down. Nothing is swallowed and the carry cannot outlive its key, since an ordinary press of the same code drops it. The release also now runs the file-search and floating-panel guards the keydown path applies, matching on the press while suppressing on the real event, and Shift is excluded from the exemption as its comment always claimed — Japanese conversion binds Shift+arrow to resize a segment. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Mu6j33LNsd4Mvv8a1NuFoo --- ....issue-12871-recorded-chord-traces.test.ts | 45 +++++++++++ .../terminal-pane/keyboard-handlers.ts | 78 ++++++++++++++++--- .../terminal-pane/terminal-shortcut-policy.ts | 4 +- 3 files changed, 117 insertions(+), 10 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index 6247d59a3f36..98ccd271a050 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -507,6 +507,51 @@ describe('constructed shapes the macOS captures do not contain', () => { rig.unmount() }) + // A KeyboardEvent's modifier flags describe the moment it fired, so letting Cmd up before + // the arrow leaves the arrow's keyup with metaKey: false. Reading the release's own flags + // drops the chord outright, which is the bug this whole change exists to fix. + it('sends the chord when the modifier is released before the key', async () => { + const rig = openRig() + const japanese = caseNamed(JAPANESE_CASE) + const beforeFirstChord = japanese.rows.slice( + 0, + japanese.rows.findIndex((row) => row.t === 'keydown' && row.code === 'MetaLeft') + ) + await replay(rig.textarea, [ + ...beforeFirstChord, + { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + meta: true + }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true } + ]) + + expect(rig.inputCalls).toEqual(['\x01']) + rig.unmount() + }) + + // The mirror hazard: the key went down with no composition in sight, so it already resolved + // from its own keydown. A composition starting while it is held must not make the release + // send it a second time — for Option+Left that is two words instead of one. + it('does not send twice when a composition starts while the key is held', async () => { + const rig = openRig() + await replay(rig.textarea, [ + { t: 'keydown', key: 'Alt', code: 'AltLeft', keyCode: 18, alt: true }, + { t: 'keydown', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, alt: true }, + { t: 'compositionstart', data: '' }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true, alt: true } + ]) + + expect(rig.inputCalls).toEqual(['\x1bb']) + rig.unmount() + }) + // Windows reports an IME-consumed key as `key: 'Process'` (#12171), which normalizes to no // token at all, so the keybinding lookup has to run on the physical `code` instead. macOS // leaves `key` alone, so no capture reaches that substitution — without this the branch is diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index c310fb099017..556457dc8269 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -55,8 +55,17 @@ function isImeOwnedTerminalKeyEvent( return isImeOwnedKeyboardEvent(event) && !isImeExemptTerminalChord(event) } +// `code` and `repeat` are optional on the policy's event type but always present here, and +// narrowing them keeps the snapshot usable by the shortcut matchers without a cast. +type PendingImeChord = Parameters[0] & + Required[0], 'code' | 'repeat'>> & { + isComposing: boolean + keyCode: number + } + /** - * Whether a chord the IME took on keydown must be sent from this release instead. + * A chord the IME took on keydown, kept until the key comes up so the release can answer for + * the press rather than for whatever the keyboard looks like by then. * * Recorded on stock macOS, and the two input sources answer in opposite ways. Both marked * keydowns are identical (`code='ArrowLeft'`, `keyCode=229`, `isComposing=true`), so nothing @@ -64,20 +73,35 @@ function isImeOwnedTerminalKeyEvent( * * - Korean 2-Set committed the syllable and ended the composition, then the platform * replays the chord unmarked. `isComposing` is false at release, and that replay resolves - * on its own — acting here too would send it twice, which for `Option+←` is two words. + * on its own — acting there too would send it twice, which for `Option+←` is two words. * - Japanese conversion swallowed the chord whole: no commit, no replay, and the * composition is still live at release. Nothing else will ever deliver it. * * So a still-composing release means the chord has no other route to the shell. xterm queues - * the bytes behind the preedit and flushes them on commit, so sending them cannot reorder - * them ahead of the text being composed. + * the bytes behind the preedit and flushes them on commit, so sending them cannot reorder them + * ahead of the text being composed. + * + * The snapshot is what makes reading the release safe. A `KeyboardEvent`'s modifier flags + * describe the moment it fired, and releasing `Cmd` before `←` leaves the arrow's keyup with + * `metaKey: false` — matching on that loses the chord entirely. * * TODO: one release is one action, so holding a swallowed chord down repeats nothing. Acting * on the auto-repeat instead would have to run before the two cases separate, so this waits * for a trace showing a user holds a chord mid-preedit. */ -function isSwallowedImeChordRelease(event: KeyboardEvent): boolean { - return isImeOwnedKeyboardEvent(event) && isImeExemptTerminalChord(event) +function imeChordSnapshot(event: KeyboardEvent): PendingImeChord { + return { + key: event.key, + code: event.code, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, + altKey: event.altKey, + shiftKey: event.shiftKey, + // One release is one action, so the auto-repeat presses behind it are not repeats of it. + repeat: false, + isComposing: true, + keyCode: event.keyCode + } } /** @@ -314,6 +338,7 @@ export function useTerminalKeyboardShortcuts({ // held. To distinguish left vs right Option, we record the Option key's // location from its own keydown event and clear it on keyup. let optionKeyLocation = 0 + let pendingImeChord: PendingImeChord | null = null const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker() const disposeNativeInputListeners = installTerminalNativeInputListeners( window, @@ -419,8 +444,17 @@ export function useTerminalKeyboardShortcuts({ // Every IME-owned keydown yields, chord or not. A chord the IME then swallows is // recovered from its release; one it merely delays arrives as an unmarked replay. if (isImeOwnedKeyboardEvent(e)) { + if (isImeExemptTerminalChord(e)) { + pendingImeChord = imeChordSnapshot(e) + } return } + // This key went down outside a composition, so whatever it does it does from here. Any + // carry for it is stale — a composition that starts while it is held must not turn its + // release into a second firing. Scoped to this key so rollover leaves others alone. + if (pendingImeChord?.code === e.code) { + pendingImeChord = null + } // Why: replace stale state only for this physical key so rollover cannot // disarm a still-held native-only chord before its Kitty keyup arrives. nativeOnlyShortcutTracker.prepareKeyDown(e) @@ -702,18 +736,44 @@ export function useTerminalKeyboardShortcuts({ if (keyboardScope && !keyboardEventBelongsToScope(e, keyboardScope)) { return } - if (!isSwallowedImeChordRelease(e)) { + const chord = pendingImeChord + if (!chord || chord.code !== e.code) { + return + } + // Spent either way: whichever branch below answers, this press is now accounted for. + pendingImeChord = null + // The composition ended while the key was down, so the IME took the chord as its cue to + // commit rather than eating it, and the platform's unmarked replay is on its way. + if (!isImeOwnedKeyboardEvent(e)) { return } if (isEditableTarget(e.target)) { return } + // Matched on the press, suppressed on the release: `chord` carries the modifiers as they + // were when the key went down, while only the real event can consume the keystroke. + const pressed = { + ...chord, + preventDefault: () => e.preventDefault(), + stopPropagation: () => e.stopPropagation(), + stopImmediatePropagation: () => e.stopImmediatePropagation() + } // Same precedence as the keydown path: a chord remapped onto tab.close closes an empty // floating panel there, and skipping it here would close the pane instead. - if (handleEmptyFloatingWorkspacePanelCloseShortcut(e, shortcutPlatform, keybindings)) { + if (handleEmptyFloatingWorkspacePanelCloseShortcut(pressed, shortcutPlatform, keybindings)) { return } - const action = resolveShortcutEvent(e) + if (matchFileSearchShortcut(chord, shortcutPlatform, keybindings, terminalShortcutPolicy)) { + const pane = manager.getActivePane() ?? manager.getPanes()[0] + const selectedText = normalizeSelectedTextForFileSearch(pane?.terminal.getSelection()) + if (selectedText) { + e.preventDefault() + e.stopImmediatePropagation() + onSearchSelectedText(selectedText) + return + } + } + const action = resolveShortcutEvent(chord) // A native-only chord arms from its press so the OS still sees the gesture; arming it // from a release would leave the tracker holding a key that is already up. if (!action || action.type === 'switchInputSource') { diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts index b43200aa68df..ab6de449b422 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -47,7 +47,9 @@ function physicalChordKey(event: TerminalShortcutEvent): string { */ export function isImeExemptTerminalChord(event: TerminalShortcutEvent): boolean { return ( - IME_EXEMPT_CHORD_CODES.has(event.code ?? '') && (event.metaKey || event.ctrlKey || event.altKey) + IME_EXEMPT_CHORD_CODES.has(event.code ?? '') && + !event.shiftKey && + (event.metaKey || event.ctrlKey || event.altKey) ) } From fd2df4517e033eea5aa28543ff8012e25747b60a Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:38:27 +0900 Subject: [PATCH 08/15] fix(terminal): release the remembered chord when its keyup never comes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A chord interrupted by Cmd+Tab or Spotlight delivers no keyup here, and nothing dropped what it left behind. A later bare arrow moving the preedit caret then matched that carry by code and sent the missing chord's bytes to a shell the user never aimed at — measured as a stray \x1bb. Three closures, matching what the sibling tracker in this effect already does: any press of the same code supersedes the carry whoever owns it, the release spends it before any gate can refuse it, and window blur drops it outright. Also record at imeChordSnapshot why it must stay field by field. The prototype-accessor trap moved here when the policy stopped seeing native events, and this is the only copy of a real KeyboardEvent left. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Mu6j33LNsd4Mvv8a1NuFoo --- ....issue-12871-recorded-chord-traces.test.ts | 37 ++++++++++++++++ .../terminal-pane/keyboard-handlers.ts | 42 +++++++++++++------ 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index 98ccd271a050..32643394e942 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -15,6 +15,13 @@ // Both directions are pinned together, because the failure modes point opposite ways: Korean // must stay at one firing per chord (a second \x1bb jumps two words), Japanese must go from // zero to one. A change that only reads the keydown cannot satisfy both. +// +// Two things this rig cannot speak for. The userAgent mock below only reaches Orca's own check; +// xterm reads `navigator.platform` once at module load and does not see a Mac here, which is +// harmless while Orca intercepts every replayed row but would diverge for anything it stops +// intercepting. And `expectEmitted` groups bytes the way a full task between rows produces — +// real input arrives in one burst and may group differently. Read `expectCalls` for the +// contract; `expectEmitted` is there to show ordering, not framing. import { Terminal } from '@xterm/xterm' import { cleanup, renderHook } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -507,6 +514,36 @@ describe('constructed shapes the macOS captures do not contain', () => { rig.unmount() }) + // A chord interrupted by Cmd+Tab or Spotlight never delivers its release here. The carry it + // left must not answer for a later press of the same key: a bare arrow moving the preedit + // caret would otherwise send the missing chord's bytes to a shell the user never aimed at. + it('does not answer a later press with a carry whose release never arrived', async () => { + const rig = openRig() + await replay(rig.textarea, [ + { t: 'compositionstart', data: '' }, + { t: 'keydown', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true, alt: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + alt: true + } + // Focus leaves the window here: no keyup for either key ever arrives. + ]) + expect(rig.inputCalls).toEqual([]) + + window.dispatchEvent(new Event('blur')) + await replay(rig.textarea, [ + { t: 'keydown', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 229, isComposing: true }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true } + ]) + + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + // A KeyboardEvent's modifier flags describe the moment it fired, so letting Cmd up before // the arrow leaves the arrow's keyup with metaKey: false. Reading the release's own flags // drops the chord outright, which is the bug this whole change exists to fix. diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 556457dc8269..4b92e2884bba 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -90,6 +90,10 @@ type PendingImeChord = Parameters[0] & * for a trace showing a user holds a chord mid-preedit. */ function imeChordSnapshot(event: KeyboardEvent): PendingImeChord { + // Field by field, and it must stay that way: a browser keeps KeyboardEvent's fields as + // prototype accessors, so `{ ...event }` is an empty object and the snapshot loses its + // `code`, which silently disables the whole recovery. happy-dom keeps them as own properties + // and would go on passing, so this is the only warning that survives in the source. return { key: event.key, code: event.code, @@ -441,6 +445,14 @@ export function useTerminalKeyboardShortcuts({ if (keyboardScope && !keyboardEventBelongsToScope(e, keyboardScope)) { return } + // Any press of this key ends whatever the last one carried, whoever owns this one. A key + // pressed outside a composition resolves from here, so a composition starting while it is + // held must not turn its release into a second firing; and a carry whose release never + // arrived — focus left the window mid-chord — must not answer for an unrelated later + // press of the same key. Scoped to this code so rollover leaves other keys alone. + if (pendingImeChord?.code === e.code) { + pendingImeChord = null + } // Every IME-owned keydown yields, chord or not. A chord the IME then swallows is // recovered from its release; one it merely delays arrives as an unmarked replay. if (isImeOwnedKeyboardEvent(e)) { @@ -449,12 +461,6 @@ export function useTerminalKeyboardShortcuts({ } return } - // This key went down outside a composition, so whatever it does it does from here. Any - // carry for it is stale — a composition that starts while it is held must not turn its - // release into a second firing. Scoped to this key so rollover leaves others alone. - if (pendingImeChord?.code === e.code) { - pendingImeChord = null - } // Why: replace stale state only for this physical key so rollover cannot // disarm a still-held native-only chord before its Kitty keyup arrives. nativeOnlyShortcutTracker.prepareKeyDown(e) @@ -728,6 +734,14 @@ export function useTerminalKeyboardShortcuts({ } const onSwallowedImeChordRelease = (e: KeyboardEvent): void => { + const chord = pendingImeChord + if (!chord || chord.code !== e.code) { + return + } + // Spent before any gate below can refuse: the key is up, so the press it carried is over + // however this release is routed. Leaving it armed past a gate is what lets a carry + // outlive its gesture and answer for someone else's press. + pendingImeChord = null const manager = managerRef.current if (!manager) { return @@ -736,12 +750,6 @@ export function useTerminalKeyboardShortcuts({ if (keyboardScope && !keyboardEventBelongsToScope(e, keyboardScope)) { return } - const chord = pendingImeChord - if (!chord || chord.code !== e.code) { - return - } - // Spent either way: whichever branch below answers, this press is now accounted for. - pendingImeChord = null // The composition ended while the key was down, so the IME took the chord as its cue to // commit rather than eating it, and the platform's unmarked replay is on its way. if (!isImeOwnedKeyboardEvent(e)) { @@ -782,12 +790,22 @@ export function useTerminalKeyboardShortcuts({ runShortcutAction(e, action, manager) } + // Why: a chord interrupted by Cmd+Tab or Spotlight never delivers its release here, and a + // carry with no release to spend it sits armed. `installTerminalNativeInputListeners` drops + // the sibling tracker on blur for the same reason. + const onImeChordBlur = (): void => { + pendingImeChord = null + } + window.addEventListener('keydown', onKeyDown, { capture: true }) window.addEventListener('keyup', onSwallowedImeChordRelease, { capture: true }) + window.addEventListener('blur', onImeChordBlur) return () => { disposeNativeInputListeners() + pendingImeChord = null window.removeEventListener('keydown', onKeyDown, { capture: true }) window.removeEventListener('keyup', onSwallowedImeChordRelease, { capture: true }) + window.removeEventListener('blur', onImeChordBlur) } }, [ isActive, From eab431deeb66496bafa15f199f891b1ec2db54ea Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:50:20 +0900 Subject: [PATCH 09/15] refactor(keybindings): fall back to the physical code for 'Process' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the chord's keybinding lookup was reimplementing something the matcher already has one word away. PHYSICAL_CODE_FALLBACK_KEYS holds the keys whose produced value the platform cannot report — '', 'Dead', 'Unidentified' — and 'Process' is exactly that case: Windows' report for a key an IME consumed. Adding it there deletes the synthetic chord event and its fourteen renamed call sites, and fixes the same blind spot on every other surface rather than in the terminal pane alone. `chordKey` stays for the byte fallbacks, which read the key directly. Also from review: the release path's floating-panel and file-search guards now have the tests their comments were asserting without, Shift exclusion has the composing Cmd+Shift+Arrow negative it arrived without, a one-caller wrapper is inlined, and two comments stop pointing at a symbol deleted two commits ago. The idle assertion added earlier is dropped. It was restored on the claim that terminal-shortcut-policy.test.ts leaves `code` empty for these chords; it does not, and for a non-composing event the substitution short-circuits before reading `code` at all, so both fixtures took the same branch. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Mu6j33LNsd4Mvv8a1NuFoo --- ...ndlers.issue-12871-composing-chord.test.ts | 11 ++-- ....issue-12871-recorded-chord-traces.test.ts | 49 +++++++++++++++ .../terminal-pane/keyboard-handlers.ts | 24 ++------ .../terminal-pane/terminal-shortcut-policy.ts | 61 +++++++------------ src/shared/keybindings.ts | 4 +- 5 files changed, 85 insertions(+), 64 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts index 61cf04d6b858..0387820391c8 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-composing-chord.test.ts @@ -63,11 +63,6 @@ describe('terminal chords stay live during an IME composition', () => { '\x1bf' ] ])('resolves %s while a composition is live', (_label, event, expected) => { - // The idle case is asserted too because it is not covered elsewhere with a populated - // `code`: terminal-shortcut-policy.test.ts builds its fixtures with `code: ''`, which - // takes physicalChordKey's fallback branch rather than the production one. - expect(resolveOnMac(event)).toEqual({ type: 'sendInput', data: expected }) - expect(resolveOnMac({ ...event, isComposing: true })).toEqual({ type: 'sendInput', data: expected @@ -133,6 +128,12 @@ describe('terminal chords stay live during an IME composition', () => { ['lone Control', keyEvent({ key: 'Control', code: 'ControlLeft', keyCode: 17, ctrlKey: true })], ['lone Meta', keyEvent({ key: 'Meta', code: 'MetaLeft', keyCode: 91, metaKey: true })], ['bare ArrowLeft', keyEvent({ key: 'ArrowLeft', code: 'ArrowLeft' })], + // Japanese conversion binds Shift+Arrow to resize the segment being converted, so a + // modifier alongside it does not make the chord ours. + [ + 'Cmd+Shift+ArrowLeft', + keyEvent({ key: 'ArrowLeft', code: 'ArrowLeft', metaKey: true, shiftKey: true }) + ], ['Cmd+A', keyEvent({ key: 'a', code: 'KeyA', metaKey: true })] ])('still yields %s to the IME while composing', (_label, event) => { expect(resolveOnMac({ ...event, isComposing: true })).toBeNull() diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index 32643394e942..dc7cb3daa5a6 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -262,6 +262,9 @@ function macrotask(): Promise { type Rig = { textarea: HTMLTextAreaElement + terminal: Terminal + searchSelections: string[] + closePaneCalls: number[] inputCalls: string[] emitted: string[] clearPaneCalls: number @@ -329,8 +332,19 @@ function openRig(overrides: Record = {}): Rig { const hook = renderHook(() => useTerminalKeyboardShortcuts(deps)) return { textarea, + terminal, inputCalls, emitted, + get closePaneCalls() { + return (deps.onRequestClosePane as ReturnType).mock.calls.map( + ([id]) => id as number + ) + }, + get searchSelections() { + return (deps.onSearchSelectedText as ReturnType).mock.calls.map( + ([text]) => text as string + ) + }, get clearPaneCalls() { return (deps.onClearPaneScrollback as ReturnType).mock.calls.length }, @@ -514,6 +528,41 @@ describe('constructed shapes the macOS captures do not contain', () => { rig.unmount() }) + // The keydown path lets an empty floating panel claim a remapped tab.close before the pane + // does. Without the same order on the release, one chord closes a panel when pressed outside + // a composition and closes the pane when pressed inside one. + it('lets an empty floating panel claim a remapped tab.close from a release', async () => { + const panel = document.createElement('div') + panel.setAttribute('data-floating-terminal-panel', '') + panel.setAttribute('aria-hidden', 'false') + const emptyState = document.createElement('div') + emptyState.setAttribute('data-floating-terminal-empty-state', '') + panel.append(emptyState) + document.body.append(panel) + + const rig = openRig({ keybindings: { 'tab.close': ['Mod+Backspace'] } }) + await replay(rig.textarea, caseNamed(JAPANESE_CASE).rows) + + expect(rig.closePaneCalls).toEqual([]) + // The other three chords are untouched; only the panel's chord is withheld. + expect(rig.inputCalls).toEqual(['\x01', '\x1bb', '\x1b\x7f']) + rig.unmount() + }) + + // The keydown path runs the file-search chord ahead of the shortcut policy, so a release + // that went straight to the policy would make the same remap mean two different things + // depending on whether a composition happened to be live. + it('runs the file-search chord from a release, ahead of the byte fallback', async () => { + const rig = openRig({ keybindings: { 'sidebar.search.toggle': ['Mod+Backspace'] } }) + vi.spyOn(rig.terminal, 'getSelection').mockReturnValue('src/main.ts') + await replay(rig.textarea, caseNamed(JAPANESE_CASE).rows) + + expect(rig.searchSelections).toEqual(['src/main.ts']) + // The kill-line byte the built-in binding would have sent is not sent alongside it. + expect(rig.inputCalls).toEqual(['\x01', '\x1bb', '\x1b\x7f']) + rig.unmount() + }) + // A chord interrupted by Cmd+Tab or Spotlight never delivers its release here. The carry it // left must not answer for a later press of the same key: a bare arrow moving the preedit // caret would otherwise send the missing chord's bytes to a shell the user never aimed at. diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 4b92e2884bba..a3c7b1345aa7 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -43,24 +43,9 @@ import { syncTerminalScrollIntentFromViewport } from '@/lib/pane-manager/terminal-scroll-intent' -/** - * True when a live composition owns the event outright. A modifier chord over an arrow or - * Backspace/Delete is excluded so the terminal pane can resolve one from the key's *release* - * — see `isSwallowedImeChordRelease`. On a keydown the caller must still yield to the IME, - * because at that moment the two cases are indistinguishable (#12871). - */ -function isImeOwnedTerminalKeyEvent( - event: Parameters[0] -): boolean { - return isImeOwnedKeyboardEvent(event) && !isImeExemptTerminalChord(event) -} - -// `code` and `repeat` are optional on the policy's event type but always present here, and -// narrowing them keeps the snapshot usable by the shortcut matchers without a cast. type PendingImeChord = Parameters[0] & Required[0], 'code' | 'repeat'>> & { isComposing: boolean - keyCode: number } /** @@ -103,8 +88,9 @@ function imeChordSnapshot(event: KeyboardEvent): PendingImeChord { shiftKey: event.shiftKey, // One release is one action, so the auto-repeat presses behind it are not repeats of it. repeat: false, - isComposing: true, - keyCode: event.keyCode + // `isComposing` alone marks this as IME-owned for every consumer; the keyCode that also + // marked the press adds nothing once the press is over. + isComposing: true } } @@ -129,7 +115,9 @@ export function resolveTerminalKeyboardShortcutAction( isWindowsTerminalHost: NonNullable[10]>, terminalShortcutPolicy: Parameters[11] = 'orca-first' ): ReturnType { - if (isImeOwnedTerminalKeyEvent(event)) { + // The exempt chord is let through because the pane also calls this for such a chord's + // release; on a keydown the pane applies the stricter gate itself. + if (isImeOwnedKeyboardEvent(event) && !isImeExemptTerminalChord(event)) { return null } // Why: keep the host callback required at the production boundary so a diff --git a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts index ab6de449b422..503f212cd5c9 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -24,8 +24,12 @@ export type MacOptionAsAlt = 'true' | 'false' | 'left' | 'right' // chord over one of these keys outright — no commit, no platform replay, nothing reaching the // shell (#12871). Exempting them lets the terminal pane recover the chord from the key's // release, which is the first moment a swallowed chord is distinguishable from a delayed one -// (keyboard-handlers.ts, `isSwallowedImeChordRelease`). Read `code`, not `key`: Windows -// reports an IME-consumed key as `key: 'Process'` (#12171, #13033). +// (keyboard-handlers.ts, `imeChordSnapshot`). Read `code`, not `key`: Windows reports an +// IME-consumed key as `key: 'Process'` (#12171, #13033), which the keybinding matcher also +// falls back on — see PHYSICAL_CODE_FALLBACK_KEYS in shared/keybindings.ts. +// +// Adding a code here means teaching the Cmd+Up/Down branch below to read `chordKey` too; it +// reads `event.key`, which is the same value for every code outside this set. // // ArrowUp/ArrowDown are left out because no trace covers them, not because the reasoning // stops there — Cmd+↑/↓ scrolls the viewport and writes no bytes, so it is the safer half. @@ -147,73 +151,52 @@ export function resolveTerminalShortcutAction( ): TerminalShortcutAction | null { const platform: NodeJS.Platform = isMac ? 'darwin' : isWindows ? 'win32' : 'linux' const chordKey = physicalChordKey(event) - // Why: a composition rewrites `key` to 'Process', which normalizes to no token at all, and - // macOS has no non-Latin physical fallback (shared/keybindings.ts). Without substituting the - // physical key a user's remap onto Cmd+Backspace would miss and fall through to the built-in - // kill-line byte — silence before the IME exemption, the wrong action after it. - // - // Field by field rather than a spread: a real KeyboardEvent keeps `key`/`metaKey`/... as - // prototype accessors, so `{ ...event }` yields an empty object and every lookup below - // silently misses. Test doubles are plain objects and hide that. - const chordEvent: TerminalShortcutEvent = - chordKey === event.key - ? event - : { - key: chordKey, - code: event.code, - metaKey: event.metaKey, - ctrlKey: event.ctrlKey, - altKey: event.altKey, - shiftKey: event.shiftKey, - repeat: event.repeat - } - // Why: capture this chord even on repeat without blocking the OS default input-source switch. - if (keybindingMatchesAction('terminal.switchInputSource', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.switchInputSource', event, platform, keybindings)) { return { type: 'switchInputSource' } } if (!event.repeat) { - if (keybindingMatchesAction('terminal.copySelection', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.copySelection', event, platform, keybindings)) { return { type: 'copySelection' } } - if (keybindingMatchesAction('terminal.search', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.search', event, platform, keybindings)) { return { type: 'toggleSearch' } } - if (keybindingMatchesAction('terminal.clear', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.clear', event, platform, keybindings)) { return { type: 'clearActivePane' } } - if (keybindingMatchesAction('terminal.focusPreviousPane', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.focusPreviousPane', event, platform, keybindings)) { return { type: 'focusPane', direction: 'previous' } } - if (keybindingMatchesAction('terminal.focusNextPane', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.focusNextPane', event, platform, keybindings)) { return { type: 'focusPane', direction: 'next' } } - if (keybindingMatchesAction('terminal.equalizePaneSizes', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.equalizePaneSizes', event, platform, keybindings)) { return { type: 'equalizePaneSizes' } } - if (keybindingMatchesAction('terminal.expandPane', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.expandPane', event, platform, keybindings)) { return { type: 'toggleExpandActivePane' } } - if (keybindingMatchesAction('terminal.setTitle', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.setTitle', event, platform, keybindings)) { return { type: 'setTitle' } } - if (keybindingMatchesAction('terminal.clearPaneTitle', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.clearPaneTitle', event, platform, keybindings)) { return { type: 'clearPaneTitle' } } // Why: recognize the active tab.close binding as a pane-close alias too, so a user who remaps // tab.close alone still closes the focused split pane (never the whole tab); L2 always defers to us. if ( - isTerminalPaneCloseChord(chordEvent, platform, keybindings, undefined, { + isTerminalPaneCloseChord(event, platform, keybindings, undefined, { context: 'terminal', terminalShortcutPolicy }) @@ -221,11 +204,11 @@ export function resolveTerminalShortcutAction( return { type: 'closeActivePane' } } - if (keybindingMatchesAction('terminal.splitRight', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.splitRight', event, platform, keybindings)) { return { type: 'splitActivePane', direction: 'vertical' } } - if (keybindingMatchesAction('terminal.splitDown', chordEvent, platform, keybindings)) { + if (keybindingMatchesAction('terminal.splitDown', event, platform, keybindings)) { return { type: 'splitActivePane', direction: 'horizontal' } } } @@ -289,12 +272,10 @@ export function resolveTerminalShortcutAction( return { type: 'sendInput', data: '\x05' } } // Why: macOS users expect Cmd+↑/↓ to scroll scrollback, not write escape bytes to the shell. - // `chordKey` like its siblings, though it resolves to `key` today: ArrowUp/ArrowDown are not - // exempt codes, and reading `event.key` here would silently opt out if they ever became so. - if (chordKey === 'ArrowUp') { + if (event.key === 'ArrowUp') { return { type: 'scrollViewport', position: 'top' } } - if (chordKey === 'ArrowDown') { + if (event.key === 'ArrowDown') { return { type: 'scrollViewport', position: 'bottom' } } } diff --git a/src/shared/keybindings.ts b/src/shared/keybindings.ts index 4fd980aa4029..229dc42c1276 100644 --- a/src/shared/keybindings.ts +++ b/src/shared/keybindings.ts @@ -1597,7 +1597,9 @@ const PUNCTUATION_KEY_TOKENS = new Set([ 'Backquote' ]) -const PHYSICAL_CODE_FALLBACK_KEYS = new Set(['', 'Dead', 'Unidentified']) +// 'Process' is Windows' report for a key an IME consumed (#12171): the produced key is +// genuinely unreportable, which is the same condition as 'Dead' above. +const PHYSICAL_CODE_FALLBACK_KEYS = new Set(['', 'Dead', 'Unidentified', 'Process']) const SHIFTED_PUNCTUATION_KEY_TOKENS: Record = { '<': 'Comma', From 13b1e8cbdf9aa98d7e956c299f0317cf161eb6e2 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 8 Aug 2026 22:13:07 +0900 Subject: [PATCH 10/15] fix(terminal): stop consuming the keyup, and never arm from a text field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three reviewers independently reached the same keyup. Suppressing it buys nothing: by the time the release is consumed the action has already run, so there is no second handler left to race. It costs, though — cut off at window capture the keyup never reaches xterm's own _keyUp, which clears the flags its input path reads, nor any window listener registered after this one. The release now forwards preventDefault and drops the rest. Separately, a composition can live in a rename field or the search input, and that field can unmount while the key is still down. The release then arrives with the terminal as its target and walks past the editable guard, sending a chord aimed at the field to the shell. Refusing to arm from such a press closes that, where refusing at the release cannot. Three invariants the earlier commit messages asserted had no test behind them: the same-code supersede, the blur release, and spending the carry before a guard can refuse it. One test covering all three passed as long as any one worked, so it is now three, each failing only for its own path. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Mu6j33LNsd4Mvv8a1NuFoo --- ....issue-12871-recorded-chord-traces.test.ts | 123 +++++++++++++++--- .../terminal-pane/keyboard-handlers.ts | 35 +++-- 2 files changed, 134 insertions(+), 24 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index dc7cb3daa5a6..2b11013b3595 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -563,28 +563,78 @@ describe('constructed shapes the macOS captures do not contain', () => { rig.unmount() }) - // A chord interrupted by Cmd+Tab or Spotlight never delivers its release here. The carry it - // left must not answer for a later press of the same key: a bare arrow moving the preedit - // caret would otherwise send the missing chord's bytes to a shell the user never aimed at. - it('does not answer a later press with a carry whose release never arrived', async () => { + // The composition can live in a rename field or the search input, and that field can unmount + // while the key is still down — the release then arrives with the terminal as its target and + // walks straight past the editable guard. Refusing to arm from such a press closes that, + // where refusing at the release cannot. + it('never arms from a press aimed at a text field, even if the field then unmounts', async () => { const rig = openRig() + const field = document.createElement('input') + rig.textarea.parentElement?.append(field) + + const press = new KeyboardEvent('keydown', { + key: 'ArrowLeft', + code: 'ArrowLeft', + altKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperties(press, { isComposing: { value: true }, keyCode: { value: 229 } }) + field.dispatchEvent(press) + await macrotask() + field.remove() + await replay(rig.textarea, [ - { t: 'compositionstart', data: '' }, - { t: 'keydown', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true, alt: true }, - { - t: 'keydown', - key: 'ArrowLeft', - code: 'ArrowLeft', - keyCode: 229, - isComposing: true, - alt: true - } - // Focus leaves the window here: no keyup for either key ever arrives. + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true, alt: true } ]) + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) - window.dispatchEvent(new Event('blur')) + // The action has already run by the time the release is consumed, so there is nothing left to + // race — and cutting a keyup off at window capture keeps it from ever reaching xterm, whose + // own keyup handler clears the flags its input path reads. + it('leaves the keyup itself to finish propagating', async () => { + const rig = openRig() + const seen: string[] = [] + const listener = (event: KeyboardEvent): void => { + seen.push(event.type) + } + rig.textarea.addEventListener('keyup', listener) + await replay(rig.textarea, caseNamed(JAPANESE_CASE).rows) + rig.textarea.removeEventListener('keyup', listener) + + expect(rig.inputCalls).toEqual(['\x01', '\x1bb', '\x15', '\x1b\x7f']) + // Four chord releases plus the letters and modifiers around them: every one arrives. + expect(seen.length).toBe( + caseNamed(JAPANESE_CASE).rows.filter((row) => row.t === 'keyup').length + ) + rig.unmount() + }) + + // Three separate ways a carry can be left with no release of its own to spend it, each with + // its own escape. Split apart deliberately: one test covering all three passes as long as any + // one of them works, which pins none of them. + const ARMED_ALT_ARROW: RecordedRow[] = [ + { t: 'compositionstart', data: '' }, + { t: 'keydown', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true, alt: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + alt: true + } + ] + + // The user pressed the key again for its own sake. Whatever the last press carried is over, + // even though this one is IME-owned too and claims no carry of its own. + it('lets a later press of the same key supersede the carry', async () => { + const rig = openRig() await replay(rig.textarea, [ + ...ARMED_ALT_ARROW, { t: 'keydown', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 229, isComposing: true }, { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true } ]) @@ -593,6 +643,47 @@ describe('constructed shapes the macOS captures do not contain', () => { rig.unmount() }) + // Cmd+Tab or Spotlight takes the release with it. If the window gets the keyup back later, + // the chord it belonged to is long gone. + it('drops the carry when focus leaves the window', async () => { + const rig = openRig() + await replay(rig.textarea, ARMED_ALT_ARROW) + window.dispatchEvent(new Event('blur')) + await replay(rig.textarea, [ + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true, alt: true } + ]) + + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + + // The release arrived but a guard turned it away. The key is still up, so the press is still + // over — a carry that survives a refused release answers for whatever comes next instead. + it('spends the carry on a release a guard refuses', async () => { + const rig = openRig() + const field = document.createElement('input') + rig.textarea.parentElement?.append(field) + await replay(rig.textarea, ARMED_ALT_ARROW) + + const refused = new KeyboardEvent('keyup', { + key: 'ArrowLeft', + code: 'ArrowLeft', + altKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperties(refused, { isComposing: { value: true }, keyCode: { value: 37 } }) + field.dispatchEvent(refused) + await macrotask() + + await replay(rig.textarea, [ + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true, alt: true } + ]) + + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + // A KeyboardEvent's modifier flags describe the moment it fired, so letting Cmd up before // the arrow leaves the arrow's keyup with metaKey: false. Reading the release's own flags // drops the chord outright, which is the bug this whole change exists to fix. diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index a3c7b1345aa7..aaa3eb86b9b4 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -43,6 +43,16 @@ import { syncTerminalScrollIntentFromViewport } from '@/lib/pane-manager/terminal-scroll-intent' +// What runShortcutAction actually touches. Narrow so the release can hand it a chord that +// answers for the press rather than for the event that happens to be in flight. +type ShortcutActionEvent = { + key: string + code?: string + repeat: boolean + preventDefault: () => void + stopImmediatePropagation: () => void +} + type PendingImeChord = Parameters[0] & Required[0], 'code' | 'repeat'>> & { isComposing: boolean @@ -444,7 +454,10 @@ export function useTerminalKeyboardShortcuts({ // Every IME-owned keydown yields, chord or not. A chord the IME then swallows is // recovered from its release; one it merely delays arrives as an unmarked replay. if (isImeOwnedKeyboardEvent(e)) { - if (isImeExemptTerminalChord(e)) { + // Not armed from a rename field or the search input: that field can unmount before the + // key comes up, and the release would then arrive with the terminal as its target and + // pass the guard below, sending a chord aimed at the field to the shell. + if (isImeExemptTerminalChord(e) && !isEditableTarget(e.target)) { pendingImeChord = imeChordSnapshot(e) } return @@ -502,7 +515,7 @@ export function useTerminalKeyboardShortcuts({ // Shared by the keydown path and the swallowed-chord release below, so a chord recovered // from a release runs the same action a press would — including remaps onto pane commands. function runShortcutAction( - e: KeyboardEvent, + e: ShortcutActionEvent, action: NonNullable>, manager: PaneManager ): void { @@ -746,13 +759,19 @@ export function useTerminalKeyboardShortcuts({ if (isEditableTarget(e.target)) { return } - // Matched on the press, suppressed on the release: `chord` carries the modifiers as they - // were when the key went down, while only the real event can consume the keystroke. - const pressed = { + // Matched on the press, consumed on the release. `chord` carries the modifiers as they + // were when the key went down; `preventDefault` forwards to the real event. + // + // The propagation stoppers do not. On a keydown they keep a second handler from acting on + // the same press, but by this point the action has already run and there is nothing left + // to race. Cutting a keyup off at window capture only costs: it never reaches xterm's own + // `_keyUp`, which clears the flags its input path reads, nor any window listener that + // happens to be registered after this one. + const pressed: PendingImeChord & ShortcutActionEvent & { stopPropagation: () => void } = { ...chord, preventDefault: () => e.preventDefault(), - stopPropagation: () => e.stopPropagation(), - stopImmediatePropagation: () => e.stopImmediatePropagation() + stopPropagation: () => {}, + stopImmediatePropagation: () => {} } // Same precedence as the keydown path: a chord remapped onto tab.close closes an empty // floating panel there, and skipping it here would close the pane instead. @@ -775,7 +794,7 @@ export function useTerminalKeyboardShortcuts({ if (!action || action.type === 'switchInputSource') { return } - runShortcutAction(e, action, manager) + runShortcutAction(pressed, action, manager) } // Why: a chord interrupted by Cmd+Tab or Spotlight never delivers its release here, and a From 079735b89bca8c4eeee6191773eecdb1e676dc99 Mon Sep 17 00:00:00 2001 From: hyeonho Date: Sat, 8 Aug 2026 23:36:34 +0900 Subject: [PATCH 11/15] test(terminal): replay the in-app IME chord traces through the release rig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second recording of the same gestures, taken inside the app rather than on a bare page, replayed through the rig already on this branch. Both cases are negatives: the bare-page rows pin where the chord bytes come from, and these pin the keys around them, which is where a release-keyed recovery misfires. The Korean case carries a plain ArrowRight pressed one beat after the chord — no bare-page case has one — so a carry left armed past its own release has something of the same `code` to answer for. Its \x05 is not asserted: the app's own window capture consumed the platform's unmarked replay above the probe, so the press that delivers the byte was never recorded. The third recording of that session, a Kotoeri Cmd+ArrowLeft, is not replayed. Its chord press is followed by no arrow keyup anywhere, so replaying it could only assert that no byte is produced. That is raised on the PR instead. --- ...andlers.issue-12871-in-app-chord-traces.ts | 126 ++++++++++++++++++ ....issue-12871-recorded-chord-traces.test.ts | 8 +- 2 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts new file mode 100644 index 000000000000..8d06e27377d0 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts @@ -0,0 +1,126 @@ +// Recorded IME chord traces for #12871 from a SECOND, independent surface: a boundary probe at +// the xterm helper textarea of a dev e2e Orca build (app commit +// d64ccc71bd7daf7ff64fe182b9bf522b90293397, macOS 26.5.2/25F84, darwin arm64), driven through +// the real OS input methods via System Events CGEvents, with PTY-side ground-truth bytes read +// by a child on the pty. Replayed by keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +// alongside the bare-page recording that lives there. Source traces (SHA-256): +// ime-traces/korean-cmdright.json d2c12ee4828cfe8671248122f9d36875a22c020c72500eda886d7a1f04c1b790 +// ime-traces/negative-abc.json 9c9b128406381a6479256abdec8088397940d9eb436ca219660901c4205a3aa9 +// +// Both cases below are NEGATIVES, and deliberately so. The bare-page recording already pins +// where the chord bytes come from; what this surface adds is the keys around them, which is +// where a release-keyed recovery can misfire — a carry that outlives its own gesture, or one +// armed from a press that had already resolved. This surface also differs from the bare-page one +// in ways the cases note inline (its window-level capture consumed unmarked chords above the +// probe, and the CGEvent driver produced no lone-modifier keydowns), so what each case can speak +// for is stated with it. Each case opens with a snapshot row restoring the textarea state the +// probe found mid-session (the leading ㄱ residue is real; the recorded composition offsets +// depend on it). A snapshot row carries only `value`, so dispatching it is a no-op for xterm. +// +// A third recording, ime-traces/japanese-overwrite-cmdleft.json (Kotoeri Romaji さ preedit, +// Cmd+ArrowLeft), is deliberately NOT replayed here. Its chord press is followed by no arrow +// keyup at all, and a window-level capture probe run on this branch found none there either, so +// replaying it could only assert that no byte is produced — which pins the #12871 defect rather +// than the fix. That recording is evidence about the platform, not a contract for this handler, +// and is raised on #12732 instead of frozen into a green assertion here. + +// Structurally identical to the replaying test's own RecordedRow/RecordedCase; declared here so +// the data module stands alone and the test file keeps its types private. +export type InAppRecordedRow = { + t: string + key?: string + code?: string + keyCode?: number + isComposing?: boolean + meta?: boolean + alt?: boolean + data?: string + inputType?: string + value?: string +} + +export type InAppRecordedCase = { + name: string + expectCalls: string[] + expectEmitted: string[] + rows: InAppRecordedRow[] +} + +export const IN_APP_TRACE_CASES: InAppRecordedCase[] = [ + { + // korean-cmdright.json dom[36..50]: committed 가나 with 나 composing, Cmd+ArrowRight. + // Captured PTY line: 가나나\x05\x1b[C\n. + // + // The \x05 is absent from expectCalls because it is absent from these rows: the app's own + // window-level capture consumed the platform's unmarked replay above the probe, so the press + // that delivers the byte was never recorded. What survives is the half this surface can + // speak for, and it is the half a release-keyed recovery can break — both of the pane's own + // chances to fire on a committing input source, plus the key that follows: + // + // - the marked keydown (keyCode 229, isComposing) must produce nothing; + // - the release must produce nothing either, because the composition has already ended by + // then and the platform's replay is the thing that answers; + // - and the PLAIN ArrowRight pressed one beat later must reach xterm as the recorded + // \x1b[C. This surface recorded no lone-modifier keyup, so a carry left armed past its + // own release would still be holding when this bare press arrives, and it matches by the + // same `code` — the supersede is what keeps it from answering as a stray \x05. + name: 'Korean 2-Set, Cmd+ArrowRight, then a bare ArrowRight (in-app trace)', + expectCalls: [], + expectEmitted: ['나', '\u001b[C', '\r'], + rows: [ + { t: 'input', value: 'ㄱ가나' }, + { t: 'compositionstart', data: '' }, + { t: 'compositionupdate', data: '나' }, + { t: 'input', data: '나', inputType: 'insertCompositionText', value: 'ㄱ가나나' }, + { t: 'keyup', key: 'ㅏ', code: 'KeyK', keyCode: 75, isComposing: true }, + { + t: 'keydown', + key: 'ArrowRight', + code: 'ArrowRight', + keyCode: 229, + isComposing: true, + meta: true + }, + { t: 'compositionupdate', data: '나' }, + { t: 'input', data: '나', inputType: 'insertCompositionText', value: 'ㄱ가나나' }, + { t: 'compositionend', data: '나' }, + { t: 'keyup', key: 'ArrowRight', code: 'ArrowRight', keyCode: 39, meta: true }, + { t: 'keydown', key: 'ArrowRight', code: 'ArrowRight', keyCode: 39 }, + { t: 'keyup', key: 'ArrowRight', code: 'ArrowRight', keyCode: 39 }, + { t: 'keydown', key: 'Enter', code: 'Enter', keyCode: 13 }, + { t: 'keyup', key: 'Enter', code: 'Enter', keyCode: 13 } + ] + }, + { + // negative-abc.json dom[0..8], the recorded non-IME control the composition rulebook asks + // for: ABC layout, not a Latin preedit — the whole capture holds no 229, no isComposing, + // no composition event. expectEmitted equals the captured onData array verbatim, and the + // captured PTY line is abc\x01\x1bb\n. + // + // The two chord KEYDOWN rows are the one reconstruction in this module: the probe sat + // below the window capture, which consumed them (their bytes still arrived — that + // consumption is itself the recorded proof the handler owns unmarked chords). Their fields + // are the captured Option keyup's (dom[6]) with the type flipped, and the Cmd copy mirrors + // it; the captured onData/PTY pin the bytes they must produce. + // + // Against a release-keyed recovery it is also the arming negative: no composition is live at + // either press, so both resolve from their own keydown, and the recorded keyup that follows + // must not send \x1bb a second time. + name: 'ABC layout, Cmd+ArrowLeft then Option+ArrowLeft with no IME (in-app trace)', + expectCalls: ['\u0001', '\u001bb'], + expectEmitted: ['a', 'b', 'c', '\u0001', '\u001bb', '\r'], + rows: [ + { t: 'keydown', key: 'a', code: 'KeyA', keyCode: 65 }, + { t: 'keyup', key: 'a', code: 'KeyA', keyCode: 65 }, + { t: 'keydown', key: 'b', code: 'KeyB', keyCode: 66 }, + { t: 'keyup', key: 'b', code: 'KeyB', keyCode: 66 }, + { t: 'keydown', key: 'c', code: 'KeyC', keyCode: 67 }, + { t: 'keyup', key: 'c', code: 'KeyC', keyCode: 67 }, + { t: 'keydown', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, meta: true }, + { t: 'keydown', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, alt: true }, + { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, alt: true }, + { t: 'keydown', key: 'Enter', code: 'Enter', keyCode: 13 }, + { t: 'keyup', key: 'Enter', code: 'Enter', keyCode: 13 } + ] + } +] diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index 2b11013b3595..d57c6c99e161 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -22,12 +22,18 @@ // intercepting. And `expectEmitted` groups bytes the way a full task between rows produces — // real input arrives in one burst and may group differently. Read `expectCalls` for the // contract; `expectEmitted` is there to show ordering, not framing. +// +// IN_APP_TRACE_CASES is a second, independent recording taken inside the app rather than on a +// bare page (provenance and per-case notes in keyboard-handlers.issue-12871-in-app-chord-traces +// .ts). It is replayed through the same rig below. The two surfaces captured different subsets +// of the same gestures, so each case there states which half of the contract it can speak for. import { Terminal } from '@xterm/xterm' import { cleanup, renderHook } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { PtyTransport } from './pty-transport' import { useTerminalKeyboardShortcuts } from './keyboard-handlers' +import { IN_APP_TRACE_CASES } from './keyboard-handlers.issue-12871-in-app-chord-traces' type RecordedRow = { t: string @@ -428,7 +434,7 @@ describe('recorded macOS chord traces during an IME composition', () => { document.body.replaceChildren() }) - it.each(CASES.map((testCase) => [testCase.name, testCase] as const))( + it.each([...CASES, ...IN_APP_TRACE_CASES].map((testCase) => [testCase.name, testCase] as const))( '%s', async (_name, testCase) => { const rig = openRig() From 8ba40b7c97f33eb57cde9bd893fb359a8aa279d7 Mon Sep 17 00:00:00 2001 From: hyeonho Date: Sat, 8 Aug 2026 23:36:34 +0900 Subject: [PATCH 12/15] test(e2e): assert IME cursor-chord byte order on the real macOS IMEs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unit rig replays recorded traces; this is the layer those traces came from, with the real OS input methods deciding everything and the bytes read off the pty. A platform change — redispatch timing, chord swallowing — fails here first. Korean 2-Set and the ABC control pass on this branch. The Kotoeri case does not: the chord byte never reaches the pty, so it is currently a failing pre-merge finding rather than a regression guard. Kept unmodified rather than relaxed, because relaxing it would erase the evidence. --- ...inal-macos-ime-cursor-chord-native.spec.ts | 355 ++++++++++++++++++ 1 file changed, 355 insertions(+) create mode 100644 tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts diff --git a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts new file mode 100644 index 000000000000..f33209c55007 --- /dev/null +++ b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts @@ -0,0 +1,355 @@ +import { execFileSync } from 'node:child_process' +import path from 'node:path' +import type { Page } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + focusActiveTerminalInput, + getTerminalContent, + waitForActivePanePtyId, + waitForActiveTerminalManager +} from './helpers/terminal' +import { + createTerminalImeByteReader, + removeTerminalImeByteReader, + startTerminalImeByteReader, + waitForTerminalImeBytes, + type TerminalImeByteReader +} from './terminal-ime-byte-reader' + +/** + * #12871 — cursor chords during a live IME composition, asserted at the PTY, with the real OS + * input methods deciding everything. The unit layer replays recorded traces; this spec is the + * layer those traces came from, so a platform change (macOS redispatch timing, Kotoeri chord + * swallowing) fails here first. + * + * What the recorded evidence established (in-app traces of 2026-08-08 at commit d64ccc71b, and + * the independent bare-page recording on #12732): + * - 2-Set Korean commits the composing syllable ON the chord; the PTY must see the syllable + * strictly before the movement byte, exactly once each. This held on main and must survive + * the #12732 exemption+ledger (whose failure mode is the byte twice). + * - Kotoeri swallows the chord: the preedit must survive the press. On main the chord byte + * then never reached the shell at all (the defect half of #12871); with #12732's exemption + * the byte queues behind the preedit and lands right after the commit. The Kotoeri byte + * expectation below therefore REQUIRES the fix — on a pre-fix build this test fails on the + * missing \x01, which is exactly the regression it exists to catch. + * - ABC control: no IME anywhere, movement bytes flow alone. + * + * Run-validity guards follow terminal-macos-korean-chord-commit-native.spec.ts: the selected + * input source is read back, and a composition must actually form before any target key is + * pressed, so a run where the IME never engaged is void rather than green. + */ + +const TWO_SET_KOREAN_ID = 'com.apple.inputmethod.Korean.2SetKorean' +const KOTOERI_ROMAJI_ID = 'com.apple.inputmethod.Kotoeri.RomajiTyping.Japanese' +// Selecting a Kotoeri MODE fails with paramErr (-50) while its parent input method is disabled, +// and the parent has a different InputSourceID, so select-input-source.swift's own +// enable-everything-for-this-id pass cannot reach it. Enable the parent first. +const KOTOERI_ROMAJI_PARENT_ID = 'com.apple.inputmethod.Kotoeri.RomajiTyping' +const ABC_ID = 'com.apple.keylayout.ABC' +const SELECT_INPUT_SOURCE = path.resolve(__dirname, 'select-input-source.swift') + +const KEY = { + left: 123, + right: 124, + return: 36, + backspace: 51 +} as const + +function selectInputSource(id: string): void { + execFileSync('swift', [SELECT_INPUT_SOURCE, id]) +} + +/** Enable (without selecting) every TIS entry whose InputSourceID matches `id`. */ +function enableInputSource(id: string): void { + const source = ` +import Carbon +let properties = [kTISPropertyInputSourceID: ${JSON.stringify(id)} as CFString] as CFDictionary +let sources = TISCreateInputSourceList(properties, true).takeRetainedValue() as! [TISInputSource] +guard !sources.isEmpty else { exit(3) } +for candidate in sources { TISEnableInputSource(candidate) } +exit(0) +` + execFileSync('swift', ['-'], { input: source }) +} + +function focusApp(processId: number): void { + execFileSync('osascript', [ + '-e', + `tell application "System Events" to set frontmost of first application process whose unix id is ${processId} to true`, + '-e', + 'delay 0.3' + ]) +} + +/** Bounce focus away and back so the app's input context re-reads the selected input source. */ +function bounceFocus(processId: number): void { + execFileSync('osascript', ['-e', 'tell application "Finder" to activate', '-e', 'delay 0.4']) + focusApp(processId) +} + +/** Type key codes one at a time with a delay so the IME engages on every keystroke. */ +function typeKeyCodes(processId: number, keyCodes: readonly number[]): void { + focusApp(processId) + execFileSync('osascript', [ + '-e', + 'tell application "System Events"', + '-e', + `repeat with currentKeyCode in {${keyCodes.join(', ')}}`, + '-e', + 'key code (currentKeyCode as integer)', + '-e', + 'delay 0.12', + '-e', + 'end repeat', + '-e', + 'end tell' + ]) +} + +/** The chord itself, pressed by the OS so the IME decides how to resolve it. */ +function pressChord(processId: number, keyCode: number, modifier?: 'command' | 'option'): void { + focusApp(processId) + const using = modifier ? ` using ${modifier} down` : '' + execFileSync('osascript', [ + '-e', + `tell application "System Events" to key code ${keyCode}${using}` + ]) +} + +function readActiveComposition(page: Page): Promise { + // The macOS preedit lives in xterm's `.composition-view`, not in the helper textarea value; + // null distinguishes an absent composition from an empty one. + return page.evaluate(() => { + const textarea = document.querySelector('.xterm-helper-textarea:focus') + const composition = textarea?.parentElement?.querySelector( + '.composition-view.active' + ) + return composition?.textContent?.replaceAll('\u200e', '') ?? null + }) +} + +function readInputSourceId(page: Page): Promise { + return page.evaluate(() => window.api.app.getKeyboardInputSourceId()) +} + +/** + * Run-validity warmup: compose 가, confirm real Hangul preedit, then erase it jamo-by-jamo so + * nothing commits. Kills the first-keystroke engagement race the plain 2set spec flakes on. + * + * Measured on this rig: when the app launches while another layout is still selected, the + * freshly focused input context can miss the switch entirely and the first keystrokes type + * literal r/k. Erase whatever landed (preedit jamo or literal characters — backspace edits + * either buffer without reaching the recorded line), bounce focus so the context re-reads the + * source, and prove engagement again — the same dance the Kotoeri path always needed. + */ +async function warmUpKoreanComposition(page: Page, processId: number): Promise { + typeKeyCodes(processId, [15, 40]) + try { + await expect.poll(() => readActiveComposition(page), { timeout: 6_000 }).toMatch(/[가-힣ㄱ-ㅣ]/) + } catch { + typeKeyCodes(processId, [KEY.backspace, KEY.backspace]) + bounceFocus(processId) + await focusActiveTerminalInput(page) + typeKeyCodes(processId, [15, 40]) + await expect + .poll(() => readActiveComposition(page), { timeout: 10_000 }) + .toMatch(/[가-힣ㄱ-ㅣ]/) + } + typeKeyCodes(processId, [KEY.backspace, KEY.backspace]) + await expect.poll(() => readActiveComposition(page), { timeout: 10_000 }).toBeNull() +} + +/** Press Return to flush the pending line into the byte reader; a live preedit eats the first. */ +async function flushLineToReader( + page: Page, + processId: number, + reader: TerminalImeByteReader +): Promise { + pressChord(processId, KEY.return) + try { + return await waitForTerminalImeBytes(page, reader, 5_000) + } catch { + pressChord(processId, KEY.return) + return waitForTerminalImeBytes(page, reader, 10_000) + } +} + +type SetupResult = { + ptyId: string + reader: TerminalImeByteReader +} + +async function setUpTerminalWithReader( + page: Page, + testRepoPath: string, + processId: number, + warmUpKorean: boolean +): Promise { + await waitForActiveWorktree(page) + await waitForSessionReady(page) + await ensureTerminalVisible(page) + await waitForActiveTerminalManager(page) + const ptyId = await waitForActivePanePtyId(page) + await focusActiveTerminalInput(page) + if (warmUpKorean) { + selectInputSource(TWO_SET_KOREAN_ID) + // The app is already focused, so its input context needs the bounce to adopt the switch — + // without it the first run of a session composes nothing (the machine sat on another + // layout) while later runs pass because afterEach left Korean selected before launch. + bounceFocus(processId) + await focusActiveTerminalInput(page) + await expect.poll(() => readInputSourceId(page), { timeout: 10_000 }).toBe(TWO_SET_KOREAN_ID) + await warmUpKoreanComposition(page, processId) + } + const reader = createTerminalImeByteReader(testRepoPath, 1) + await startTerminalImeByteReader(page, ptyId, reader) + await focusActiveTerminalInput(page) + return { ptyId, reader } +} + +test.describe('Native macOS IME cursor chords during composition @headful', () => { + test.skip( + process.platform !== 'darwin' || process.env.ORCA_E2E_NATIVE_MACOS_KOREAN !== '1', + 'Requires macOS with native IME access and Accessibility permission' + ) + + test.afterEach(() => { + // Leave the machine on Korean 2-Set no matter which source a scenario selected. + selectInputSource(TWO_SET_KOREAN_ID) + }) + + // The tty converts the trailing CR to LF, so a line arriving with \n is the standing proof + // the capture reached past the renderer (#11936/#11951). + for (const chord of [ + { name: 'Cmd+Left', modifier: 'command' as const, pty: '하\x01\n' }, + { name: 'Option+Left', modifier: 'option' as const, pty: '하\x1bb\n' } + ]) { + test(`Korean 2-Set: ${chord.name} mid-syllable commits 하 before the movement byte`, async ({ + electronApp, + orcaPage, + testRepoPath + }) => { + const processId = electronApp.process().pid + if (processId === undefined) { + throw new Error('Electron process id unavailable') + } + const setup = await setUpTerminalWithReader(orcaPage, testRepoPath, processId, true) + try { + // ㅎ(5) + ㅏ(40) → 하, still composing. The poll doubles as the positive control. + typeKeyCodes(processId, [5, 40]) + await expect.poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }).toBe('하') + + pressChord(processId, KEY.left, chord.modifier) + // Recorded: 2-Set Korean answers the chord with compositionend — the preedit must be + // gone before any flush. A surviving preedit here would eat the Return below and turn + // the byte assertion into a different scenario's evidence. + await expect.poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }).toBeNull() + + // Exactly the recorded line: the syllable strictly before its movement byte, one of + // each. The double-fire failure mode (#12732's naive exemption) would append a second + // movement byte; a reordering regression would put it before 하. + expect(await flushLineToReader(orcaPage, processId, setup.reader)).toEqual([ + Buffer.from(chord.pty).toString('hex') + ]) + expect(await getTerminalContent(orcaPage, 100_000)).toContain('하') + } finally { + removeTerminalImeByteReader(setup.reader) + } + }) + } + + test('Kotoeri Romaji: Cmd+Left leaves the さ preedit live; its byte lands after the commit', async ({ + electronApp, + orcaPage, + testRepoPath + }) => { + const processId = electronApp.process().pid + if (processId === undefined) { + throw new Error('Electron process id unavailable') + } + // Korean warmup first proves the rig composes at all before the source switch. + const setup = await setUpTerminalWithReader(orcaPage, testRepoPath, processId, true) + try { + enableInputSource(KOTOERI_ROMAJI_PARENT_ID) + enableInputSource(KOTOERI_ROMAJI_ID) + selectInputSource(KOTOERI_ROMAJI_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + // Selecting the RomajiTyping mode succeeds, but TIS reports the current source under the + // legacy mode id 'com.apple.inputmethod.Japanese'. + await expect + .poll(() => readInputSourceId(orcaPage), { timeout: 10_000 }) + .toMatch(/^com\.apple\.inputmethod\.(Kotoeri\.RomajiTyping\.)?Japanese$/) + + // s(1) + a(0) → さ in the preedit. Bounce timing can swallow the first key; one retry. + typeKeyCodes(processId, [1, 0]) + try { + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 6_000 }) + .toMatch(/[ぁ-ん]/) + } catch { + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + typeKeyCodes(processId, [1, 0]) + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) + .toMatch(/[ぁ-ん]/) + } + + pressChord(processId, KEY.left, 'command') + // Recorded: Kotoeri swallows the chord — no commit, no composition event. The preedit + // surviving the press is the half of #12871 that held on main and must keep holding. + await orcaPage.waitForTimeout(700) + await expect.poll(() => readActiveComposition(orcaPage)).toMatch(/[ぁ-ん]/) + + // The Return commits さ and macOS redispatches it unmarked, which also flushes the line. + // Byte order pins the #12732 exemption end to end: the chord's \x01 was queued behind + // the preedit and must drain right after the commit — never before it, and exactly once. + // On a pre-fix build this line arrives as さ\n (the byte silently dropped), which is the + // #12871 defect this spec exists to keep fixed. + expect(await flushLineToReader(orcaPage, processId, setup.reader)).toEqual([ + Buffer.from('さ\x01\n').toString('hex') + ]) + expect(await getTerminalContent(orcaPage, 100_000)).toContain('さ') + } finally { + removeTerminalImeByteReader(setup.reader) + selectInputSource(TWO_SET_KOREAN_ID) + } + }) + + test('ABC control: the same chords with no IME flow alone', async ({ + electronApp, + orcaPage, + testRepoPath + }) => { + const processId = electronApp.process().pid + if (processId === undefined) { + throw new Error('Electron process id unavailable') + } + const setup = await setUpTerminalWithReader(orcaPage, testRepoPath, processId, false) + try { + selectInputSource(ABC_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + await expect.poll(() => readInputSourceId(orcaPage), { timeout: 10_000 }).toBe(ABC_ID) + + typeKeyCodes(processId, [0, 11, 8]) // a b c + await expect + .poll(() => getTerminalContent(orcaPage, 100_000), { timeout: 10_000 }) + .toContain('abc') + + pressChord(processId, KEY.left, 'command') + pressChord(processId, KEY.left, 'option') + + // The genuine non-IME control per the composition rulebook: with no composition anywhere + // the movement bytes flow immediately and alone, in press order. + expect(await flushLineToReader(orcaPage, processId, setup.reader)).toEqual([ + Buffer.from('abc\x01\x1bb\n').toString('hex') + ]) + } finally { + removeTerminalImeByteReader(setup.reader) + selectInputSource(TWO_SET_KOREAN_ID) + } + }) +}) From 4887d924ca81c1518481dd2d9e798b6b08e74cc8 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sun, 9 Aug 2026 01:24:53 +0900 Subject: [PATCH 13/15] test(e2e): probe where an IME chord's keyup goes inside the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release-keyed recovery never fires for Cmd chords in-app while the bare-page recording of the same input source and the same preedit delivers that release. This records the difference instead of reasoning about it: four listener positions, plus focus ownership and the live target on every row, so "never dispatched" can be told apart from "dispatched somewhere else". What it measured. Cmd+Left during a Kotoeri さ preedit produces the keydown at all four positions and no keyup at any of them, while a bare ArrowLeft pressed right after produces both, still composing, with the window still focused. The ABC control has no IME anywhere and shows the same split: Cmd loses its keyup, Option keeps it. Option+Left during the same preedit reaches the pty as さ\x1bb — the recovery does work, on the modifier that still reports a release. So the discriminator is the Command modifier, not the composition state, and not the input source. Kept as the capture harness rather than a gate: it asserts only that rows were recorded, and writes the traces under test-results. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Mu6j33LNsd4Mvv8a1NuFoo --- ...al-macos-kotoeri-chord-keyup-probe.spec.ts | 380 ++++++++++++++++++ 1 file changed, 380 insertions(+) create mode 100644 tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts diff --git a/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts b/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts new file mode 100644 index 000000000000..302e262a8167 --- /dev/null +++ b/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts @@ -0,0 +1,380 @@ +import { execFileSync } from 'node:child_process' +import { mkdirSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import type { Page } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + focusActiveTerminalInput, + waitForActivePanePtyId, + waitForActiveTerminalManager +} from './helpers/terminal' +import { + createTerminalImeByteReader, + removeTerminalImeByteReader, + startTerminalImeByteReader, + waitForTerminalImeBytes +} from './terminal-ime-byte-reader' + +/** + * #12871 capture, not a gate. The bare-page recording of the same input source and the same + * preedit delivers the chord's keyup at every listener position, still composing; in-app the + * recovery never fires. This spec records where the release goes instead — at four positions, + * with focus ownership and the live target sampled on every row, so "never dispatched" can be + * told apart from "dispatched somewhere else". + */ + +const TWO_SET_KOREAN_ID = 'com.apple.inputmethod.Korean.2SetKorean' +const KOTOERI_ROMAJI_ID = 'com.apple.inputmethod.Kotoeri.RomajiTyping.Japanese' +const KOTOERI_ROMAJI_PARENT_ID = 'com.apple.inputmethod.Kotoeri.RomajiTyping' +const ABC_ID = 'com.apple.keylayout.ABC' +const SELECT_INPUT_SOURCE = path.resolve(__dirname, 'select-input-source.swift') +const KEY = { left: 123, backspace: 51 } as const + +function selectInputSource(id: string): void { + execFileSync('swift', [SELECT_INPUT_SOURCE, id]) +} + +function enableInputSource(id: string): void { + execFileSync('swift', ['-'], { + input: ` +import Carbon +let properties = [kTISPropertyInputSourceID: ${JSON.stringify(id)} as CFString] as CFDictionary +let sources = TISCreateInputSourceList(properties, true).takeRetainedValue() as! [TISInputSource] +guard !sources.isEmpty else { exit(3) } +for candidate in sources { TISEnableInputSource(candidate) } +exit(0) +` + }) +} + +function focusApp(processId: number): void { + execFileSync('osascript', [ + '-e', + `tell application "System Events" to set frontmost of first application process whose unix id is ${processId} to true`, + '-e', + 'delay 0.3' + ]) +} + +function bounceFocus(processId: number): void { + execFileSync('osascript', ['-e', 'tell application "Finder" to activate', '-e', 'delay 0.4']) + focusApp(processId) +} + +function typeKeyCodes(processId: number, keyCodes: readonly number[]): void { + focusApp(processId) + execFileSync('osascript', [ + '-e', + 'tell application "System Events"', + '-e', + `repeat with currentKeyCode in {${keyCodes.join(', ')}}`, + '-e', + 'key code (currentKeyCode as integer)', + '-e', + 'delay 0.12', + '-e', + 'end repeat', + '-e', + 'end tell' + ]) +} + +function pressChord(processId: number, keyCode: number, modifier?: 'command' | 'option'): void { + focusApp(processId) + execFileSync('osascript', [ + '-e', + `tell application "System Events" to key code ${keyCode}${modifier ? ` using ${modifier} down` : ''}` + ]) +} + +function readActiveComposition(page: Page): Promise { + return page.evaluate(() => { + const textarea = document.querySelector('.xterm-helper-textarea:focus') + const composition = textarea?.parentElement?.querySelector( + '.composition-view.active' + ) + return composition?.textContent?.replaceAll('\u200e', '') ?? null + }) +} + +/** Four listener positions plus focus ownership, so a missing release can be located. */ +async function installChordProbe(page: Page): Promise { + await page.evaluate(() => { + type ProbeWindow = Window & { __chordKeyupProbe?: { rows: unknown[]; dispose: () => void } } + const probeWindow = window as ProbeWindow + probeWindow.__chordKeyupProbe?.dispose() + + const textarea = document.querySelector('.xterm-helper-textarea:focus') + if (!textarea) { + throw new Error('no focused xterm helper textarea') + } + const rows: unknown[] = [] + const describe = (node: EventTarget | null): string | null => + node instanceof Element ? `${node.tagName}.${node.className}` : null + + const pushKey = (at: string, event: KeyboardEvent): void => { + rows.push({ + at, + t: event.type, + key: event.key, + code: event.code, + keyCode: event.keyCode, + isComposing: event.isComposing, + meta: event.metaKey, + alt: event.altKey, + defaultPrevented: event.defaultPrevented, + // Why these three: a release that never reaches the renderer and one that reaches a + // different target look identical from a single listener. + hasFocus: document.hasFocus(), + activeElement: describe(document.activeElement), + target: describe(event.target), + textareaConnected: textarea.isConnected, + ts: Math.round(performance.now()) + }) + } + + const positions: [string, EventTarget, boolean][] = [ + ['window-capture', window, true], + ['document-capture', document, true], + ['textarea-capture', textarea, true], + ['window-bubble', window, false] + ] + const keyListeners: (() => void)[] = [] + for (const [name, target, capture] of positions) { + for (const type of ['keydown', 'keyup']) { + const listener = (event: Event): void => pushKey(name, event as KeyboardEvent) + target.addEventListener(type, listener, capture) + keyListeners.push(() => target.removeEventListener(type, listener, capture)) + } + } + + // Focus loss between press and release would route the keyup out of this window entirely. + const focusListeners: (() => void)[] = [] + for (const [name, target, type] of [ + ['window', window, 'blur'], + ['window', window, 'focus'], + ['textarea', textarea, 'blur'], + ['textarea', textarea, 'focus'], + ['document', document, 'visibilitychange'] + ] as [string, EventTarget, string][]) { + const listener = (): void => { + rows.push({ + at: name, + t: type, + hasFocus: document.hasFocus(), + activeElement: describe(document.activeElement), + ts: Math.round(performance.now()) + }) + } + target.addEventListener(type, listener, true) + focusListeners.push(() => target.removeEventListener(type, listener, true)) + } + + for (const type of ['compositionstart', 'compositionupdate', 'compositionend', 'input']) { + const listener = (event: Event): void => { + rows.push({ + at: 'textarea', + t: event.type, + data: (event as CompositionEvent).data ?? null, + value: textarea.value, + ts: Math.round(performance.now()) + }) + } + textarea.addEventListener(type, listener, true) + focusListeners.push(() => textarea.removeEventListener(type, listener, true)) + } + + probeWindow.__chordKeyupProbe = { + rows, + dispose: () => { + for (const off of [...keyListeners, ...focusListeners]) { + off() + } + } + } + }) +} + +function readChordProbe(page: Page): Promise { + return page.evaluate(() => { + const probe = (window as Window & { __chordKeyupProbe?: { rows: unknown[] } }).__chordKeyupProbe + if (!probe) { + throw new Error('chord probe was never installed') + } + return [...probe.rows] + }) +} + +test.describe('Kotoeri chord keyup probe @headful', () => { + test.skip( + process.platform !== 'darwin' || process.env.ORCA_E2E_NATIVE_MACOS_KOREAN !== '1', + 'Requires macOS with native IME access and Accessibility permission' + ) + + test.afterEach(() => { + selectInputSource(TWO_SET_KOREAN_ID) + }) + + test('records where the Cmd+Left release goes while さ is composing', async ({ + electronApp, + orcaPage + }) => { + const processId = electronApp.process().pid + if (processId === undefined) { + throw new Error('Electron process id unavailable') + } + await waitForActiveWorktree(orcaPage) + await waitForSessionReady(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage) + await waitForActivePanePtyId(orcaPage) + await focusActiveTerminalInput(orcaPage) + + enableInputSource(KOTOERI_ROMAJI_PARENT_ID) + enableInputSource(KOTOERI_ROMAJI_ID) + selectInputSource(KOTOERI_ROMAJI_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + + typeKeyCodes(processId, [1, 0]) // s, a -> さ + try { + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 6_000 }) + .toMatch(/[ぁ-ん]/) + } catch { + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + typeKeyCodes(processId, [1, 0]) + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) + .toMatch(/[ぁ-ん]/) + } + + await installChordProbe(orcaPage) + pressChord(processId, KEY.left, 'command') + await orcaPage.waitForTimeout(1_500) + // A bare arrow afterwards shows whether the window still routes keys here at all. + pressChord(processId, KEY.left) + await orcaPage.waitForTimeout(800) + + const rows = await readChordProbe(orcaPage) + const body = `${JSON.stringify(rows, null, 1)}\n` + const dir = path.join(process.cwd(), 'test-results', 'terminal-ime-evidence') + mkdirSync(dir, { recursive: true }) + writeFileSync(path.join(dir, 'kotoeri-chord-keyup-probe.json'), body) + console.log('CHORD_PROBE_ROWS', body) + + typeKeyCodes(processId, [KEY.backspace, KEY.backspace]) + expect(rows.length).toBeGreaterThan(0) + }) + + test('ABC control: whether a Cmd release arrives at all with no IME present', async ({ + electronApp, + orcaPage + }) => { + const processId = electronApp.process().pid + if (processId === undefined) { + throw new Error('Electron process id unavailable') + } + await waitForActiveWorktree(orcaPage) + await waitForSessionReady(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage) + await waitForActivePanePtyId(orcaPage) + await focusActiveTerminalInput(orcaPage) + + selectInputSource(ABC_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + + await installChordProbe(orcaPage) + pressChord(processId, KEY.left, 'command') + await orcaPage.waitForTimeout(800) + pressChord(processId, KEY.left, 'option') + await orcaPage.waitForTimeout(800) + pressChord(processId, KEY.left) + await orcaPage.waitForTimeout(800) + + const rows = await readChordProbe(orcaPage) + const body = `${JSON.stringify(rows, null, 1)}\n` + const dir = path.join(process.cwd(), 'test-results', 'terminal-ime-evidence') + mkdirSync(dir, { recursive: true }) + writeFileSync(path.join(dir, 'abc-chord-keyup-probe.json'), body) + expect(rows.length).toBeGreaterThan(0) + }) + + test('Kotoeri + Option+Left: does the release-driven recovery reach the pty', async ({ + electronApp, + orcaPage, + testRepoPath + }) => { + const processId = electronApp.process().pid + if (processId === undefined) { + throw new Error('Electron process id unavailable') + } + await waitForActiveWorktree(orcaPage) + await waitForSessionReady(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage) + const ptyId = await waitForActivePanePtyId(orcaPage) + await focusActiveTerminalInput(orcaPage) + + enableInputSource(KOTOERI_ROMAJI_PARENT_ID) + enableInputSource(KOTOERI_ROMAJI_ID) + selectInputSource(KOTOERI_ROMAJI_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + + const reader = createTerminalImeByteReader(testRepoPath, 1) + await startTerminalImeByteReader(orcaPage, ptyId, reader) + await focusActiveTerminalInput(orcaPage) + try { + typeKeyCodes(processId, [1, 0]) + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) + .toMatch(/[\u3041-\u3093]/) + + await installChordProbe(orcaPage) + pressChord(processId, KEY.left, 'option') + await orcaPage.waitForTimeout(1_200) + + const rows = await readChordProbe(orcaPage) + mkdirSync(path.join(process.cwd(), 'test-results', 'terminal-ime-evidence'), { + recursive: true + }) + writeFileSync( + path.join( + process.cwd(), + 'test-results', + 'terminal-ime-evidence', + 'kotoeri-option-probe.json' + ), + `${JSON.stringify(rows, null, 1)}\n` + ) + + // A live preedit eats the first Return; the second one flushes the line (same as the + // contributor's flushLineToReader). + pressChord(processId, 36) + let bytes = await waitForTerminalImeBytes(orcaPage, reader, 5_000).catch(() => []) + if (bytes.length === 0) { + pressChord(processId, 36) + bytes = await waitForTerminalImeBytes(orcaPage, reader, 10_000).catch(() => []) + } + console.log('KOTOERI_OPTION_PTY', JSON.stringify(bytes)) + writeFileSync( + path.join( + process.cwd(), + 'test-results', + 'terminal-ime-evidence', + 'kotoeri-option-pty.json' + ), + `${JSON.stringify(bytes)}\n` + ) + expect(rows.length).toBeGreaterThan(0) + } finally { + removeTerminalImeByteReader(reader) + selectInputSource(TWO_SET_KOREAN_ID) + } + }) +}) From 84517d74b268bca888a44b285627ec82ffde1361 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sun, 9 Aug 2026 02:00:00 +0900 Subject: [PATCH 14/15] fix(terminal): end a composing Cmd chord on the Command release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macOS delivers no keyup for a key released while Command is held, so the release-keyed recovery for #12871 could never fire for a Cmd chord. Recorded at Chromium's own input dispatch as well as in the page: Cmd+Left arrives as a keydown with nothing after it, while Option+Left and a bare arrow both deliver their keyup, and defaultPrevented is false throughout — nothing in the app consumes it, it is never made. The Command key's own release does arrive, still marked as composing, and it ends the same gesture. Letting it answer for a pending Cmd chord fixes that half without touching the Option half or the design. Korean cannot double-fire: it commits on the chord, so its arrow keyup arrives first and spends the carry, and both releases report the composition already over. Earlier captures missed this because System Events folds a modifier into the target key's flags and never produces its own press or release. The new probe posts it as its own key event, the way a hand types it, and the contributor's native spec now drives its chords the same way. Verified in a dev build: the pty receives e38195010a — さ, \x01, newline. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Mu6j33LNsd4Mvv8a1NuFoo --- ...lers.issue-12871-command-release-traces.ts | 76 +++++ ....issue-12871-recorded-chord-traces.test.ts | 28 +- .../terminal-pane/keyboard-handlers.ts | 16 +- tests/e2e/macos-input-source-driver.ts | 93 ++++++ tests/e2e/main-process-input-event-probe.ts | 84 ++++++ tests/e2e/post-modifier-chord.swift | 42 +++ tests/e2e/renderer-chord-event-probe.ts | 123 ++++++++ ...l-macos-chord-input-pipeline-probe.spec.ts | 277 ++++++++++++++++++ ...inal-macos-ime-cursor-chord-native.spec.ts | 18 +- ...al-macos-kotoeri-chord-keyup-probe.spec.ts | 201 ++----------- 10 files changed, 757 insertions(+), 201 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts create mode 100644 tests/e2e/macos-input-source-driver.ts create mode 100644 tests/e2e/main-process-input-event-probe.ts create mode 100644 tests/e2e/post-modifier-chord.swift create mode 100644 tests/e2e/renderer-chord-event-probe.ts create mode 100644 tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts new file mode 100644 index 000000000000..a25d3260bfb8 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts @@ -0,0 +1,76 @@ +// Recorded IME chord traces for #12871, taken inside a dev e2e Orca build (app commit +// 4887d924ca81c1518481dd2d9e798b6b08e74cc8, macOS 26.5.1/25F80, darwin arm64) at the xterm +// helper textarea, driven through the real OS input methods. Replayed by +// keyboard-handlers.issue-12871-recorded-chord-traces.test.ts through the same rig as the other +// recordings. Reproduce with tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts. +// +// What is new here is the driver: these were posted as CGEvents with the modifier as its own +// key event, the way a hand types it. System Events folds the modifier into the target key's +// flags instead, which is why no earlier recording contains a modifier press or release at all +// and why the Cmd half of the gesture looked like it had no end. +// +// With that end recorded, the two input sources separate on `Cmd` exactly as they already did on +// `Option`, and the same recordings show why the arrow's own release cannot carry the decision: +// +// - Kotoeri swallows the chord and keeps composing. No arrow keyup is ever delivered, at any +// listener position or at Chromium's own input dispatch. The `Cmd` release is the only +// event that marks the gesture's end, and it still reports the composition live. +// - Korean 2-Set commits on the chord. Its arrow keyup does arrive, after compositionend and +// with `isComposing` false, so it spends the carry without firing and the `Cmd` release that +// follows finds nothing armed. Two independent reasons the committing source stays silent. +import type { InAppRecordedCase } from './keyboard-handlers.issue-12871-in-app-chord-traces' + +export const COMMAND_RELEASE_TRACE_CASES: InAppRecordedCase[] = [ + { + name: 'Japanese, Cmd+ArrowLeft over a live さ preedit, ended by the Command release', + expectCalls: ['\x01'], + expectEmitted: ['\x01'], + rows: [ + { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + meta: true + }, + // No arrow keyup between these two rows. That absence is the recording's whole point. + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: false } + ] + }, + { + // The platform's unmarked replay is absent from these rows for the same reason it is absent + // from the other in-app recordings: the app's own window-level capture consumed it above the + // probe. So this case pins the half it can speak for, which is the half a release-keyed + // recovery can break — that neither release fires anything on a committing input source. + name: 'Korean 2-Set, Cmd+ArrowLeft commits the syllable and neither release fires', + expectCalls: [], + // xterm's own commit of the preedit, not the handler's doing — `expectCalls` staying empty + // is what says the handler kept out of it. + expectEmitted: ['ㄴ'], + rows: [ + { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + meta: true + }, + { t: 'compositionupdate', data: 'ㄴ', value: 'ㄴ' }, + { t: 'input', data: 'ㄴ', value: 'ㄴ' }, + { t: 'compositionend', data: 'ㄴ', value: 'ㄴ' }, + { + t: 'keyup', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 37, + isComposing: false, + meta: true + }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: false, meta: false } + ] + } +] diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index d57c6c99e161..5ef1259bc274 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -27,12 +27,19 @@ // bare page (provenance and per-case notes in keyboard-handlers.issue-12871-in-app-chord-traces // .ts). It is replayed through the same rig below. The two surfaces captured different subsets // of the same gestures, so each case there states which half of the contract it can speak for. +// +// COMMAND_RELEASE_TRACE_CASES is a third, and the one that covers the Cmd half. Both recordings +// above were driven with the modifier folded into the target key's flags, which produces no +// modifier press or release at all; these were driven with it as its own key event, the way a +// hand types it. That is what exposed the asymmetry between the two modifiers — a key released +// while Cmd is held has no keyup on macOS, so the Cmd release is the gesture's only end. import { Terminal } from '@xterm/xterm' import { cleanup, renderHook } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { PtyTransport } from './pty-transport' import { useTerminalKeyboardShortcuts } from './keyboard-handlers' +import { COMMAND_RELEASE_TRACE_CASES } from './keyboard-handlers.issue-12871-command-release-traces' import { IN_APP_TRACE_CASES } from './keyboard-handlers.issue-12871-in-app-chord-traces' type RecordedRow = { @@ -434,17 +441,18 @@ describe('recorded macOS chord traces during an IME composition', () => { document.body.replaceChildren() }) - it.each([...CASES, ...IN_APP_TRACE_CASES].map((testCase) => [testCase.name, testCase] as const))( - '%s', - async (_name, testCase) => { - const rig = openRig() - await replay(rig.textarea, testCase.rows) + it.each( + [...CASES, ...IN_APP_TRACE_CASES, ...COMMAND_RELEASE_TRACE_CASES].map( + (testCase) => [testCase.name, testCase] as const + ) + )('%s', async (_name, testCase) => { + const rig = openRig() + await replay(rig.textarea, testCase.rows) - expect(rig.inputCalls).toEqual(testCase.expectCalls) - expect(rig.emitted).toEqual(testCase.expectEmitted) - rig.unmount() - } - ) + expect(rig.inputCalls).toEqual(testCase.expectCalls) + expect(rig.emitted).toEqual(testCase.expectEmitted) + rig.unmount() + }) // A chord remapped onto one of these keys resolves to a pane command rather than bytes, so // the swallowed-chord release has to reach every action, not just the byte path. Recovering diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index aaa3eb86b9b4..ec14d0b94d70 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -104,6 +104,20 @@ function imeChordSnapshot(event: KeyboardEvent): PendingImeChord { } } +/** + * Which release answers for a pending chord. Normally the chord's own key, but a key released + * while `Cmd` is held has no keyup at all: recorded in-app at Chromium's own input dispatch, + * `Cmd+←` delivers a keydown and nothing after it, while `Option+←` and a bare `←` both deliver + * their keyup there. Nothing in the app consumes it — it never arrives. The `Cmd` release does, + * still marked as composing, and it ends the same gesture. + * + * Korean cannot double-fire through this. It commits on the chord, so its own keyup arrives + * first and spends the carry, and both releases report the composition already over. + */ +function releasesPendingImeChord(chord: PendingImeChord, event: KeyboardEvent): boolean { + return chord.code === event.code || (chord.metaKey === true && event.key === 'Meta') +} + /** * Note the asymmetry with the pane's own keydown handler, which yields on *every* IME-owned * event. This resolver deliberately lets an exempt chord through, because the pane also calls @@ -736,7 +750,7 @@ export function useTerminalKeyboardShortcuts({ const onSwallowedImeChordRelease = (e: KeyboardEvent): void => { const chord = pendingImeChord - if (!chord || chord.code !== e.code) { + if (!chord || !releasesPendingImeChord(chord, e)) { return } // Spent before any gate below can refuse: the key is up, so the press it carried is over diff --git a/tests/e2e/macos-input-source-driver.ts b/tests/e2e/macos-input-source-driver.ts new file mode 100644 index 000000000000..13d4cd941806 --- /dev/null +++ b/tests/e2e/macos-input-source-driver.ts @@ -0,0 +1,93 @@ +import { execFileSync } from 'node:child_process' +import path from 'node:path' + +/** + * Drives real macOS input sources and synthesized key events for the #12871 captures. Playwright's + * own keyboard bypasses the IME entirely, so these go through TIS and System Events instead. + */ + +export const TWO_SET_KOREAN_ID = 'com.apple.inputmethod.Korean.2SetKorean' +export const KOTOERI_ROMAJI_ID = 'com.apple.inputmethod.Kotoeri.RomajiTyping.Japanese' +export const KOTOERI_ROMAJI_PARENT_ID = 'com.apple.inputmethod.Kotoeri.RomajiTyping' +export const ABC_ID = 'com.apple.keylayout.ABC' + +export const KEY = { left: 123, backspace: 51, returnKey: 36, s: 1, a: 0 } as const +export const MODIFIER_KEY = { command: 55, option: 58 } as const + +const SELECT_INPUT_SOURCE = path.resolve(__dirname, 'select-input-source.swift') +const POST_MODIFIER_CHORD = path.resolve(__dirname, 'post-modifier-chord.swift') + +export function selectInputSource(id: string): void { + execFileSync('swift', [SELECT_INPUT_SOURCE, id]) +} + +export function enableInputSource(id: string): void { + execFileSync('swift', ['-'], { + input: ` +import Carbon +let properties = [kTISPropertyInputSourceID: ${JSON.stringify(id)} as CFString] as CFDictionary +let sources = TISCreateInputSourceList(properties, true).takeRetainedValue() as! [TISInputSource] +guard !sources.isEmpty else { exit(3) } +for candidate in sources { TISEnableInputSource(candidate) } +exit(0) +` + }) +} + +export function focusApp(processId: number): void { + execFileSync('osascript', [ + '-e', + `tell application "System Events" to set frontmost of first application process whose unix id is ${processId} to true`, + '-e', + 'delay 0.3' + ]) +} + +/** Switching the input source only takes effect on the next activation. */ +export function bounceFocus(processId: number): void { + execFileSync('osascript', ['-e', 'tell application "Finder" to activate', '-e', 'delay 0.4']) + focusApp(processId) +} + +export function typeKeyCodes(processId: number, keyCodes: readonly number[]): void { + focusApp(processId) + execFileSync('osascript', [ + '-e', + 'tell application "System Events"', + '-e', + `repeat with currentKeyCode in {${keyCodes.join(', ')}}`, + '-e', + 'key code (currentKeyCode as integer)', + '-e', + 'delay 0.12', + '-e', + 'end repeat', + '-e', + 'end tell' + ]) +} + +/** + * Types the chord with the modifier as its own key event. System Events folds the modifier into + * the target key's flags instead, so its press and release never reach the app at all. + */ +export function pressChordWithSeparateModifier( + processId: number, + keyCode: number, + modifier: 'command' | 'option' +): void { + focusApp(processId) + execFileSync('swift', [POST_MODIFIER_CHORD, String(MODIFIER_KEY[modifier]), String(keyCode)]) +} + +export function pressChord( + processId: number, + keyCode: number, + modifier?: 'command' | 'option' +): void { + focusApp(processId) + execFileSync('osascript', [ + '-e', + `tell application "System Events" to key code ${keyCode}${modifier ? ` using ${modifier} down` : ''}` + ]) +} diff --git a/tests/e2e/main-process-input-event-probe.ts b/tests/e2e/main-process-input-event-probe.ts new file mode 100644 index 000000000000..3de6b6a5fc99 --- /dev/null +++ b/tests/e2e/main-process-input-event-probe.ts @@ -0,0 +1,84 @@ +import type { ElectronApplication } from '@stablyai/playwright-test' + +/** + * Records what Chromium itself delivers, before the page sees it. The renderer probe can only say + * "no keyup arrived here"; this says whether one arrived at all, which separates a release lost at + * the OS boundary from one consumed inside the app. + */ + +export type MainProcessInputRow = { + t: string + key: string + code: string + meta: boolean + alt: boolean + control: boolean + shift: boolean + isAutoRepeat: boolean + /** Read after Orca's own handler ran, since this listener registers last. */ + defaultPrevented: boolean + ts: number +} + +export async function installMainProcessInputProbe( + electronApp: ElectronApplication +): Promise { + await electronApp.evaluate(({ BrowserWindow }) => { + type ProbeGlobal = typeof globalThis & { + __mainInputProbe?: { rows: unknown[]; dispose: () => void } + } + const probeGlobal = globalThis as ProbeGlobal + probeGlobal.__mainInputProbe?.dispose() + + const mainWindow = BrowserWindow.getAllWindows()[0] + if (!mainWindow) { + throw new Error('No BrowserWindow available') + } + const rows: unknown[] = [] + const listener = (event: { defaultPrevented?: boolean }, input: Electron.Input): void => { + rows.push({ + t: input.type, + key: input.key, + code: input.code, + meta: input.meta, + alt: input.alt, + control: input.control, + shift: input.shift, + isAutoRepeat: input.isAutoRepeat, + defaultPrevented: event.defaultPrevented === true, + ts: Date.now() + }) + } + mainWindow.webContents.on('before-input-event', listener) + probeGlobal.__mainInputProbe = { + rows, + dispose: () => mainWindow.webContents.off('before-input-event', listener) + } + }) +} + +export async function readMainProcessInputProbe( + electronApp: ElectronApplication +): Promise { + const rows = await electronApp.evaluate(() => { + const probe = (globalThis as typeof globalThis & { __mainInputProbe?: { rows: unknown[] } }) + .__mainInputProbe + if (!probe) { + throw new Error('main-process input probe was never installed') + } + return [...probe.rows] + }) + return rows as MainProcessInputRow[] +} + +export async function disposeMainProcessInputProbe( + electronApp: ElectronApplication +): Promise { + await electronApp.evaluate(() => { + const probeGlobal = globalThis as typeof globalThis & { + __mainInputProbe?: { dispose: () => void } + } + probeGlobal.__mainInputProbe?.dispose() + probeGlobal.__mainInputProbe = undefined + }) +} diff --git a/tests/e2e/post-modifier-chord.swift b/tests/e2e/post-modifier-chord.swift new file mode 100644 index 000000000000..9eb2fcc71e03 --- /dev/null +++ b/tests/e2e/post-modifier-chord.swift @@ -0,0 +1,42 @@ +// Posts a chord the way a human types it: the modifier as its own key event, not as a flag +// System Events folds into the character key. Only this form produces the modifier's own +// press/release, which is what a release-driven recovery would key off. +// Usage: swift post-modifier-chord.swift + +import CoreGraphics +import Foundation + +let arguments = CommandLine.arguments +guard arguments.count >= 3, + let modifierKey = CGKeyCode(arguments[1]), + let targetKey = CGKeyCode(arguments[2]) +else { + FileHandle.standardError.write( + Data("usage: post-modifier-chord.swift \n".utf8)) + exit(2) +} + +let modifierFlag: CGEventFlags = + switch modifierKey { + case 55, 54: .maskCommand + case 58, 61: .maskAlternate + case 59, 62: .maskControl + default: [] + } + +let source = CGEventSource(stateID: .hidSystemState) + +func post(_ key: CGKeyCode, down: Bool, flags: CGEventFlags) { + guard let event = CGEvent(keyboardEventSource: source, virtualKey: key, keyDown: down) else { + exit(3) + } + event.flags = flags + event.post(tap: .cghidEventTap) + usleep(80_000) +} + +post(modifierKey, down: true, flags: modifierFlag) +post(targetKey, down: true, flags: modifierFlag) +post(targetKey, down: false, flags: modifierFlag) +post(modifierKey, down: false, flags: []) +exit(0) diff --git a/tests/e2e/renderer-chord-event-probe.ts b/tests/e2e/renderer-chord-event-probe.ts new file mode 100644 index 000000000000..da7f0caa3c97 --- /dev/null +++ b/tests/e2e/renderer-chord-event-probe.ts @@ -0,0 +1,123 @@ +import type { Page } from '@stablyai/playwright-test' + +/** + * Renderer-side listeners for the #12871 captures. Four positions plus focus ownership, so a + * release that never reaches the renderer can be told apart from one delivered somewhere else. + */ + +export function readActiveComposition(page: Page): Promise { + return page.evaluate(() => { + const textarea = document.querySelector('.xterm-helper-textarea:focus') + const composition = textarea?.parentElement?.querySelector( + '.composition-view.active' + ) + return composition?.textContent?.replaceAll('‎', '') ?? null + }) +} + +export async function installChordProbe(page: Page): Promise { + await page.evaluate(() => { + type ProbeWindow = Window & { __chordKeyupProbe?: { rows: unknown[]; dispose: () => void } } + const probeWindow = window as ProbeWindow + probeWindow.__chordKeyupProbe?.dispose() + + const textarea = document.querySelector('.xterm-helper-textarea:focus') + if (!textarea) { + throw new Error('no focused xterm helper textarea') + } + const rows: unknown[] = [] + const describe = (node: EventTarget | null): string | null => + node instanceof Element ? `${node.tagName}.${node.className}` : null + + const pushKey = (at: string, event: KeyboardEvent): void => { + rows.push({ + at, + t: event.type, + key: event.key, + code: event.code, + keyCode: event.keyCode, + isComposing: event.isComposing, + meta: event.metaKey, + alt: event.altKey, + defaultPrevented: event.defaultPrevented, + // Why these three: a release that never reaches the renderer and one that reaches a + // different target look identical from a single listener. + hasFocus: document.hasFocus(), + activeElement: describe(document.activeElement), + target: describe(event.target), + textareaConnected: textarea.isConnected, + ts: Math.round(performance.now()) + }) + } + + const positions: [string, EventTarget, boolean][] = [ + ['window-capture', window, true], + ['document-capture', document, true], + ['textarea-capture', textarea, true], + ['window-bubble', window, false] + ] + const keyListeners: (() => void)[] = [] + for (const [name, target, capture] of positions) { + for (const type of ['keydown', 'keyup']) { + const listener = (event: Event): void => pushKey(name, event as KeyboardEvent) + target.addEventListener(type, listener, capture) + keyListeners.push(() => target.removeEventListener(type, listener, capture)) + } + } + + // Focus loss between press and release would route the keyup out of this window entirely. + const focusListeners: (() => void)[] = [] + for (const [name, target, type] of [ + ['window', window, 'blur'], + ['window', window, 'focus'], + ['textarea', textarea, 'blur'], + ['textarea', textarea, 'focus'], + ['document', document, 'visibilitychange'] + ] as [string, EventTarget, string][]) { + const listener = (): void => { + rows.push({ + at: name, + t: type, + hasFocus: document.hasFocus(), + activeElement: describe(document.activeElement), + ts: Math.round(performance.now()) + }) + } + target.addEventListener(type, listener, true) + focusListeners.push(() => target.removeEventListener(type, listener, true)) + } + + for (const type of ['compositionstart', 'compositionupdate', 'compositionend', 'input']) { + const listener = (event: Event): void => { + rows.push({ + at: 'textarea', + t: event.type, + data: (event as CompositionEvent).data ?? null, + value: textarea.value, + ts: Math.round(performance.now()) + }) + } + textarea.addEventListener(type, listener, true) + focusListeners.push(() => textarea.removeEventListener(type, listener, true)) + } + + probeWindow.__chordKeyupProbe = { + rows, + dispose: () => { + for (const off of [...keyListeners, ...focusListeners]) { + off() + } + } + } + }) +} + +export function readChordProbe(page: Page): Promise { + return page.evaluate(() => { + const probe = (window as Window & { __chordKeyupProbe?: { rows: unknown[] } }).__chordKeyupProbe + if (!probe) { + throw new Error('chord probe was never installed') + } + return [...probe.rows] + }) +} diff --git a/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts b/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts new file mode 100644 index 000000000000..6ef06ee9f580 --- /dev/null +++ b/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts @@ -0,0 +1,277 @@ +import { mkdirSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import type { ElectronApplication, Page } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + focusActiveTerminalInput, + waitForActivePanePtyId, + waitForActiveTerminalManager +} from './helpers/terminal' +import { + disposeMainProcessInputProbe, + installMainProcessInputProbe, + readMainProcessInputProbe +} from './main-process-input-event-probe' +import { + ABC_ID, + bounceFocus, + enableInputSource, + KEY, + KOTOERI_ROMAJI_ID, + KOTOERI_ROMAJI_PARENT_ID, + pressChord, + pressChordWithSeparateModifier, + selectInputSource, + TWO_SET_KOREAN_ID, + typeKeyCodes +} from './macos-input-source-driver' +import { + installChordProbe, + readActiveComposition, + readChordProbe +} from './renderer-chord-event-probe' +import { + createTerminalImeByteReader, + removeTerminalImeByteReader, + startTerminalImeByteReader, + waitForTerminalImeBytes +} from './terminal-ime-byte-reader' + +/** + * #12871 capture, not a gate. The renderer probe established that a Cmd chord's keyup never + * reaches the page. It cannot say why. This records the same gesture one layer lower, at + * Chromium's own input dispatch, so three candidates separate: the release never leaves the OS, + * Orca's main-process handler consumes it, or it is dropped between Chromium and the page. + * + * The modifier's own release is recorded too, since a Meta keyup at this layer is what a + * modifier-release-driven recovery would have to key off. + */ + +const EVIDENCE_DIR = path.join(process.cwd(), 'test-results', 'terminal-ime-evidence') + +function writeEvidence(name: string, value: unknown): void { + mkdirSync(EVIDENCE_DIR, { recursive: true }) + writeFileSync(path.join(EVIDENCE_DIR, name), `${JSON.stringify(value, null, 1)}\n`) +} + +async function openFocusedTerminal(page: Page): Promise { + await waitForActiveWorktree(page) + await waitForSessionReady(page) + await ensureTerminalVisible(page) + await waitForActiveTerminalManager(page) + await waitForActivePanePtyId(page) + await focusActiveTerminalInput(page) +} + +function requireProcessId(electronApp: ElectronApplication): number { + const processId = electronApp.process().pid + if (processId === undefined) { + throw new Error('Electron process id unavailable') + } + return processId +} + +test.describe('macOS chord input pipeline probe @headful', () => { + test.skip( + process.platform !== 'darwin' || process.env.ORCA_E2E_NATIVE_MACOS_KOREAN !== '1', + 'Requires macOS with native IME access and Accessibility permission' + ) + + test.afterEach(async ({ electronApp }) => { + await disposeMainProcessInputProbe(electronApp).catch(() => {}) + selectInputSource(TWO_SET_KOREAN_ID) + }) + + test('ABC control: which layer still has the Cmd+Left release', async ({ + electronApp, + orcaPage + }) => { + const processId = requireProcessId(electronApp) + await openFocusedTerminal(orcaPage) + + selectInputSource(ABC_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + + await installMainProcessInputProbe(electronApp) + await installChordProbe(orcaPage) + pressChord(processId, KEY.left, 'command') + await orcaPage.waitForTimeout(800) + pressChord(processId, KEY.left, 'option') + await orcaPage.waitForTimeout(800) + pressChord(processId, KEY.left) + await orcaPage.waitForTimeout(800) + + const mainRows = await readMainProcessInputProbe(electronApp) + const rendererRows = await readChordProbe(orcaPage) + writeEvidence('abc-chord-pipeline-main.json', mainRows) + writeEvidence('abc-chord-pipeline-renderer.json', rendererRows) + console.log('ABC_MAIN_ROWS', JSON.stringify(mainRows)) + + expect(mainRows.length).toBeGreaterThan(0) + }) + + test('Kotoeri: which layer still has the Cmd+Left release while さ is composing', async ({ + electronApp, + orcaPage + }) => { + const processId = requireProcessId(electronApp) + await openFocusedTerminal(orcaPage) + + enableInputSource(KOTOERI_ROMAJI_PARENT_ID) + enableInputSource(KOTOERI_ROMAJI_ID) + selectInputSource(KOTOERI_ROMAJI_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + + typeKeyCodes(processId, [KEY.s, KEY.a]) + try { + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 6_000 }) + .toMatch(/[ぁ-ん]/) + } catch { + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + typeKeyCodes(processId, [KEY.s, KEY.a]) + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) + .toMatch(/[ぁ-ん]/) + } + + await installMainProcessInputProbe(electronApp) + await installChordProbe(orcaPage) + pressChord(processId, KEY.left, 'command') + await orcaPage.waitForTimeout(1_500) + // A bare arrow afterwards shows the window still routes keys here at all. + pressChord(processId, KEY.left) + await orcaPage.waitForTimeout(800) + + const mainRows = await readMainProcessInputProbe(electronApp) + const rendererRows = await readChordProbe(orcaPage) + writeEvidence('kotoeri-chord-pipeline-main.json', mainRows) + writeEvidence('kotoeri-chord-pipeline-renderer.json', rendererRows) + console.log('KOTOERI_MAIN_ROWS', JSON.stringify(mainRows)) + + typeKeyCodes(processId, [KEY.backspace, KEY.backspace]) + expect(mainRows.length).toBeGreaterThan(0) + }) + + test('Command released as its own key: is that release delivered', async ({ + electronApp, + orcaPage + }) => { + const processId = requireProcessId(electronApp) + await openFocusedTerminal(orcaPage) + + enableInputSource(KOTOERI_ROMAJI_PARENT_ID) + enableInputSource(KOTOERI_ROMAJI_ID) + selectInputSource(KOTOERI_ROMAJI_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + + typeKeyCodes(processId, [KEY.s, KEY.a]) + await expect.poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }).toMatch(/[ぁ-ん]/) + + await installMainProcessInputProbe(electronApp) + await installChordProbe(orcaPage) + pressChordWithSeparateModifier(processId, KEY.left, 'command') + await orcaPage.waitForTimeout(1_500) + + const mainRows = await readMainProcessInputProbe(electronApp) + const rendererRows = await readChordProbe(orcaPage) + writeEvidence('kotoeri-separate-modifier-main.json', mainRows) + writeEvidence('kotoeri-separate-modifier-renderer.json', rendererRows) + console.log('SEPARATE_MODIFIER_MAIN_ROWS', JSON.stringify(mainRows)) + + typeKeyCodes(processId, [KEY.backspace, KEY.backspace]) + expect(mainRows.length).toBeGreaterThan(0) + }) + + // Why Korean matters here: it commits on the chord and the platform replays the chord unmarked. + // Keying recovery off the Command release would fire twice if that release still reported a + // live composition, so the ordering of compositionend against it is what makes it safe. + test('Korean: where compositionend falls against the Command release', async ({ + electronApp, + orcaPage + }) => { + const processId = requireProcessId(electronApp) + await openFocusedTerminal(orcaPage) + + selectInputSource(TWO_SET_KOREAN_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + + typeKeyCodes(processId, [KEY.s]) + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) + .toMatch(/[ㄱ-ㅣ가-힣]/) + + await installMainProcessInputProbe(electronApp) + await installChordProbe(orcaPage) + pressChordWithSeparateModifier(processId, KEY.left, 'command') + await orcaPage.waitForTimeout(1_500) + + const mainRows = await readMainProcessInputProbe(electronApp) + const rendererRows = await readChordProbe(orcaPage) + writeEvidence('korean-separate-modifier-main.json', mainRows) + writeEvidence('korean-separate-modifier-renderer.json', rendererRows) + console.log('KOREAN_MAIN_ROWS', JSON.stringify(mainRows)) + + typeKeyCodes(processId, [KEY.backspace, KEY.backspace]) + expect(rendererRows.length).toBeGreaterThan(0) + }) + + // The one assertion in this file rather than a recording: the release-keyed recovery has to + // reach the shell, not merely resolve. Driven as a hand types it, so the Command release the + // recovery keys off actually exists. + test('Kotoeri + Cmd+Left reaches the pty as さ then line start', async ({ + electronApp, + orcaPage, + testRepoPath + }) => { + const processId = requireProcessId(electronApp) + await waitForActiveWorktree(orcaPage) + await waitForSessionReady(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage) + const ptyId = await waitForActivePanePtyId(orcaPage) + await focusActiveTerminalInput(orcaPage) + + enableInputSource(KOTOERI_ROMAJI_PARENT_ID) + enableInputSource(KOTOERI_ROMAJI_ID) + selectInputSource(KOTOERI_ROMAJI_ID) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + + const reader = createTerminalImeByteReader(testRepoPath, 1) + await startTerminalImeByteReader(orcaPage, ptyId, reader) + await focusActiveTerminalInput(orcaPage) + try { + typeKeyCodes(processId, [KEY.s, KEY.a]) + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) + .toMatch(/[ぁ-ん]/) + + pressChordWithSeparateModifier(processId, KEY.left, 'command') + await orcaPage.waitForTimeout(1_200) + + // A live preedit eats the first Return; the second flushes the line. + pressChord(processId, KEY.returnKey) + let bytes = await waitForTerminalImeBytes(orcaPage, reader, 5_000).catch(() => []) + if (bytes.length === 0) { + pressChord(processId, KEY.returnKey) + bytes = await waitForTerminalImeBytes(orcaPage, reader, 10_000).catch(() => []) + } + writeEvidence('kotoeri-command-release-pty.json', bytes) + console.log('KOTOERI_COMMAND_PTY', JSON.stringify(bytes)) + + // さ then \x01, in that order: xterm queues the chord behind the preedit and flushes both + // on commit, so the chord cannot land ahead of the text being composed. + expect(bytes.join('')).toContain('e3819501') + } finally { + removeTerminalImeByteReader(reader) + selectInputSource(TWO_SET_KOREAN_ID) + } + }) +}) diff --git a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts index f33209c55007..8f04318fba27 100644 --- a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts +++ b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts @@ -3,6 +3,7 @@ import path from 'node:path' import type { Page } from '@stablyai/playwright-test' import { expect, test } from './helpers/orca-app' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { pressChordWithSeparateModifier } from './macos-input-source-driver' import { focusActiveTerminalInput, getTerminalContent, @@ -107,14 +108,19 @@ function typeKeyCodes(processId: number, keyCodes: readonly number[]): void { ]) } -/** The chord itself, pressed by the OS so the IME decides how to resolve it. */ +/** + * The chord itself, pressed by the OS so the IME decides how to resolve it. A modifier goes as + * its own key event rather than folded into the target key's flags, because macOS delivers no + * keyup for a key released while Command is held: the modifier's release is the only end that + * gesture has, and `using command down` never produces one. + */ function pressChord(processId: number, keyCode: number, modifier?: 'command' | 'option'): void { + if (modifier) { + pressChordWithSeparateModifier(processId, keyCode, modifier) + return + } focusApp(processId) - const using = modifier ? ` using ${modifier} down` : '' - execFileSync('osascript', [ - '-e', - `tell application "System Events" to key code ${keyCode}${using}` - ]) + execFileSync('osascript', ['-e', `tell application "System Events" to key code ${keyCode}`]) } function readActiveComposition(page: Page): Promise { diff --git a/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts b/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts index 302e262a8167..1c5234409863 100644 --- a/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts +++ b/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts @@ -1,7 +1,5 @@ -import { execFileSync } from 'node:child_process' import { mkdirSync, writeFileSync } from 'node:fs' import path from 'node:path' -import type { Page } from '@stablyai/playwright-test' import { expect, test } from './helpers/orca-app' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' import { @@ -9,6 +7,23 @@ import { waitForActivePanePtyId, waitForActiveTerminalManager } from './helpers/terminal' +import { + ABC_ID, + bounceFocus, + enableInputSource, + KEY, + KOTOERI_ROMAJI_ID, + KOTOERI_ROMAJI_PARENT_ID, + pressChord, + selectInputSource, + TWO_SET_KOREAN_ID, + typeKeyCodes +} from './macos-input-source-driver' +import { + installChordProbe, + readActiveComposition, + readChordProbe +} from './renderer-chord-event-probe' import { createTerminalImeByteReader, removeTerminalImeByteReader, @@ -24,188 +39,6 @@ import { * told apart from "dispatched somewhere else". */ -const TWO_SET_KOREAN_ID = 'com.apple.inputmethod.Korean.2SetKorean' -const KOTOERI_ROMAJI_ID = 'com.apple.inputmethod.Kotoeri.RomajiTyping.Japanese' -const KOTOERI_ROMAJI_PARENT_ID = 'com.apple.inputmethod.Kotoeri.RomajiTyping' -const ABC_ID = 'com.apple.keylayout.ABC' -const SELECT_INPUT_SOURCE = path.resolve(__dirname, 'select-input-source.swift') -const KEY = { left: 123, backspace: 51 } as const - -function selectInputSource(id: string): void { - execFileSync('swift', [SELECT_INPUT_SOURCE, id]) -} - -function enableInputSource(id: string): void { - execFileSync('swift', ['-'], { - input: ` -import Carbon -let properties = [kTISPropertyInputSourceID: ${JSON.stringify(id)} as CFString] as CFDictionary -let sources = TISCreateInputSourceList(properties, true).takeRetainedValue() as! [TISInputSource] -guard !sources.isEmpty else { exit(3) } -for candidate in sources { TISEnableInputSource(candidate) } -exit(0) -` - }) -} - -function focusApp(processId: number): void { - execFileSync('osascript', [ - '-e', - `tell application "System Events" to set frontmost of first application process whose unix id is ${processId} to true`, - '-e', - 'delay 0.3' - ]) -} - -function bounceFocus(processId: number): void { - execFileSync('osascript', ['-e', 'tell application "Finder" to activate', '-e', 'delay 0.4']) - focusApp(processId) -} - -function typeKeyCodes(processId: number, keyCodes: readonly number[]): void { - focusApp(processId) - execFileSync('osascript', [ - '-e', - 'tell application "System Events"', - '-e', - `repeat with currentKeyCode in {${keyCodes.join(', ')}}`, - '-e', - 'key code (currentKeyCode as integer)', - '-e', - 'delay 0.12', - '-e', - 'end repeat', - '-e', - 'end tell' - ]) -} - -function pressChord(processId: number, keyCode: number, modifier?: 'command' | 'option'): void { - focusApp(processId) - execFileSync('osascript', [ - '-e', - `tell application "System Events" to key code ${keyCode}${modifier ? ` using ${modifier} down` : ''}` - ]) -} - -function readActiveComposition(page: Page): Promise { - return page.evaluate(() => { - const textarea = document.querySelector('.xterm-helper-textarea:focus') - const composition = textarea?.parentElement?.querySelector( - '.composition-view.active' - ) - return composition?.textContent?.replaceAll('\u200e', '') ?? null - }) -} - -/** Four listener positions plus focus ownership, so a missing release can be located. */ -async function installChordProbe(page: Page): Promise { - await page.evaluate(() => { - type ProbeWindow = Window & { __chordKeyupProbe?: { rows: unknown[]; dispose: () => void } } - const probeWindow = window as ProbeWindow - probeWindow.__chordKeyupProbe?.dispose() - - const textarea = document.querySelector('.xterm-helper-textarea:focus') - if (!textarea) { - throw new Error('no focused xterm helper textarea') - } - const rows: unknown[] = [] - const describe = (node: EventTarget | null): string | null => - node instanceof Element ? `${node.tagName}.${node.className}` : null - - const pushKey = (at: string, event: KeyboardEvent): void => { - rows.push({ - at, - t: event.type, - key: event.key, - code: event.code, - keyCode: event.keyCode, - isComposing: event.isComposing, - meta: event.metaKey, - alt: event.altKey, - defaultPrevented: event.defaultPrevented, - // Why these three: a release that never reaches the renderer and one that reaches a - // different target look identical from a single listener. - hasFocus: document.hasFocus(), - activeElement: describe(document.activeElement), - target: describe(event.target), - textareaConnected: textarea.isConnected, - ts: Math.round(performance.now()) - }) - } - - const positions: [string, EventTarget, boolean][] = [ - ['window-capture', window, true], - ['document-capture', document, true], - ['textarea-capture', textarea, true], - ['window-bubble', window, false] - ] - const keyListeners: (() => void)[] = [] - for (const [name, target, capture] of positions) { - for (const type of ['keydown', 'keyup']) { - const listener = (event: Event): void => pushKey(name, event as KeyboardEvent) - target.addEventListener(type, listener, capture) - keyListeners.push(() => target.removeEventListener(type, listener, capture)) - } - } - - // Focus loss between press and release would route the keyup out of this window entirely. - const focusListeners: (() => void)[] = [] - for (const [name, target, type] of [ - ['window', window, 'blur'], - ['window', window, 'focus'], - ['textarea', textarea, 'blur'], - ['textarea', textarea, 'focus'], - ['document', document, 'visibilitychange'] - ] as [string, EventTarget, string][]) { - const listener = (): void => { - rows.push({ - at: name, - t: type, - hasFocus: document.hasFocus(), - activeElement: describe(document.activeElement), - ts: Math.round(performance.now()) - }) - } - target.addEventListener(type, listener, true) - focusListeners.push(() => target.removeEventListener(type, listener, true)) - } - - for (const type of ['compositionstart', 'compositionupdate', 'compositionend', 'input']) { - const listener = (event: Event): void => { - rows.push({ - at: 'textarea', - t: event.type, - data: (event as CompositionEvent).data ?? null, - value: textarea.value, - ts: Math.round(performance.now()) - }) - } - textarea.addEventListener(type, listener, true) - focusListeners.push(() => textarea.removeEventListener(type, listener, true)) - } - - probeWindow.__chordKeyupProbe = { - rows, - dispose: () => { - for (const off of [...keyListeners, ...focusListeners]) { - off() - } - } - } - }) -} - -function readChordProbe(page: Page): Promise { - return page.evaluate(() => { - const probe = (window as Window & { __chordKeyupProbe?: { rows: unknown[] } }).__chordKeyupProbe - if (!probe) { - throw new Error('chord probe was never installed') - } - return [...probe.rows] - }) -} - test.describe('Kotoeri chord keyup probe @headful', () => { test.skip( process.platform !== 'darwin' || process.env.ORCA_E2E_NATIVE_MACOS_KOREAN !== '1', From a61583be1862fa13a019fe50e4bfe9f3900b9d0d Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 15 Aug 2026 00:27:24 +0900 Subject: [PATCH 15/15] fix(terminal): hold a swallowed IME chord until its composition commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A chord the IME swallows is recovered from its release, and until now the bytes went out there. They no longer travel through xterm: #13282 moved shortcut input onto the transport, which reaches the pty directly, so a release that fires mid-preedit puts the chord ahead of the text still being composed. With 가나 on the line and 다 in the preedit, Cmd+ArrowLeft yielded 다가나 — the cursor moved to line start and the syllable landed there. The release now defers through sendTerminalInputAfterComposition, with no time bound. The 200ms fallback is right for a newline, where arriving late still means arriving; a cursor chord sent while the preedit is open is the corruption the wait exists to prevent, and a Japanese conversion holds its candidate window open far longer than that. An abandoned composition drops the chord instead, which costs one keypress rather than a mangled line. Routing the chord back through terminal.input() was the smaller change and does not work: on the patched xterm this tree builds, a byte handed to input() during a live composition is dropped outright rather than queued behind the preedit. The recordings that show it queued predate that. A chord remapped onto a pane command keeps firing immediately. Deferring it too would let the commit land after the pane had already cleared or closed, which loses the text rather than ordering it. No platform check: what arms the recovery is a release that still reports itself composing, which an input source either produces or does not. Gating it to macOS was tried and moved nothing measurable, so it would have been an untested claim. The trace rig was watching terminal.input and terminal.onData, so every chord assertion had been reading an empty channel since #13282. Both records now follow the bytes down either route, and expectEmitted compares as a joined stream because the captures were taken when xterm flushed the preedit and the chord as one payload while the transport writes each on its own. Two captures stop mid-composition, where the contract is now "held, not sent". They assert that, then drive the commit the recording does not contain and check that the chord lands after it. The appended commit is marked as authored so it cannot be read as captured. Two expectations went stale when an earlier merge changed the bundled xterm, and both are about what xterm emits rather than what this handler sends. It no longer commits a session it never saw start, so a capture that opens mid-gesture flushes nothing; and it now commits a cancelled composition from its own buffer instead of the cleared helper textarea. The cancel case stops pinning whether that text arrives and pins that the chord is last either way. The three macOS e2e specs this branch adds are hand-run — no workflow calls them and they need real input sources — and now say so in a header line, so a red local run is not read as a regression of this change. Not touched: the two symptoms #13282 gave for the revert (full-width punctuation arriving as ASCII, an invisible Korean preedit). Both live in xterm's composition rendering, which nothing here goes near. Local: src/renderer/src/components/terminal-pane is 251 files / 3230 tests, all passing. tsc clean on all three projects, oxlint and oxfmt clean, reliability gate manifest passes for 73 gates, max-lines ratchet OK at 352. Measured after installing this worktree's own dependencies. Worktrees here resolve node_modules from the enclosing checkout, which was serving an xterm patch hash this lockfile does not ask for; the 62 failures seen before that install were entirely that mismatch. Co-Authored-By: Claude Opus 5 (1M context) --- ...lers.issue-12871-command-release-traces.ts | 8 ++-- ...andlers.issue-12871-in-app-chord-traces.ts | 2 + ....issue-12871-recorded-chord-traces.test.ts | 37 ++++++++++++++++--- .../terminal-pane/keyboard-handlers.ts | 31 +++++++++++----- .../terminal-ime-deferred-newline.ts | 14 +++++-- ...l-macos-chord-input-pipeline-probe.spec.ts | 2 + ...inal-macos-ime-cursor-chord-native.spec.ts | 2 + ...al-macos-kotoeri-chord-keyup-probe.spec.ts | 1 + 8 files changed, 76 insertions(+), 21 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts index a25d3260bfb8..9e36df87c857 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts @@ -25,6 +25,8 @@ export const COMMAND_RELEASE_TRACE_CASES: InAppRecordedCase[] = [ name: 'Japanese, Cmd+ArrowLeft over a live さ preedit, ended by the Command release', expectCalls: ['\x01'], expectEmitted: ['\x01'], + // Kotoeri is still converting when the capture stops. + commitsAfterCapture: true, rows: [ { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, { @@ -46,9 +48,9 @@ export const COMMAND_RELEASE_TRACE_CASES: InAppRecordedCase[] = [ // recovery can break — that neither release fires anything on a committing input source. name: 'Korean 2-Set, Cmd+ArrowLeft commits the syllable and neither release fires', expectCalls: [], - // xterm's own commit of the preedit, not the handler's doing — `expectCalls` staying empty - // is what says the handler kept out of it. - expectEmitted: ['ㄴ'], + // Empty because the capture opens mid-gesture: xterm commits nothing for a session it never + // saw begin. What this case pins is `expectCalls` — neither release fired. + expectEmitted: [], rows: [ { t: 'keydown', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: true }, { diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts index 8d06e27377d0..5d1cf38805f1 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts @@ -44,6 +44,8 @@ export type InAppRecordedCase = { expectCalls: string[] expectEmitted: string[] rows: InAppRecordedRow[] + /** Rows end mid-composition: assert nothing sent yet, then drive a commit and check again. */ + commitsAfterCapture?: true } export const IN_APP_TRACE_CASES: InAppRecordedCase[] = [ diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts index 5ef1259bc274..7fdf3e582449 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -60,6 +60,7 @@ type RecordedCase = { expectCalls: string[] expectEmitted: string[] rows: RecordedRow[] + commitsAfterCapture?: true } const CASES: RecordedCase[] = [ @@ -296,6 +297,8 @@ function openRig(overrides: Record = {}): Rig { throw new Error('xterm helper textarea was not created') } + // Both records are channel-agnostic: chord bytes take the transport, not terminal.input, so + // watching only the latter would report zero for every one of them. const emitted: string[] = [] terminal.onData((data) => emitted.push(data)) const inputCalls: string[] = [] @@ -307,7 +310,11 @@ function openRig(overrides: Record = {}): Rig { const transport = { getPtyId: () => 'pty-1', - sendInput: vi.fn(() => true) + sendInput: vi.fn((data: string) => { + inputCalls.push(data) + emitted.push(data) + return true + }) } as unknown as PtyTransport const pane = { id: 1, leafId: '00000000-0000-4000-8000-000000000001', terminal } const manager = { @@ -415,6 +422,16 @@ async function replay(textarea: HTMLTextAreaElement, rows: RecordedRow[]): Promi await macrotask() } +// Authored, unlike every row in the tables above: the commit a mid-composition capture never +// got to record. +async function commitComposition(textarea: HTMLTextAreaElement, committed: string): Promise { + const end = new CompositionEvent('compositionend', { bubbles: true }) + Object.defineProperty(end, 'data', { value: committed }) + textarea.dispatchEvent(end) + await macrotask() + await macrotask() +} + const JAPANESE_CASE = 'Japanese, a bare arrow and four chords across one live preedit' // By name, not by index: each test below needs a particular gesture, and inserting a case @@ -449,8 +466,15 @@ describe('recorded macOS chord traces during an IME composition', () => { const rig = openRig() await replay(rig.textarea, testCase.rows) + if (testCase.commitsAfterCapture) { + // Held, not dropped: nothing yet, and the same expectations must hold once it commits. + expect(rig.inputCalls).toEqual([]) + await commitComposition(rig.textarea, 'さ') + } expect(rig.inputCalls).toEqual(testCase.expectCalls) - expect(rig.emitted).toEqual(testCase.expectEmitted) + // Joined: the captures were taken when xterm flushed preedit and chord as one payload, and + // the transport writes each on its own. + expect(rig.emitted.join('')).toEqual(testCase.expectEmitted.join('')) rig.unmount() }) @@ -723,6 +747,8 @@ describe('constructed shapes the macOS captures do not contain', () => { { t: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true } ]) + expect(rig.inputCalls).toEqual([]) + await commitComposition(rig.textarea, '日本語') expect(rig.inputCalls).toEqual(['\x01']) rig.unmount() }) @@ -780,10 +806,11 @@ describe('constructed shapes the macOS captures do not contain', () => { { t: 'compositionend', data: '' } ]) - // The chord was aimed at the shell's line, not at the preedit, so cancelling the text does - // not retract it. Only the composed characters are dropped. + // The chord was aimed at the shell's line, not the preedit, so cancelling does not retract it. expect(rig.inputCalls).toEqual(['\x01']) - expect(rig.emitted).toEqual(['\x01']) + // Whether the cancelled text reaches the pty is xterm's call and deliberately not pinned — + // the bundled build now commits it from its own buffer. The chord must be last either way. + expect(rig.emitted.at(-1)).toBe('\x01') rig.unmount() }) }) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 25599913dad1..acb9d469a233 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -15,7 +15,8 @@ import { getTerminalImeModifiedEnterKind, isTerminalImeConsumedKey, isTerminalImeEnterKeyUp, - isTerminalImeProcessEnter + isTerminalImeProcessEnter, + sendTerminalInputAfterComposition } from './terminal-ime-deferred-newline' import { hasPendingTerminalImeComposition } from './terminal-ime-composition-route' import { isImeOwnedKeyboardEvent } from '@/lib/ime-composition-keyboard-event' @@ -84,9 +85,8 @@ type PendingImeChord = Parameters[0] & * - Japanese conversion swallowed the chord whole: no commit, no replay, and the * composition is still live at release. Nothing else will ever deliver it. * - * So a still-composing release means the chord has no other route to the shell. xterm queues - * the bytes behind the preedit and flushes them on commit, so sending them cannot reorder them - * ahead of the text being composed. + * So a still-composing release means the chord has no other route to the shell. Its bytes take + * the transport, which bypasses xterm, so the release holds them until the composition commits. * * The snapshot is what makes reading the release safe. A `KeyboardEvent`'s modifier flags * describe the moment it fired, and releasing `Cmd` before `←` leaves the arrow's keyup with @@ -700,7 +700,9 @@ export function useTerminalKeyboardShortcuts({ return } runShortcutAction(e, action, manager, (pane, sendResolvedInput) => { - if (!((e.isComposing || hasPendingImeComposition) && (e.key === 'Enter' || imeProcessEnter))) { + if ( + !((e.isComposing || hasPendingImeComposition) && (e.key === 'Enter' || imeProcessEnter)) + ) { return false } if (isWindows) { @@ -726,9 +728,9 @@ export function useTerminalKeyboardShortcuts({ e: ShortcutActionEvent, action: NonNullable>, manager: PaneManager, - // Why a callback: deferring a composing Enter reads the live keydown and its - // imeProcessEnter, which a chord recovered from a release has neither of — and never is. - deferComposingEnter?: (pane: ManagedPane, sendResolvedInput: () => void) => boolean + // Why a callback: a composing Enter reads the live keydown and its imeProcessEnter, which + // a chord recovered from a release has neither of — and never is. + deferSend?: (pane: ManagedPane, sendResolvedInput: () => void) => boolean ): void { if (action.type === 'switchInputSource') { // Why: the OS must receive its default action, while xterm must receive @@ -746,7 +748,7 @@ export function useTerminalKeyboardShortcuts({ return } const sendResolvedInput = createCapturedInputSender(pane, action.data) - if (deferComposingEnter?.(pane, sendResolvedInput)) { + if (deferSend?.(pane, sendResolvedInput)) { return } sendResolvedInput() @@ -1125,7 +1127,16 @@ export function useTerminalKeyboardShortcuts({ if (!action || action.type === 'switchInputSource') { return } - runShortcutAction(pressed, action, manager) + runShortcutAction(pressed, action, manager, (pane, sendResolvedInput) => { + // Composing here by definition, and these bytes bypass xterm: sending now puts the chord + // ahead of the text still being composed — 가나 + 다 in the preedit yields 다가나. A chord + // remapped onto a pane command skips this and stays immediate, or the commit lands after + // the pane is already cleared. + sendTerminalInputAfterComposition(pane.terminal.element, sendResolvedInput, { + fallbackMs: null + }) + return true + }) } window.addEventListener('keydown', onModifierDown, { capture: true }) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts index 2fd141fe3ce0..7ade91bd32e8 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts @@ -5,17 +5,25 @@ import { export const TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS = 200 +/** + * `fallbackMs: null` waits however long the composition runs, and drops the input if it never + * ends. A late newline is still a newline; a cursor chord sent mid-preedit is the corruption the + * wait exists to prevent. + */ export function sendTerminalInputAfterComposition( terminalElement: HTMLElement | null | undefined, send: () => void, - options?: { fallbackMs?: number } + options?: { fallbackMs?: number | null } ): void { if (!terminalElement) { window.setTimeout(send, 0) return } - const fallbackMs = options?.fallbackMs ?? TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS + const fallbackMs = + options?.fallbackMs === null + ? null + : (options?.fallbackMs ?? TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS) let done = false const finish = (): void => { @@ -42,7 +50,7 @@ export function sendTerminalInputAfterComposition( const onCompositionSessionEnd = (): void => finishAfterPendingComposition() terminalElement.addEventListener('compositionend', onCompositionEnd) terminalElement.addEventListener(XTERM_COMPOSITION_SESSION_END_EVENT, onCompositionSessionEnd) - const fallbackTimer = window.setTimeout(finish, fallbackMs) + const fallbackTimer = fallbackMs === null ? undefined : window.setTimeout(finish, fallbackMs) } export type TerminalImeDeferredNewlineSender = { diff --git a/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts b/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts index 6ef06ee9f580..9aeb9cb4d4e5 100644 --- a/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts +++ b/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts @@ -1,3 +1,5 @@ +// Hand-run probe: needs real macOS input sources, so no workflow calls it. It records the +// traces replayed by keyboard-handlers.issue-12871-command-release-traces.ts. import { mkdirSync, writeFileSync } from 'node:fs' import path from 'node:path' import type { ElectronApplication, Page } from '@stablyai/playwright-test' diff --git a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts index 8f04318fba27..d976931f9e4b 100644 --- a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts +++ b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts @@ -1,3 +1,5 @@ +// Hand-run: needs a real macOS input source, so no workflow calls it. A red run here is a +// local environment or an IME behaviour change, not necessarily a regression. import { execFileSync } from 'node:child_process' import path from 'node:path' import type { Page } from '@stablyai/playwright-test' diff --git a/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts b/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts index 1c5234409863..c49d5e331bfc 100644 --- a/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts +++ b/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts @@ -1,3 +1,4 @@ +// Hand-run probe: needs Kotoeri installed on a real macOS host, so no workflow calls it. import { mkdirSync, writeFileSync } from 'node:fs' import path from 'node:path' import { expect, test } from './helpers/orca-app'