From c070b84fa019e1281489f5971b7fe266bce05f12 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 15 Aug 2026 21:38:50 +0900 Subject: [PATCH 1/9] fix(terminal): send a composing cursor chord once, not twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #14730 fixed the order: a chord resolved mid-composition now waits for the syllable to commit instead of overtaking it. It still resolves that chord on the composing keydown, and on Korean 2-Set the platform then replays the same chord unmarked after keyup, so both copies fire. Cmd+Left hides it by being idempotent; Option+Left sends \x1bb twice and jumps two words. Recorded on stock macOS: the two input sources are indistinguishable while the key is down (both `code='ArrowLeft'`, `keyCode=229`, `isComposing=true`) and have separated by the time it comes up. Korean has committed and reports the composition over — its replay is on the way, so the press must stay silent. Japanese conversion swallowed the chord whole, with no commit and no replay, and is still composing at the release — nothing else will ever deliver it. So an exempt chord (Cmd/Option/Ctrl over ArrowLeft/ArrowRight/Backspace/Delete, never with Shift, which Japanese binds to resize a segment) is remembered on the composing keydown and decided on its release. A release that still reports itself composing runs the action; one that does not lets the replay answer. Cmd+Left delivers no arrow keyup at all, so the Command release ends that gesture. The snapshot is taken field by field because a KeyboardEvent keeps its fields as prototype accessors, and it carries the modifiers as they were at the press. Bytes still take #14730's deferral, now reached by both paths through one condition rather than two. Also read `code` rather than `key` for those chords while an IME owns the event — a CJK source rewrites `key` to 'Process' (#12171, #13033) — and add 'Process' to the keybinding matcher's physical-code fallback beside 'Dead'. keyboard-handlers-ime-composing-chord.test.tsx from #14730: its chord press now runs to the release, because that is where a swallowed chord is resolvable and what hardware delivers. Same assertions. Fixes #12871 Co-authored-by: hyeonho Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WjefjguNCzpY2kVq3Q69wH --- ...oard-handlers-ime-composing-chord.test.tsx | 7 + ...lers.issue-12871-command-release-traces.ts | 78 ++ ...ssue-12871-exempt-chord-resolution.test.ts | 153 ++++ ...andlers.issue-12871-in-app-chord-traces.ts | 128 +++ ....issue-12871-recorded-chord-traces.test.ts | 816 ++++++++++++++++++ .../terminal-pane/keyboard-handlers.ts | 224 ++++- .../terminal-pane/terminal-shortcut-policy.ts | 60 +- src/shared/keybindings.ts | 4 +- 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 | 279 ++++++ ...inal-macos-ime-cursor-chord-native.spec.ts | 363 ++++++++ ...al-macos-kotoeri-chord-keyup-probe.spec.ts | 214 +++++ 15 files changed, 2640 insertions(+), 28 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.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 create mode 100644 tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts create mode 100644 tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx index 2200f2d5c9ee..db5aaae185f4 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx @@ -145,6 +145,10 @@ describe('a cursor chord pressed during a composition', () => { vi.restoreAllMocks() }) + // The gesture runs to its release because a chord the IME swallowed is only resolvable there: + // on the keydown, Korean's committing source and Japanese's swallowing one are byte-identical, + // and acting on both would fire Korean's twice once the platform replays it. Recorded on macOS + // 26.5.1: `Cmd+←` delivers no arrow keyup at all, so the Command release ends it. function pressCmdArrowLeft( harness: ReturnType, isComposing: boolean @@ -158,6 +162,9 @@ describe('a cursor chord pressed during a composition', () => { isComposing }) ) + harness.terminalInput.dispatchEvent( + keyboardEvent('keyup', { key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing }) + ) } // The Korean 2-Set shape: the platform replays the chord unmarked after keyup, so `isComposing` 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..9e36df87c857 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-command-release-traces.ts @@ -0,0 +1,78 @@ +// 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'], + // Kotoeri is still converting when the capture stops. + commitsAfterCapture: true, + 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: [], + // 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 }, + { + 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-exempt-chord-resolution.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts new file mode 100644 index 000000000000..722e0262203d --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts @@ -0,0 +1,153 @@ +// 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. 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. +import { describe, expect, it } from 'vitest' +import { resolveTerminalKeyboardShortcutAction } from './keyboard-handlers' +import { isImeExemptTerminalChord, 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) => { + expect(resolveOnMac({ ...event, isComposing: true })).toEqual({ + type: 'sendInput', + data: expected + }) + }) + + // 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', () => { + const event = keyEvent({ + key: 'Process', + code: 'ArrowLeft', + metaKey: true, + isComposing: true, + keyCode: 229 + }) + + expect(resolveOnMac(event)).toEqual({ type: 'sendInput', data: '\x01' }) + }) + + // The gate that decides whether a composing keydown is remembered for its release. Asserted + // directly rather than through the resolver: the resolver answers for every binding in the + // registry, so a new one landing there would break these rows for a reason unrelated to the + // exemption. The exemption is for chords over a physical key, not for the IME's own gesture + // keys — a composing Ctrl+Space mode switch reaches this 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' })], + // 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 })] + ])('does not remember %s for its release', (_label, event) => { + expect(isImeExemptTerminalChord({ ...event, isComposing: true })).toBe(false) + }) + + it.each([ + ['Cmd+ArrowLeft', keyEvent({ key: 'ArrowLeft', code: 'ArrowLeft', metaKey: true })], + ['Option+ArrowRight', keyEvent({ key: 'ArrowRight', code: 'ArrowRight', altKey: true })], + // `key` is already rewritten here, which is why the gate reads `code`. + ['Cmd+Backspace as Process', keyEvent({ key: 'Process', code: 'Backspace', metaKey: true })] + ])('remembers %s for its release', (_label, event) => { + expect(isImeExemptTerminalChord({ ...event, isComposing: true })).toBe(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 new file mode 100644 index 000000000000..5d1cf38805f1 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-in-app-chord-traces.ts @@ -0,0 +1,128 @@ +// 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[] + /** Rows end mid-composition: assert nothing sent yet, then drive a commit and check again. */ + commitsAfterCapture?: true +} + +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 new file mode 100644 index 000000000000..7fdf3e582449 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-recorded-chord-traces.test.ts @@ -0,0 +1,816 @@ +// @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, 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 while the key is down. The release does: +// +// - Korean 2-Set commits the syllable, emits compositionend, and the platform replays the +// 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 this fix its chords produced no bytes at all. +// +// 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. +// +// 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. +// +// 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 = { + 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[] + commitsAfterCapture?: true +} + +const CASES: RecordedCase[] = [ + { + 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: ['\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: ['\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, 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: '日本語' } + ] + } +] + +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 + terminal: Terminal + searchSelections: string[] + closePaneCalls: number[] + inputCalls: string[] + emitted: string[] + clearPaneCalls: number + unmount: () => void +} + +function openRig(overrides: Record = {}): 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') + } + + // 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[] = [] + 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((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 = { + 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' }, + ...overrides + } as unknown as Parameters[0] + + 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 + }, + 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) + // 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() + 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 +// 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({ + 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, ...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) + + 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) + // 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() + }) + + // 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, caseNamed(JAPANESE_CASE).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, caseNamed('Korean 2-Set, Cmd+Backspace').rows) + + expect(rig.clearPaneCalls).toBe(1) + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + + // 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() + const field = document.createElement('input') + rig.textarea.parentElement?.append(field) + + 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([]) + 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() + }) + + // 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() + }) + + // 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: 'keyup', key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37, isComposing: true, alt: true } + ]) + + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + + // 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 } + ]) + + expect(rig.inputCalls).toEqual([]) + 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. + 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([]) + await commitComposition(rig.textarea, '日本語') + 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 + // 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 the preedit, so cancelling does not retract it. + expect(rig.inputCalls).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 c796e58ed049..4a359d54d398 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -6,7 +6,7 @@ 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 { @@ -19,6 +19,7 @@ import { sendTerminalInputAfterComposition } from './terminal-ime-deferred-newline' import { hasPendingTerminalImeComposition } from './terminal-ime-composition-route' +import { isImeOwnedKeyboardEvent } from '@/lib/ime-composition-keyboard-event' import { requestCapturedTerminalReconfirmation, sendCapturedTerminalInput @@ -55,6 +56,86 @@ 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 + } + +/** + * 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 + * 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 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. 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 + * `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 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, + 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` 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 + } +} + +/** + * 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') +} + +/** + * No IME gate here on purpose: the pane calls this for a swallowed chord's RELEASE, where the + * event still reports itself as composing and resolving it is the whole point. Which composing + * events reach a resolver at all is the pane's decision, not this function's — so a keydown + * outcome asserted against this function alone pins something the pane does not do. + */ export function resolveTerminalKeyboardShortcutAction( event: Parameters[0], isMac: Parameters[1], @@ -286,6 +367,7 @@ export function useTerminalKeyboardShortcuts({ let optionKeyLocation = 0 const heldImeEnterModifiers = new Set<'shift' | 'ctrl'>() const terminalImeEnterModifierKeydowns = new Set<'shift' | 'ctrl'>() + let pendingImeChord: PendingImeChord | null = null const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker() const deferredNewlineSender = createTerminalImeDeferredNewlineSender() const modifiedEnterChordOwner = createTerminalImeModifiedEnterChordOwner() @@ -510,6 +592,26 @@ 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 + } + // Only the exempt chords yield here: an IME that swallows one leaves no other route to the + // shell, so it is recovered from the release. Every other IME-owned key keeps the paths + // below, which own the composing Enter. + if (isImeOwnedKeyboardEvent(e) && 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 (!isEditableTarget(e.target)) { + pendingImeChord = imeChordSnapshot(e) + } + return + } const modifiedEnterChord = isWindows ? getModifiedEnterChord(e) : null if ( @@ -597,7 +699,41 @@ export function useTerminalKeyboardShortcuts({ if (!action) { return } + const composing = e.isComposing || hasPendingImeComposition + runShortcutAction(e, action, manager, composing, (pane, sendResolvedInput) => { + if (!(composing && (e.key === 'Enter' || imeProcessEnter))) { + return false + } + if (isWindows) { + const chord = getModifiedEnterChord(e) + const claimedChord = chord + ? { + ...chord, + terminalModifierKeyDownObserved: terminalImeEnterModifierKeydowns.has(chord.kind) + } + : null + if (claimedChord && !modifiedEnterChordOwner.claim(claimedChord)) { + return true + } + } + deferredNewlineSender.defer(e, pane.terminal.element, sendResolvedInput) + return true + }) + } + // 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: ShortcutActionEvent, + action: NonNullable>, + manager: PaneManager, + // Passed in because a chord recovered from a release has no live event to read it from, + // and is composing by definition — that is what armed the recovery. + composing: 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 // none of the keydown, keypress, or keyup sequence. @@ -614,20 +750,7 @@ export function useTerminalKeyboardShortcuts({ return } const sendResolvedInput = createCapturedInputSender(pane, action.data) - if ((e.isComposing || hasPendingImeComposition) && (e.key === 'Enter' || imeProcessEnter)) { - if (isWindows) { - const chord = getModifiedEnterChord(e) - const claimedChord = chord - ? { - ...chord, - terminalModifierKeyDownObserved: terminalImeEnterModifierKeydowns.has(chord.kind) - } - : null - if (claimedChord && !modifiedEnterChordOwner.claim(claimedChord)) { - return - } - } - deferredNewlineSender.defer(e, pane.terminal.element, sendResolvedInput) + if (deferSend?.(pane, sendResolvedInput)) { return } // Why: the composed glyph reaches the pty from the composition session-end handler, which @@ -635,7 +758,7 @@ export function useTerminalKeyboardShortcuts({ // after — `가나다` then Cmd+Left leaves `다가나` (#12871). Enter is handled above, where a // fallback timer is right because a newline arriving late still arrives; a chord arriving // mid-preedit is the corruption itself, so this one waits without a deadline. - if (e.isComposing || hasPendingImeComposition) { + if (composing) { sendTerminalInputAfterComposition(pane.terminal.element, sendResolvedInput, { fallbackMs: null }) @@ -960,6 +1083,9 @@ export function useTerminalKeyboardShortcuts({ const onNativeOnlyBlur = (): void => { nativeOnlyShortcutTracker.clear() + // Why: a chord interrupted by Cmd+Tab or Spotlight never delivers its release, and a carry + // with no release to spend it sits armed. + pendingImeChord = null heldImeEnterModifiers.clear() terminalImeEnterModifierKeydowns.clear() modifiedEnterChordOwner.clear() @@ -967,12 +1093,76 @@ export function useTerminalKeyboardShortcuts({ observedEnterKeydownTimeStamps.clear() } + const onSwallowedImeChordRelease = (e: KeyboardEvent): void => { + const chord = pendingImeChord + if (!chord || !releasesPendingImeChord(chord, e)) { + 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 + } + const keyboardScope = keyboardScopeRef.current + if (keyboardScope && !keyboardEventBelongsToScope(e, keyboardScope)) { + return + } + // 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, 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: () => {}, + 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(pressed, shortcutPlatform, keybindings)) { + return + } + 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') { + return + } + runShortcutAction(pressed, action, manager, true) + } + window.addEventListener('keydown', onModifierDown, { capture: true }) window.addEventListener('keyup', onKeyUp, { capture: true }) window.addEventListener('keydown', onKeyDown, { capture: true }) window.addEventListener('keypress', onNativeOnlyShortcutCompanion, { capture: true }) window.addEventListener('keyup', onNativeOnlyShortcutCompanion, { capture: true }) window.addEventListener('beforeinput', onNativeOnlyBeforeInput, { capture: true }) + window.addEventListener('keyup', onSwallowedImeChordRelease, { capture: true }) window.addEventListener('blur', onNativeOnlyBlur) return () => { modifiedEnterChordOwner.clear() @@ -984,6 +1174,8 @@ export function useTerminalKeyboardShortcuts({ window.removeEventListener('keypress', onNativeOnlyShortcutCompanion, { capture: true }) window.removeEventListener('keyup', onNativeOnlyShortcutCompanion, { capture: true }) window.removeEventListener('beforeinput', onNativeOnlyBeforeInput, { capture: true }) + pendingImeChord = null + window.removeEventListener('keyup', onSwallowedImeChordRelease, { capture: true }) window.removeEventListener('blur', onNativeOnlyBlur) } }, [ 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 9fba9efb4479..983f6ffa9162 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,6 +20,43 @@ export type TerminalShortcutEvent = { 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 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, `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. +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 { + 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 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.shiftKey && + (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 — @@ -113,7 +151,7 @@ export function resolveTerminalShortcutAction( hasCtrlEnterCsiUAuthority?: () => boolean ): TerminalShortcutAction | null { const platform: NodeJS.Platform = isMac ? 'darwin' : isWindows ? 'win32' : 'linux' - + const chordKey = physicalChordKey(event) // Why: capture this chord even on repeat without blocking the OS default input-source switch. if (keybindingMatchesAction('terminal.switchInputSource', event, platform, keybindings)) { return { type: 'switchInputSource' } @@ -221,23 +259,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. @@ -254,7 +292,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?.()) { @@ -268,14 +306,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 ( @@ -284,14 +322,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. diff --git a/src/shared/keybindings.ts b/src/shared/keybindings.ts index 43c30d89d909..974bca5feaa4 100644 --- a/src/shared/keybindings.ts +++ b/src/shared/keybindings.ts @@ -1624,7 +1624,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', 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..9aeb9cb4d4e5 --- /dev/null +++ b/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts @@ -0,0 +1,279 @@ +// 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' +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 new file mode 100644 index 000000000000..d976931f9e4b --- /dev/null +++ b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts @@ -0,0 +1,363 @@ +// 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' +import { expect, test } from './helpers/orca-app' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { pressChordWithSeparateModifier } from './macos-input-source-driver' +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. 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) + execFileSync('osascript', ['-e', `tell application "System Events" to key code ${keyCode}`]) +} + +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) + } + }) +}) 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..c49d5e331bfc --- /dev/null +++ b/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts @@ -0,0 +1,214 @@ +// 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' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + focusActiveTerminalInput, + 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, + 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". + */ + +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 fa49cb54c2d08d9bbdcdfce15ca15777309d8ffa Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 15 Aug 2026 21:38:50 +0900 Subject: [PATCH 2/9] fix(terminal): bound the chord recovery to the platform it has evidence for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent reviews of the previous commit, run without sight of each other, agreed on where it overreached. Arming is now macOS-only. Both behaviours the release reads are stock-macOS captures: that a committing source replays the chord unmarked, and that Cmd+key delivers no keyup. An input source that commits without replaying — unrecorded for ibus and MS-IME — would arm, see a release that no longer reports itself composing, hand the chord to a replay that never comes, and drop it. That is worse than the ordering bug: #14730 delivered it late, this lost it. Elsewhere the deferred send stands, now pinned by a test on a Windows user agent. Reading the physical code moved out of the shared policy and into the snapshot, which is the only consumer that needed it. The policy is also called by the dashboard popout preview terminal, whose IME gate excludes modifier chords, so a composing Ctrl+Left there resolved to bytes with none of the recovery around it — the ordering bug reintroduced on a surface with no deferral. Direct calls into both trees confirmed it: `{key:'Process', code:'Backspace', ctrlKey:true, isComposing:true}` on win32 returned null before and `\x17` after. The snapshot stores `event.code` as its key instead, which is correct because the caller has already narrowed to four codes whose key is that same string. `'Process'` in the shared keybinding matcher's physical-code fallback goes with it: it short- circuited past a guard written on purpose for AltGr, and nothing needs it now. The arming keydown consumes the press again, as it did before this branch existed. Returning early skipped preventDefault and stopImmediatePropagation, so a global handler bound to the same remapped chord acted on the press while the release acted again — one gesture, two firings. The carry is keyed by code rather than a single slot, following the native-only tracker in the same pane. Two exempt keys can be down under one Cmd hold, and the Command release ends both; a single slot dropped the first without a trace. Also: selectAll joins switchInputSource in the release guard, since both arm the native-only tracker from a press; the file-search branch stops cutting off a keyup the comment three lines above it says must keep propagating; and the composition session events carry the name of the patch that emits them, since upstream xterm does not and a regenerated patch that drops them fails silently into an unbounded wait. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WjefjguNCzpY2kVq3Q69wH --- ...lers.issue-12871-command-release-traces.ts | 9 +- ...ssue-12871-exempt-chord-resolution.test.ts | 73 +------ ...andlers.issue-12871-in-app-chord-traces.ts | 12 +- ....issue-12871-recorded-chord-traces.test.ts | 204 +++++++++++------- .../terminal-pane/keyboard-handlers.ts | 131 +++++------ .../terminal-ime-composition-route.ts | 3 + .../terminal-pane/terminal-shortcut-policy.ts | 58 ++--- src/shared/keybindings.ts | 4 +- 8 files changed, 227 insertions(+), 267 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 9e36df87c857..f5b022941ad0 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 @@ -6,8 +6,9 @@ // // 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. +// flags instead, which is why no earlier IN-APP recording contains a modifier press or release +// and why the Cmd half of the gesture looked like it had no end. The bare-page recordings in +// keyboard-handlers.issue-12871-recorded-chord-traces.test.ts do carry both. // // 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: @@ -18,9 +19,9 @@ // - 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' +import type { RecordedChordCase } from './keyboard-handlers.issue-12871-in-app-chord-traces' -export const COMMAND_RELEASE_TRACE_CASES: InAppRecordedCase[] = [ +export const COMMAND_RELEASE_TRACE_CASES: RecordedChordCase[] = [ { name: 'Japanese, Cmd+ArrowLeft over a live さ preedit, ended by the Command release', expectCalls: ['\x01'], diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts index 722e0262203d..46fa9d70bd0e 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts @@ -1,14 +1,8 @@ -// 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. 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. +// Issue #12871, unit half: the decision boundary, including the negatives. Shapes here are +// constructed from the DOM contract; hardware fidelity 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. Why the two input sources need +// different answers is in `isImeExemptTerminalChord` (terminal-shortcut-policy.ts). import { describe, expect, it } from 'vitest' import { resolveTerminalKeyboardShortcutAction } from './keyboard-handlers' import { isImeExemptTerminalChord, type TerminalShortcutEvent } from './terminal-shortcut-policy' @@ -69,59 +63,6 @@ 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', () => { - const event = keyEvent({ - key: 'Process', - code: 'ArrowLeft', - metaKey: true, - isComposing: true, - keyCode: 229 - }) - - expect(resolveOnMac(event)).toEqual({ type: 'sendInput', data: '\x01' }) - }) - // The gate that decides whether a composing keydown is remembered for its release. Asserted // directly rather than through the resolver: the resolver answers for every binding in the // registry, so a new one landing there would break these rows for a reason unrelated to the @@ -139,7 +80,7 @@ describe('terminal chords stay live during an IME composition', () => { ], ['Cmd+A', keyEvent({ key: 'a', code: 'KeyA', metaKey: true })] ])('does not remember %s for its release', (_label, event) => { - expect(isImeExemptTerminalChord({ ...event, isComposing: true })).toBe(false) + expect(isImeExemptTerminalChord(event)).toBe(false) }) it.each([ @@ -148,6 +89,6 @@ describe('terminal chords stay live during an IME composition', () => { // `key` is already rewritten here, which is why the gate reads `code`. ['Cmd+Backspace as Process', keyEvent({ key: 'Process', code: 'Backspace', metaKey: true })] ])('remembers %s for its release', (_label, event) => { - expect(isImeExemptTerminalChord({ ...event, isComposing: true })).toBe(true) + expect(isImeExemptTerminalChord(event)).toBe(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 5d1cf38805f1..c5adc8426770 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 @@ -24,9 +24,9 @@ // 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 = { +// Shared with the bare-page recordings in the replaying test and with the command-release +// module, so a field added to one shape cannot be missed by the other. +export type RecordedChordRow = { t: string key?: string code?: string @@ -39,16 +39,16 @@ export type InAppRecordedRow = { value?: string } -export type InAppRecordedCase = { +export type RecordedChordCase = { name: string expectCalls: string[] expectEmitted: string[] - rows: InAppRecordedRow[] + rows: RecordedChordRow[] /** Rows end mid-composition: assert nothing sent yet, then drive a commit and check again. */ commitsAfterCapture?: true } -export const IN_APP_TRACE_CASES: InAppRecordedCase[] = [ +export const IN_APP_TRACE_CASES: RecordedChordCase[] = [ { // korean-cmdright.json dom[36..50]: committed 가나 with 나 composing, Cmd+ArrowRight. // Captured PTY line: 가나나\x05\x1b[C\n. 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 7fdf3e582449..20a725d57568 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 @@ -40,30 +40,13 @@ 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 = { - 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[] - commitsAfterCapture?: true -} +import { + IN_APP_TRACE_CASES, + type RecordedChordCase, + type RecordedChordRow +} from './keyboard-handlers.issue-12871-in-app-chord-traces' -const CASES: RecordedCase[] = [ +const CASES: RecordedChordCase[] = [ { name: 'Korean 2-Set, Cmd+ArrowLeft', expectCalls: ['\x01'], @@ -375,7 +358,7 @@ function openRig(overrides: Record = {}): Rig { } } -function dispatchRow(textarea: HTMLTextAreaElement, row: RecordedRow): void { +function dispatchRow(textarea: HTMLTextAreaElement, row: RecordedChordRow): void { if (row.t === 'keydown' || row.t === 'keyup') { const event = new KeyboardEvent(row.t, { key: row.key, @@ -409,7 +392,7 @@ function dispatchRow(textarea: HTMLTextAreaElement, row: RecordedRow): void { } } -async function replay(textarea: HTMLTextAreaElement, rows: RecordedRow[]): Promise { +async function replay(textarea: HTMLTextAreaElement, rows: RecordedChordRow[]): Promise { for (const row of rows) { dispatchRow(textarea, row) // A full task between rows, deliberately: nothing here may depend on how fast the rows @@ -436,7 +419,7 @@ const JAPANESE_CASE = 'Japanese, a bare arrow and four chords across one live pr // 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 { +function caseNamed(name: string): RecordedChordCase { const found = CASES.find((testCase) => testCase.name === name) if (!found) { throw new Error(`recorded case not found: ${name}`) @@ -444,20 +427,20 @@ function caseNamed(name: string): RecordedCase { return found } -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) - }) +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() - }) +afterEach(() => { + cleanup() + vi.restoreAllMocks() + document.body.replaceChildren() +}) +describe('recorded macOS chord traces during an IME composition', () => { it.each( [...CASES, ...IN_APP_TRACE_CASES, ...COMMAND_RELEASE_TRACE_CASES].map( (testCase) => [testCase.name, testCase] as const @@ -506,48 +489,11 @@ describe('recorded macOS chord traces during an IME composition', () => { // 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() - const field = document.createElement('input') - rig.textarea.parentElement?.append(field) - - 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([]) - 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. @@ -654,7 +600,7 @@ describe('constructed shapes the macOS captures do not contain', () => { // 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[] = [ + const ARMED_ALT_ARROW: RecordedChordRow[] = [ { t: 'compositionstart', data: '' }, { t: 'keydown', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: true, alt: true }, { @@ -722,6 +668,108 @@ describe('constructed shapes the macOS captures do not contain', () => { rig.unmount() }) + // The release-keyed recovery reads two stock-macOS behaviours: that a committing source + // replays the chord unmarked, and that Cmd+key delivers no keyup. Neither is recorded for + // ibus or MS-IME, and an input source that commits without replaying would lose the chord + // outright. Everywhere else the chord must still arrive once the composition commits. + it('keeps the deferred send on a non-mac platform, with no release at all', async () => { + vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue( + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' + ) + const rig = openRig() + await replay(rig.textarea, [ + { t: 'compositionstart', data: '' }, + { t: 'compositionupdate', data: 'ㅅ' }, + { t: 'input', data: 'ㅅ', value: 'ㅅ' }, + { + t: 'keydown', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 229, + isComposing: true, + alt: true + } + ]) + + // Held, not sent, exactly as on the parent commit. + expect(rig.inputCalls).toEqual([]) + await commitComposition(rig.textarea, '사') + expect(rig.inputCalls).toEqual(['\x1bb']) + rig.unmount() + }) + + // Both keys are down at once under one Cmd hold, and the Command release ends both. A single + // slot would drop the first chord silently: its keydown produced no bytes, so nothing but the + // missing line-start would show it was ever pressed. + it('sends both chords when two exempt keys are held under one modifier', 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: 'keydown', + key: 'Backspace', + code: 'Backspace', + keyCode: 229, + isComposing: true, + meta: true + }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true } + ]) + + expect(rig.inputCalls).toEqual([]) + await commitComposition(rig.textarea, '日本語') + // Press order: line start, then kill to line start. + expect(rig.inputCalls).toEqual(['\x01', '\x15']) + rig.unmount() + }) + + // The pane takes the press when it remembers it, so a global handler bound to the same chord + // must not also act on it — that fires the remapped action twice, once per phase. + it('consumes the keydown it remembers', 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) + + const seenAfterUs: string[] = [] + const later = (event: Event): void => { + seenAfterUs.push((event as KeyboardEvent).code) + } + window.addEventListener('keydown', later, { capture: true }) + const press = new KeyboardEvent('keydown', { + key: 'ArrowLeft', + code: 'ArrowLeft', + metaKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperties(press, { isComposing: { value: true }, keyCode: { value: 229 } }) + rig.textarea.dispatchEvent(press) + await macrotask() + window.removeEventListener('keydown', later, { capture: true }) + + expect(press.defaultPrevented).toBe(true) + expect(seenAfterUs).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. @@ -781,8 +829,8 @@ describe('constructed shapes the macOS captures do not contain', () => { ) 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. + // The remembered chord carries the physical code as its key; without that it falls through + // to the built-in kill-line byte, the wrong action and silently, for anyone who remapped it. expect(rig.clearPaneCalls).toBe(1) expect(rig.inputCalls).toEqual(['\x01', '\x1bb', '\x1b\x7f']) 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 4a359d54d398..3d049e7c54ce 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -68,33 +68,17 @@ type ShortcutActionEvent = { type PendingImeChord = Parameters[0] & Required[0], 'code' | 'repeat'>> & { - isComposing: boolean + isComposing: true } /** - * 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 - * 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 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. 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 - * `metaKey: false` — matching on that loses the chord entirely. + * A chord the IME took on keydown, kept until the key comes up. Snapshotted rather than read + * off the release, because a `KeyboardEvent`'s modifier flags describe the moment it fired and + * releasing `Cmd` before `←` leaves the arrow's keyup with `metaKey: false`. * * 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. + * on the auto-repeat instead would have to run before the two input sources separate, so this + * waits 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 @@ -102,7 +86,10 @@ function imeChordSnapshot(event: KeyboardEvent): PendingImeChord { // `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, + // The code, not `key`: a CJK source rewrites `key` to 'Process' (#12171, #13033) and every + // consumer below matches on `key`. Safe because the caller has already checked this code is + // one of the four exempt ones, whose `key` is the same string when nothing rewrote it. + key: event.code, code: event.code, metaKey: event.metaKey, ctrlKey: event.ctrlKey, @@ -110,31 +97,20 @@ 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` 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 } } -/** - * 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. - */ +// The Meta arm is not a fallback: recorded at Chromium's own input dispatch, a key released +// while `Cmd` is held delivers no keyup at all, so the Command release is the only event that +// ends that gesture. function releasesPendingImeChord(chord: PendingImeChord, event: KeyboardEvent): boolean { - return chord.code === event.code || (chord.metaKey === true && event.key === 'Meta') + return chord.code === event.code || (chord.metaKey && event.key === 'Meta') } /** * No IME gate here on purpose: the pane calls this for a swallowed chord's RELEASE, where the - * event still reports itself as composing and resolving it is the whole point. Which composing - * events reach a resolver at all is the pane's decision, not this function's — so a keydown - * outcome asserted against this function alone pins something the pane does not do. + * event still reports itself as composing and resolving it is the whole point. */ export function resolveTerminalKeyboardShortcutAction( event: Parameters[0], @@ -367,7 +343,9 @@ export function useTerminalKeyboardShortcuts({ let optionKeyLocation = 0 const heldImeEnterModifiers = new Set<'shift' | 'ctrl'>() const terminalImeEnterModifierKeydowns = new Set<'shift' | 'ctrl'>() - let pendingImeChord: PendingImeChord | null = null + // Keyed rather than a single slot, following the native-only tracker in the same pane: two + // exempt keys can be down at once under one modifier hold, and each owes its own release. + const pendingImeChords = new Map() const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker() const deferredNewlineSender = createTerminalImeDeferredNewlineSender() const modifiedEnterChordOwner = createTerminalImeModifiedEnterChordOwner() @@ -597,18 +575,23 @@ export function useTerminalKeyboardShortcuts({ // 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 - } - // Only the exempt chords yield here: an IME that swallows one leaves no other route to the - // shell, so it is recovered from the release. Every other IME-owned key keeps the paths - // below, which own the composing Enter. - if (isImeOwnedKeyboardEvent(e) && isImeExemptTerminalChord(e)) { + pendingImeChords.delete(e.code) + // macOS only, and only the exempt chords. Both halves of what the release reads are stock + // macOS captures — that a committing source replays the chord unmarked, and that Cmd+key + // delivers no keyup — and an IME that commits without replaying would lose the chord + // outright here. Elsewhere the paths below keep sending it once the composition commits, + // which is late rather than never. + if (isMac && isImeOwnedKeyboardEvent(e) && 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 (!isEditableTarget(e.target)) { - pendingImeChord = imeChordSnapshot(e) + pendingImeChords.set(e.code, imeChordSnapshot(e)) + // Claimed here rather than at the release, so no other window listener acts on a press + // this pane has already taken. Without it the same chord fires twice on a remap: once + // from a global handler on the press, once from here on the release. + e.preventDefault() + e.stopImmediatePropagation() } return } @@ -1085,7 +1068,7 @@ export function useTerminalKeyboardShortcuts({ nativeOnlyShortcutTracker.clear() // Why: a chord interrupted by Cmd+Tab or Spotlight never delivers its release, and a carry // with no release to spend it sits armed. - pendingImeChord = null + pendingImeChords.clear() heldImeEnterModifiers.clear() terminalImeEnterModifierKeydowns.clear() modifiedEnterChordOwner.clear() @@ -1093,15 +1076,7 @@ export function useTerminalKeyboardShortcuts({ observedEnterKeydownTimeStamps.clear() } - const onSwallowedImeChordRelease = (e: KeyboardEvent): void => { - const chord = pendingImeChord - if (!chord || !releasesPendingImeChord(chord, e)) { - 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 runSwallowedImeChord = (chord: PendingImeChord, e: KeyboardEvent): void => { const manager = managerRef.current if (!manager) { return @@ -1118,14 +1093,9 @@ export function useTerminalKeyboardShortcuts({ if (isEditableTarget(e.target)) { return } - // 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. + // The propagation stoppers are no-ops, unlike `preventDefault`: the action has already run, + // and cutting a keyup off at window capture would keep it from xterm's own `_keyUp`, which + // clears the flags its input path reads. const pressed: PendingImeChord & ShortcutActionEvent & { stopPropagation: () => void } = { ...chord, preventDefault: () => e.preventDefault(), @@ -1137,25 +1107,40 @@ export function useTerminalKeyboardShortcuts({ if (handleEmptyFloatingWorkspacePanelCloseShortcut(pressed, shortcutPlatform, keybindings)) { return } - if (matchFileSearchShortcut(chord, shortcutPlatform, keybindings, terminalShortcutPolicy)) { + if (matchFileSearchShortcut(pressed, 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') { + const action = resolveShortcutEvent(pressed) + // Both of these arm the native-only tracker from the press so the OS still sees the + // gesture; arming from a release would leave the tracker holding a key that is already up. + if (!action || action.type === 'switchInputSource' || action.type === 'selectAll') { return } runShortcutAction(pressed, action, manager, true) } + const onSwallowedImeChordRelease = (e: KeyboardEvent): void => { + // In press order, because the Command release ends every chord held under it at once. + const released = [...pendingImeChords.values()].filter((chord) => + releasesPendingImeChord(chord, e) + ) + // Spent before any gate inside can refuse: the key is up, so the press it carried is over + // however this release is routed. Leaving one armed past a gate is what lets a carry + // outlive its gesture and answer for someone else's press. + for (const chord of released) { + pendingImeChords.delete(chord.code) + } + for (const chord of released) { + runSwallowedImeChord(chord, e) + } + } + window.addEventListener('keydown', onModifierDown, { capture: true }) window.addEventListener('keyup', onKeyUp, { capture: true }) window.addEventListener('keydown', onKeyDown, { capture: true }) @@ -1174,7 +1159,7 @@ export function useTerminalKeyboardShortcuts({ window.removeEventListener('keypress', onNativeOnlyShortcutCompanion, { capture: true }) window.removeEventListener('keyup', onNativeOnlyShortcutCompanion, { capture: true }) window.removeEventListener('beforeinput', onNativeOnlyBeforeInput, { capture: true }) - pendingImeChord = null + pendingImeChords.clear() window.removeEventListener('keyup', onSwallowedImeChordRelease, { capture: true }) window.removeEventListener('blur', onNativeOnlyBlur) } diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.ts b/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.ts index 788fa9b83baf..045e68e61b11 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.ts @@ -1,6 +1,9 @@ import type { IDisposable, Terminal } from '@xterm/xterm' import type { PtyTransport } from './pty-transport' +// Both are emitted only by config/patches/@xterm__xterm@6.1.0-beta.287.patch, not by upstream +// xterm. A regenerated patch that drops them leaves every waiter here silent rather than failing, +// and the deferred cursor chord in keyboard-handlers.ts waits with no deadline at all. export const XTERM_COMPOSITION_SESSION_START_EVENT = 'xterm-composition-session-start' export const XTERM_COMPOSITION_SESSION_END_EVENT = 'xterm-composition-session-end' 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 983f6ffa9162..b68a12ce970c 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shortcut-policy.ts @@ -6,7 +6,6 @@ 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 @@ -20,34 +19,19 @@ export type TerminalShortcutEvent = { 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 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, `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. +// ArrowUp/ArrowDown are absent because Cmd+↑/↓ scrolls the viewport and writes no bytes, so +// losing one to the IME costs nothing. 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 { - 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 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. + * True when a composing keydown is nonetheless an Orca terminal chord, which a Japanese + * conversion can swallow outright — no commit, no platform replay, nothing reaching the shell + * (#12871). On macOS the pane remembers one until its release, where a swallowed chord first + * becomes distinguishable from a delayed one (keyboard-handlers.ts, `imeChordSnapshot`). + * + * Reads `code`, not `key`: Windows reports an IME-consumed key as `key: 'Process'` (#12171, + * #13033). A lone modifier is excluded, so a mode-switch gesture such as composing Ctrl+Space + * never arms, and so is Shift, which Japanese conversion binds to resize a segment. */ export function isImeExemptTerminalChord(event: TerminalShortcutEvent): boolean { return ( @@ -151,7 +135,7 @@ export function resolveTerminalShortcutAction( hasCtrlEnterCsiUAuthority?: () => boolean ): TerminalShortcutAction | null { const platform: NodeJS.Platform = isMac ? 'darwin' : isWindows ? 'win32' : 'linux' - const chordKey = physicalChordKey(event) + // Why: capture this chord even on repeat without blocking the OS default input-source switch. if (keybindingMatchesAction('terminal.switchInputSource', event, platform, keybindings)) { return { type: 'switchInputSource' } @@ -259,23 +243,23 @@ export function resolveTerminalShortcutAction( !event.metaKey && !event.altKey && !event.shiftKey && - chordKey === 'Backspace' + event.key === 'Backspace' ) { return { type: 'sendInput', data: '\x17' } } if (isMac && event.metaKey && !event.ctrlKey && !event.altKey && !event.shiftKey) { - if (chordKey === 'Backspace') { + if (event.key === 'Backspace') { return { type: 'sendInput', data: '\x15' } } - if (chordKey === 'Delete') { + if (event.key === '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 (chordKey === 'ArrowLeft') { + if (event.key === 'ArrowLeft') { return { type: 'sendInput', data: '\x01' } } - if (chordKey === 'ArrowRight') { + if (event.key === 'ArrowRight') { return { type: 'sendInput', data: '\x05' } } // Why: macOS users expect Cmd+↑/↓ to scroll scrollback, not write escape bytes to the shell. @@ -292,7 +276,7 @@ export function resolveTerminalShortcutAction( !event.ctrlKey && event.altKey && !event.shiftKey && - chordKey === 'Backspace' + event.key === 'Backspace' ) { // Why: a kitty-protocol TUI binds the CSI 127;3u xterm emits natively; the legacy \x1b\x7f fallback would bypass it. if (isKittyKeyboardActivePane?.()) { @@ -306,14 +290,14 @@ export function resolveTerminalShortcutAction( !event.ctrlKey && event.altKey && !event.shiftKey && - (chordKey === 'ArrowLeft' || chordKey === 'ArrowRight') + (event.key === 'ArrowLeft' || event.key === '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: chordKey === 'ArrowLeft' ? '\x1bb' : '\x1bf' } + return { type: 'sendInput', data: event.key === 'ArrowLeft' ? '\x1bb' : '\x1bf' } } if ( @@ -322,14 +306,14 @@ export function resolveTerminalShortcutAction( event.ctrlKey && !event.altKey && !event.shiftKey && - (chordKey === 'ArrowLeft' || chordKey === 'ArrowRight') + (event.key === 'ArrowLeft' || event.key === '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: chordKey === 'ArrowLeft' ? '\x1bb' : '\x1bf' } + return { type: 'sendInput', data: event.key === 'ArrowLeft' ? '\x1bb' : '\x1bf' } } // Why: macOptionIsMeta stays off so non-US layouts can compose @/€; match event.code since composition rewrites event.key. diff --git a/src/shared/keybindings.ts b/src/shared/keybindings.ts index 974bca5feaa4..43c30d89d909 100644 --- a/src/shared/keybindings.ts +++ b/src/shared/keybindings.ts @@ -1624,9 +1624,7 @@ const PUNCTUATION_KEY_TOKENS = new Set([ 'Backquote' ]) -// '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 PHYSICAL_CODE_FALLBACK_KEYS = new Set(['', 'Dead', 'Unidentified']) const SHIFTED_PUNCTUATION_KEY_TOKENS: Record = { '<': 'Comma', From 86a9c1949c0ec454b860f17e93144ac357907433 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 15 Aug 2026 21:38:51 +0900 Subject: [PATCH 3/9] fix(terminal): claim only the presses the release answers for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit made the arming keydown consume the press, to stop a global handler and this pane both acting on one remapped chord. It consumed too much: the claim happened before anything decided whether this pane would answer. `isImeExemptTerminalChord` accepts any of Cmd/Ctrl/Alt over the four codes, and some of those resolve to nothing in the terminal policy. `Cmd+Alt+Left` is one, and it is the default binding for worktree history, owned by a window handler that mounts after this pane. Composing on macOS, the pane took that press, stopped it reaching the global handler, and then answered for nothing at the release. Worktree history back stopped working for the length of a composition — no remap needed, and it worked on the parent commit. The same early `preventDefault` also broke `switchInputSource`, whose whole contract is that the OS keeps its default action while xterm is cut off. One predicate now decides both ends: the press is claimed exactly when the release would answer for it, so the two cannot drift. `switchInputSource` and `selectAll` stay out of it — both arm the native-only tracker from a press, which a release cannot do, so leaving them unclaimed keeps them on the keydown path where they already work. Also restores coverage for the snapshot's silent failure mode. The test dropped in the simplification pass exercised the resolver, not the snapshot, so it could not have caught a `{ ...event }` regression at all — that is the whole hazard, since happy-dom keeps KeyboardEvent fields as own properties and Chromium does not. `imeChordSnapshot` is exported and pinned directly against a prototype-only object instead. And records why the release stepping back is believed safe beyond the two recorded input sources: the replay is Chromium redispatching a key the IME did not consume, so a source that consumed one is still composing and is handled. Closing the remaining gap would need a deadline on the replay, which is a second timer able to fire the chord twice. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WjefjguNCzpY2kVq3Q69wH --- ...ssue-12871-exempt-chord-resolution.test.ts | 32 ++++++++- ....issue-12871-recorded-chord-traces.test.ts | 41 ++++++++++++ .../terminal-pane/keyboard-handlers.ts | 66 +++++++++++++------ 3 files changed, 119 insertions(+), 20 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts index 46fa9d70bd0e..6c91c7df56c6 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-exempt-chord-resolution.test.ts @@ -4,7 +4,7 @@ // Korean and Japanese traces through the real handler and xterm. Why the two input sources need // different answers is in `isImeExemptTerminalChord` (terminal-shortcut-policy.ts). import { describe, expect, it } from 'vitest' -import { resolveTerminalKeyboardShortcutAction } from './keyboard-handlers' +import { imeChordSnapshot, resolveTerminalKeyboardShortcutAction } from './keyboard-handlers' import { isImeExemptTerminalChord, type TerminalShortcutEvent } from './terminal-shortcut-policy' // `isComposing` alone decides ownership; `keyCode` is carried so each fixture reads as the @@ -63,6 +63,36 @@ describe('terminal chords stay live during an IME composition', () => { }) }) + // Chromium keeps a KeyboardEvent's fields as accessors on the prototype, so a spread copies + // nothing and the remembered chord loses its `code` — every swallowed chord then goes silently + // undelivered. happy-dom keeps them as own properties, so replaying real events cannot see it. + // Modelling the browser's shape explicitly is the only form of that check which runs here. + it('copies fields that live only on the prototype, as a browser reports them', () => { + const fields = { + key: 'Process', + code: 'ArrowLeft', + metaKey: true, + ctrlKey: false, + altKey: false, + shiftKey: false + } + const prototype = Object.create( + null, + Object.fromEntries( + Object.entries(fields).map(([name, value]) => [name, { get: () => value }]) + ) + ) as KeyboardEvent + const event = Object.create(prototype) as KeyboardEvent + // What a spread would have to work with. + expect(Object.keys(event)).toEqual([]) + + const chord = imeChordSnapshot(event) + expect(chord.code).toBe('ArrowLeft') + expect(chord.metaKey).toBe(true) + // The code, not the rewritten `key`. + expect(chord.key).toBe('ArrowLeft') + }) + // The gate that decides whether a composing keydown is remembered for its release. Asserted // directly rather than through the resolver: the resolver answers for every binding in the // registry, so a new one landing there would break these rows for a reason unrelated to the 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 20a725d57568..7aa393dc2b5b 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 @@ -737,6 +737,47 @@ describe('constructed shapes the macOS captures do not contain', () => { rig.unmount() }) + // Exempt by code and modifier, but the terminal policy resolves it to nothing: Cmd+Alt+Arrow is + // the worktree history binding, owned by a window handler that mounts after this pane. Claiming + // a press this pane never answers would kill that binding for the length of a composition. + it('leaves a press it will not answer for to the handlers behind it', 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) + + const seenBehindUs: string[] = [] + const later = (event: Event): void => { + seenBehindUs.push((event as KeyboardEvent).code) + } + window.addEventListener('keydown', later, { capture: true }) + const press = new KeyboardEvent('keydown', { + key: 'ArrowLeft', + code: 'ArrowLeft', + metaKey: true, + altKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperties(press, { isComposing: { value: true }, keyCode: { value: 229 } }) + rig.textarea.dispatchEvent(press) + await macrotask() + window.removeEventListener('keydown', later, { capture: true }) + + expect(seenBehindUs).toEqual(['ArrowLeft']) + expect(press.defaultPrevented).toBe(false) + // And nothing is left armed to fire on the release either. + await replay(rig.textarea, [ + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true } + ]) + await commitComposition(rig.textarea, '日本語') + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + // The pane takes the press when it remembers it, so a global handler bound to the same chord // must not also act on it — that fires the remapped action twice, once per phase. it('consumes the keydown it remembers', async () => { diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 3d049e7c54ce..82d0d929d87e 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -80,11 +80,12 @@ type PendingImeChord = Parameters[0] & * on the auto-repeat instead would have to run before the two input sources separate, so this * waits for a trace showing a user holds a chord mid-preedit. */ -function imeChordSnapshot(event: KeyboardEvent): PendingImeChord { +export 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. + // 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, so no + // test replaying real events can see that — which is why this function is exported and pinned + // directly against a prototype-only object. return { // The code, not `key`: a CJK source rewrites `key` to 'Process' (#12171, #13033) and every // consumer below matches on `key`. Safe because the caller has already checked this code is @@ -581,18 +582,25 @@ export function useTerminalKeyboardShortcuts({ // delivers no keyup — and an IME that commits without replaying would lose the chord // outright here. Elsewhere the paths below keep sending it once the composition commits, // which is late rather than never. - if (isMac && isImeOwnedKeyboardEvent(e) && 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 (!isEditableTarget(e.target)) { - pendingImeChords.set(e.code, imeChordSnapshot(e)) - // Claimed here rather than at the release, so no other window listener acts on a press - // this pane has already taken. Without it the same chord fires twice on a remap: once - // from a global handler on the press, once from here on the release. - e.preventDefault() - e.stopImmediatePropagation() - } + // 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. + const exemptChord = + isMac && + isImeOwnedKeyboardEvent(e) && + isImeExemptTerminalChord(e) && + !isEditableTarget(e.target) + ? imeChordSnapshot(e) + : null + // Claimed on the press, so no other window listener acts on a chord this pane has already + // taken — otherwise a remap fires twice, once globally on the press and once here on the + // release. Only what the release will answer for, though: `Cmd+Alt+←` is exempt but resolves + // to nothing here, because the worktree history binding owns it, and claiming it would + // swallow a press this pane never answers. + if (exemptChord && answersSwallowedImeChord(exemptChord)) { + pendingImeChords.set(e.code, exemptChord) + e.preventDefault() + e.stopImmediatePropagation() return } @@ -1087,6 +1095,12 @@ export function useTerminalKeyboardShortcuts({ } // 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. + // + // Generalising from the two recorded input sources: the replay is Chromium redispatching a + // key the IME did not consume, so an IME that consumed one is still composing here and is + // handled below. An input source that both ended its composition and consumed the key would + // fall through this branch and lose the chord — no recording shows one, and closing that + // would need a deadline on the replay, which is a second timer able to fire the chord twice. if (!isImeOwnedKeyboardEvent(e)) { return } @@ -1117,14 +1131,28 @@ export function useTerminalKeyboardShortcuts({ } } const action = resolveShortcutEvent(pressed) - // Both of these arm the native-only tracker from the press so the OS still sees the - // gesture; arming from a release would leave the tracker holding a key that is already up. - if (!action || action.type === 'switchInputSource' || action.type === 'selectAll') { + if (!action || !answersSwallowedImeChord(pressed)) { return } runShortcutAction(pressed, action, manager, true) } + /** + * Whether the release path answers for this chord — read on the press too, so the pane claims + * exactly what it will answer for and nothing else. + * + * `switchInputSource` and `selectAll` are excluded because both arm the native-only tracker + * from a press so the OS still sees the gesture, which a release cannot do. Leaving them + * unclaimed keeps them on the keydown path, where they already work. + */ + function answersSwallowedImeChord(chord: PendingImeChord): boolean { + if (matchFileSearchShortcut(chord, shortcutPlatform, keybindings, terminalShortcutPolicy)) { + return true + } + const action = resolveShortcutEvent(chord) + return action !== null && action.type !== 'switchInputSource' && action.type !== 'selectAll' + } + const onSwallowedImeChordRelease = (e: KeyboardEvent): void => { // In press order, because the Command release ends every chord held under it at once. const released = [...pendingImeChords.values()].filter((chord) => From 27f3cf90db3a192983134e328c03087affea6a8c Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 15 Aug 2026 21:38:51 +0900 Subject: [PATCH 4/9] fix(terminal): stop a file-search match from claiming a selectAll chord MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The claim predicate returned early on a file-search match without looking at the action, so a chord carrying both bindings was claimed on the press, and a release that found no selection fell through to selectAll — arming the native-only tracker from a keyup with no press left to spend it. Resolving the action first applies the exclusion to both paths. Two things about the recording harnesses, both of which could produce a wrong gesture rather than a wrong assertion: `pressChord` existed twice with the same name and different behaviour. The driver's folded the modifier into the target key's flags; the spec's local one sent it as its own key event. Three specs called it and two got the folded shape, which produces no modifier press or release at all — the exact difference that made the Cmd half of the gesture look like it had no end. They are now `pressChordWithFoldedModifier` and `pressChordAsTyped`, so a call site says which gesture it drives. The cursor-chord spec kept its own copies of the input-source constants, the key table, and four helpers whose bodies match the driver's. The key table had already drifted to a different name for Return. Deleted in favour of the shared ones; 83 lines, and nothing left to drift. Also: a literal left-to-right mark in the composition reader is now `‎`, so a whitespace-stripping tool cannot silently break composition reads, matching the sibling spec; and the assertion that a swallowed chord leaves the preedit alive reads once instead of polling, since the settle it follows means a late preedit would be a failure rather than a pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WjefjguNCzpY2kVq3Q69wH --- ....issue-12871-recorded-chord-traces.test.ts | 68 +++++++++++ .../terminal-pane/keyboard-handlers.ts | 13 +- tests/e2e/macos-input-source-driver.ts | 25 +++- tests/e2e/renderer-chord-event-probe.ts | 2 +- ...l-macos-chord-input-pipeline-probe.spec.ts | 16 +-- ...inal-macos-ime-cursor-chord-native.spec.ts | 113 ++++-------------- ...al-macos-kotoeri-chord-keyup-probe.spec.ts | 18 +-- 7 files changed, 140 insertions(+), 115 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 7aa393dc2b5b..69b217a54d39 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 @@ -737,6 +737,74 @@ describe('constructed shapes the macOS captures do not contain', () => { rig.unmount() }) + // selectAll and switchInputSource arm the native-only tracker from a press so the OS still sees + // the gesture, which a release cannot do. So they are left unclaimed and keep running on the + // keydown — the action still happens, just not through the recovery. + it('runs a remapped selectAll on the press rather than answering from the release', async () => { + const rig = openRig({ keybindings: { 'terminal.selectAll': ['Mod+Backspace'] } }) + const selectAll = vi.spyOn(rig.terminal, 'selectAll').mockImplementation(() => {}) + 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: 'Backspace', + code: 'Backspace', + keyCode: 229, + isComposing: true, + meta: true + }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true } + ]) + await commitComposition(rig.textarea, '日本語') + + // Once, from the keydown. A second call would mean the release answered for it too. + expect(selectAll).toHaveBeenCalledTimes(1) + expect(rig.inputCalls).toEqual([]) + rig.unmount() + }) + + // One chord can carry both bindings. Reading the file-search match first would claim the press, + // and a release that then finds no selection falls through to selectAll — arming the tracker + // from a keyup, with no press left to spend it. + it('does not claim a chord that is both file search and selectAll', async () => { + const rig = openRig({ + keybindings: { + 'sidebar.search.toggle': ['Mod+Backspace'], + 'terminal.selectAll': ['Mod+Backspace'] + } + }) + const selectAll = vi.spyOn(rig.terminal, 'selectAll').mockImplementation(() => {}) + vi.spyOn(rig.terminal, 'getSelection').mockReturnValue('') + 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) + + const press = new KeyboardEvent('keydown', { + key: 'Backspace', + code: 'Backspace', + metaKey: true, + bubbles: true, + cancelable: true + }) + Object.defineProperties(press, { isComposing: { value: true }, keyCode: { value: 229 } }) + rig.textarea.dispatchEvent(press) + await macrotask() + + // Already run, from the press. A claimed press would leave this at zero here and reach + // selectAll only from the keyup, which is the stale arm. + expect(selectAll).toHaveBeenCalledTimes(1) + rig.unmount() + }) + // Exempt by code and modifier, but the terminal policy resolves it to nothing: Cmd+Alt+Arrow is // the worktree history binding, owned by a window handler that mounts after this pane. Claiming // a press this pane never answers would kill that binding for the length of a composition. diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 82d0d929d87e..51c153f810a3 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -1146,11 +1146,16 @@ export function useTerminalKeyboardShortcuts({ * unclaimed keeps them on the keydown path, where they already work. */ function answersSwallowedImeChord(chord: PendingImeChord): boolean { - if (matchFileSearchShortcut(chord, shortcutPlatform, keybindings, terminalShortcutPolicy)) { - return true - } const action = resolveShortcutEvent(chord) - return action !== null && action.type !== 'switchInputSource' && action.type !== 'selectAll' + // Checked before the file-search match, not after: one chord can carry both, and a + // file-search release that finds no selection falls through to the action. + if (action?.type === 'switchInputSource' || action?.type === 'selectAll') { + return false + } + return ( + action !== null || + matchFileSearchShortcut(chord, shortcutPlatform, keybindings, terminalShortcutPolicy) + ) } const onSwallowedImeChordRelease = (e: KeyboardEvent): void => { diff --git a/tests/e2e/macos-input-source-driver.ts b/tests/e2e/macos-input-source-driver.ts index 13d4cd941806..ba62ac538bb0 100644 --- a/tests/e2e/macos-input-source-driver.ts +++ b/tests/e2e/macos-input-source-driver.ts @@ -80,7 +80,12 @@ export function pressChordWithSeparateModifier( execFileSync('swift', [POST_MODIFIER_CHORD, String(MODIFIER_KEY[modifier]), String(keyCode)]) } -export function pressChord( +/** + * The modifier folded into the target key's flags, which is what `using down` does. + * That produces no modifier press or release at all, so a recording taken through this cannot + * show where a gesture ends. Use `pressChordAsTyped` unless the folded shape is the point. + */ +export function pressChordWithFoldedModifier( processId: number, keyCode: number, modifier?: 'command' | 'option' @@ -91,3 +96,21 @@ export function pressChord( `tell application "System Events" to key code ${keyCode}${modifier ? ` using ${modifier} down` : ''}` ]) } + +/** + * The chord as a hand types it: the modifier as its own key event. macOS delivers no keyup for a + * key released while Command is held, so the modifier's release is that gesture's only end, and + * the folded form never produces one. + */ +export function pressChordAsTyped( + processId: number, + keyCode: number, + modifier?: 'command' | 'option' +): void { + if (modifier) { + pressChordWithSeparateModifier(processId, keyCode, modifier) + return + } + focusApp(processId) + execFileSync('osascript', ['-e', `tell application "System Events" to key code ${keyCode}`]) +} diff --git a/tests/e2e/renderer-chord-event-probe.ts b/tests/e2e/renderer-chord-event-probe.ts index da7f0caa3c97..155564ded9c6 100644 --- a/tests/e2e/renderer-chord-event-probe.ts +++ b/tests/e2e/renderer-chord-event-probe.ts @@ -11,7 +11,7 @@ export function readActiveComposition(page: Page): Promise { const composition = textarea?.parentElement?.querySelector( '.composition-view.active' ) - return composition?.textContent?.replaceAll('‎', '') ?? null + return composition?.textContent?.replaceAll('\u200e', '') ?? null }) } 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 9aeb9cb4d4e5..6006df1fac32 100644 --- a/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts +++ b/tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts @@ -22,7 +22,7 @@ import { KEY, KOTOERI_ROMAJI_ID, KOTOERI_ROMAJI_PARENT_ID, - pressChord, + pressChordWithFoldedModifier, pressChordWithSeparateModifier, selectInputSource, TWO_SET_KOREAN_ID, @@ -98,11 +98,11 @@ test.describe('macOS chord input pipeline probe @headful', () => { await installMainProcessInputProbe(electronApp) await installChordProbe(orcaPage) - pressChord(processId, KEY.left, 'command') + pressChordWithFoldedModifier(processId, KEY.left, 'command') await orcaPage.waitForTimeout(800) - pressChord(processId, KEY.left, 'option') + pressChordWithFoldedModifier(processId, KEY.left, 'option') await orcaPage.waitForTimeout(800) - pressChord(processId, KEY.left) + pressChordWithFoldedModifier(processId, KEY.left) await orcaPage.waitForTimeout(800) const mainRows = await readMainProcessInputProbe(electronApp) @@ -143,10 +143,10 @@ test.describe('macOS chord input pipeline probe @headful', () => { await installMainProcessInputProbe(electronApp) await installChordProbe(orcaPage) - pressChord(processId, KEY.left, 'command') + pressChordWithFoldedModifier(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) + pressChordWithFoldedModifier(processId, KEY.left) await orcaPage.waitForTimeout(800) const mainRows = await readMainProcessInputProbe(electronApp) @@ -259,10 +259,10 @@ test.describe('macOS chord input pipeline probe @headful', () => { await orcaPage.waitForTimeout(1_200) // A live preedit eats the first Return; the second flushes the line. - pressChord(processId, KEY.returnKey) + pressChordWithFoldedModifier(processId, KEY.returnKey) let bytes = await waitForTerminalImeBytes(orcaPage, reader, 5_000).catch(() => []) if (bytes.length === 0) { - pressChord(processId, KEY.returnKey) + pressChordWithFoldedModifier(processId, KEY.returnKey) bytes = await waitForTerminalImeBytes(orcaPage, reader, 10_000).catch(() => []) } writeEvidence('kotoeri-command-release-pty.json', bytes) 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 d976931f9e4b..f6904ac01184 100644 --- a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts +++ b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts @@ -1,11 +1,20 @@ // 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' import { expect, test } from './helpers/orca-app' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' -import { pressChordWithSeparateModifier } from './macos-input-source-driver' +import { + ABC_ID, + bounceFocus, + enableInputSource, + KEY, + KOTOERI_ROMAJI_ID, + KOTOERI_ROMAJI_PARENT_ID, + pressChordAsTyped, + selectInputSource, + TWO_SET_KOREAN_ID, + typeKeyCodes +} from './macos-input-source-driver' import { focusActiveTerminalInput, getTerminalContent, @@ -43,88 +52,6 @@ import { * 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. 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) - execFileSync('osascript', ['-e', `tell application "System Events" to key code ${keyCode}`]) -} - 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. @@ -174,11 +101,11 @@ async function flushLineToReader( processId: number, reader: TerminalImeByteReader ): Promise { - pressChord(processId, KEY.return) + pressChordAsTyped(processId, KEY.returnKey) try { return await waitForTerminalImeBytes(page, reader, 5_000) } catch { - pressChord(processId, KEY.return) + pressChordAsTyped(processId, KEY.returnKey) return waitForTerminalImeBytes(page, reader, 10_000) } } @@ -248,7 +175,7 @@ test.describe('Native macOS IME cursor chords during composition @headful', () = typeKeyCodes(processId, [5, 40]) await expect.poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }).toBe('하') - pressChord(processId, KEY.left, chord.modifier) + pressChordAsTyped(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. @@ -305,11 +232,13 @@ test.describe('Native macOS IME cursor chords during composition @headful', () = .toMatch(/[ぁ-ん]/) } - pressChord(processId, KEY.left, 'command') + pressChordAsTyped(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. + // Read once rather than polled: the wait above is the settle, so the preedit is either + // still there now or the chord committed it. Polling would let a late one pass. await orcaPage.waitForTimeout(700) - await expect.poll(() => readActiveComposition(orcaPage)).toMatch(/[ぁ-ん]/) + expect(await 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 @@ -347,8 +276,8 @@ test.describe('Native macOS IME cursor chords during composition @headful', () = .poll(() => getTerminalContent(orcaPage, 100_000), { timeout: 10_000 }) .toContain('abc') - pressChord(processId, KEY.left, 'command') - pressChord(processId, KEY.left, 'option') + pressChordAsTyped(processId, KEY.left, 'command') + pressChordAsTyped(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. 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 c49d5e331bfc..ec7a62b7f2d6 100644 --- a/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts +++ b/tests/e2e/terminal-macos-kotoeri-chord-keyup-probe.spec.ts @@ -15,7 +15,7 @@ import { KEY, KOTOERI_ROMAJI_ID, KOTOERI_ROMAJI_PARENT_ID, - pressChord, + pressChordWithFoldedModifier, selectInputSource, TWO_SET_KOREAN_ID, typeKeyCodes @@ -86,10 +86,10 @@ test.describe('Kotoeri chord keyup probe @headful', () => { } await installChordProbe(orcaPage) - pressChord(processId, KEY.left, 'command') + pressChordWithFoldedModifier(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) + pressChordWithFoldedModifier(processId, KEY.left) await orcaPage.waitForTimeout(800) const rows = await readChordProbe(orcaPage) @@ -123,11 +123,11 @@ test.describe('Kotoeri chord keyup probe @headful', () => { await focusActiveTerminalInput(orcaPage) await installChordProbe(orcaPage) - pressChord(processId, KEY.left, 'command') + pressChordWithFoldedModifier(processId, KEY.left, 'command') await orcaPage.waitForTimeout(800) - pressChord(processId, KEY.left, 'option') + pressChordWithFoldedModifier(processId, KEY.left, 'option') await orcaPage.waitForTimeout(800) - pressChord(processId, KEY.left) + pressChordWithFoldedModifier(processId, KEY.left) await orcaPage.waitForTimeout(800) const rows = await readChordProbe(orcaPage) @@ -170,7 +170,7 @@ test.describe('Kotoeri chord keyup probe @headful', () => { .toMatch(/[\u3041-\u3093]/) await installChordProbe(orcaPage) - pressChord(processId, KEY.left, 'option') + pressChordWithFoldedModifier(processId, KEY.left, 'option') await orcaPage.waitForTimeout(1_200) const rows = await readChordProbe(orcaPage) @@ -189,10 +189,10 @@ test.describe('Kotoeri chord keyup probe @headful', () => { // A live preedit eats the first Return; the second one flushes the line (same as the // contributor's flushLineToReader). - pressChord(processId, 36) + pressChordWithFoldedModifier(processId, 36) let bytes = await waitForTerminalImeBytes(orcaPage, reader, 5_000).catch(() => []) if (bytes.length === 0) { - pressChord(processId, 36) + pressChordWithFoldedModifier(processId, 36) bytes = await waitForTerminalImeBytes(orcaPage, reader, 10_000).catch(() => []) } console.log('KOTOERI_OPTION_PTY', JSON.stringify(bytes)) From c8c2db9056b58804b5853f03600360b61d5d608e Mon Sep 17 00:00:00 2001 From: hyeonho Date: Sat, 15 Aug 2026 22:40:43 +0900 Subject: [PATCH 5/9] 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+Right moved the syllable to the line end. Reported with reproduction steps in #12871. Written originally in 8aec6254af against keyboard-handlers-ime-deferred-input.test.tsx, which no branch still has. Rehomed onto keyboard-handlers-ime-composing-chord.test.tsx, whose harness is the same shape, and reshaped for where the chord now resolves: a marked press is remembered for its release rather than deferred to the commit, so the macOS cases run the whole Korean gesture — press, commit, unmarked release, the platform's replay — instead of stopping at the commit. - Option+ArrowLeft / Option+ArrowRight / Cmd+ArrowRight, each a resolver branch no recorded trace reaches - Ctrl+ArrowLeft on Windows, where no release takes part at all The Windows rows were transcribed from the same macOS session rather than captured on win32, so they pin the resolver's non-mac branch and claim no platform coverage. Cmd+ArrowLeft behind a multi-character Japanese preedit is left out: the existing 'holds the chord while the keydown is still marked composing' already commits 日本語 behind the same chord for the same bytes. Its note about the overwritten destination glyph moved onto that test instead of arriving as a second copy of it. Verified non-vacuous against a663f1bd00, the parent: all three macOS cases fail there with the chord byte twice over ('사', '\eb', '\eb') — the Korean duplicate itself. The Windows case passes there and fails against b849099045, before the deferral landed, with the byte ahead of the commit. --- ...oard-handlers-ime-composing-chord.test.tsx | 97 ++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx index db5aaae185f4..e1e7a90d062f 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx @@ -185,7 +185,9 @@ describe('a cursor chord pressed during a composition', () => { harness.dispose() }) - // The Japanese shape: still marked composing when the chord is resolved. + // The Japanese shape: still marked composing when the chord is resolved. The preedit spans + // several characters here on purpose — a relocated multi-character preedit also overwrote the + // glyph already at the destination cell, so the whole run has to commit in place first. it('holds the chord while the keydown is still marked composing', () => { const harness = createHarness() const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) @@ -227,4 +229,97 @@ describe('a cursor chord pressed during a composition', () => { hook.unmount() harness.dispose() }) + + /** + * The Korean 2-Set gesture end to end. The marked press is remembered rather than sent; the + * syllable commits while the key is still down, so the release arrives already unmarked and the + * recovery declines it; and the platform then replays the chord for the pane to resolve the + * ordinary way. One byte per press is the contract — a second one jumps two words. + */ + function playCommittingChord( + harness: ReturnType, + chord: { code: string; keyCode: number; mods: KeyboardEventInit } + ): void { + const { code, keyCode, mods } = chord + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { key: code, code, keyCode: 229, isComposing: true, ...mods }) + ) + harness.terminalInput.dispatchEvent( + keyboardEvent('keyup', { key: code, code, keyCode, ...mods }) + ) + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { key: code, code, keyCode, ...mods }) + ) + } + + // With "가나 다라 마바" on the line and "사" still composing, each of these relocated the + // composing syllable to wherever the cursor landed. The Cmd+← cases above reach neither the + // other direction nor Option's word jump, and each byte is a separate resolver branch. + it.each([ + { + name: 'Option+ArrowLeft word jump', + code: 'ArrowLeft', + keyCode: 37, + mods: { altKey: true }, + sent: '\x1bb' + }, + { + name: 'Option+ArrowRight word jump', + code: 'ArrowRight', + keyCode: 39, + mods: { altKey: true }, + sent: '\x1bf' + }, + { + name: 'Cmd+ArrowRight line-end jump', + code: 'ArrowRight', + keyCode: 39, + mods: { metaKey: true }, + sent: '\x05' + } + ])('sends a $name once, behind the syllable it committed', ({ code, keyCode, mods, sent }) => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.startComposition() + playCommittingChord(harness, { code, keyCode, mods }) + expect(harness.wire, 'nothing may reach the pty while the syllable is pending').toEqual([]) + + harness.endComposition('사') + vi.runAllTimers() + + expect(harness.wire).toEqual(['사', sent]) + hook.unmount() + harness.dispose() + }) + + // Off macOS the release-keyed recovery never arms, so the chord waits for the commit instead and + // no release takes part at all. Transcribed from the same macOS session as the cases above rather + // than captured on win32: what it pins is the resolver's non-mac branch, not the platform. + it('holds a Windows Ctrl+ArrowLeft word jump behind the composing syllable', () => { + vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue( + 'Mozilla/5.0 (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, + ctrlKey: true, + isComposing: true + }) + ) + expect(harness.wire).toEqual([]) + + harness.endComposition('사') + vi.runAllTimers() + + expect(harness.wire).toEqual(['사', '\x1bb']) + hook.unmount() + harness.dispose() + }) }) From 7e710f67ab8b8aa3da84ac145671f0a5edf274cd Mon Sep 17 00:00:00 2001 From: hyeonho Date: Sat, 15 Aug 2026 22:47:55 +0900 Subject: [PATCH 6/9] test(terminal): pin the Chinese chords, measured on the Command release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR body no longer calls Chinese untested — it says Pinyin and Zhuyin are affected. This replaces that citation with measurement, at both layers. Every earlier Chinese capture is void twice over: it predates the Command release being honoured, and it was driven with the modifier folded into the target key's flags, which emits no modifier press or release at all and so could not see the end those gestures turn on. What the recordings say. Both sources swallow the chord and keep composing, like Kotoeri and unlike Korean, and the byte drains at the commit. A Cmd chord's arrow keydown does not reach Chromium's own input dispatch at all and its keyup exists nowhere; the Command release carries it, still marked composing, and both Cmd cells pass only because of that. Option resolves through the arrow's own keyup, as it always did. One gesture is deliberately left unpinned at the unit layer. Recorded here, Zhuyin Option+Left commits on the chord; checked by hand at the same keyboard, the preedit block stays up. Three synthesis regimes were tried against that — a flat 80ms with the modifier as a synthetic key event, then the hardware medians and a much slower run with it posted as a real flagsChanged — and all three committed on the press, so it is not a timing artifact this rig can drive out. Rather than freeze a recording no hand can reproduce, that fixture case is dropped rather than softened. Its e2e case stays, minus the intermediate-composition assertion: both behaviours put the same line on the pty, so the byte order is asserted and the disputed state is not. The e2e cases assert the line's shape rather than a particular reading, because which candidate Zhuyin commits adapts to use — one run committed 你好, another 妳好, and the byte order was the same contract in both. Rebased from 878ae5ef with the expectations re-measured, as the mechanism moved on since: all three fixture cases now stop mid-composition and drain at the commit, so each carries the text its source committed and commitsAfterCapture holds that text rather than a bare flag — the rig had been committing さ for every case. Verified non-vacuous by removing the Command-release arm: both Cmd cases fail there alongside the Kotoeri one. The Option case is the half that already worked and passes either way, as its note says. --- ...ndlers.issue-12871-chinese-chord-traces.ts | 117 ++++++++++++ ...lers.issue-12871-command-release-traces.ts | 2 +- ...andlers.issue-12871-in-app-chord-traces.ts | 8 +- ....issue-12871-recorded-chord-traces.test.ts | 5 +- tests/e2e/macos-input-source-driver.ts | 6 + ...inal-macos-ime-cursor-chord-native.spec.ts | 177 ++++++++++++++++++ 6 files changed, 310 insertions(+), 5 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts new file mode 100644 index 000000000000..e4bbcdca7422 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts @@ -0,0 +1,117 @@ +// Recorded IME chord traces for #12871 from the two Chinese input sources, taken inside a dev e2e +// Orca build (app commit 84517d74b268bca888a44b285627ec82ffde1361, macOS 26.5.2/25F84, darwin +// arm64) at the xterm helper textarea, with PTY-side ground-truth bytes read by a child on the +// pty. Replayed by keyboard-handlers.issue-12871-recorded-chord-traces.test.ts through the same +// rig as the other recordings. Reproduce with the Chinese cases in +// tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts, which drives the same gestures against +// the real input sources and asserts the resulting line end to end. +// +// These replace an earlier Chinese recording entirely. That one was driven with the modifier +// folded into the target key's flags, which produces no modifier press or release at all, so it +// could not see the Command release these cases turn on — and it predates that release being +// honoured. Nothing from it survives here. +// +// What the three cases establish, measured rather than assumed: Chinese behaves like Kotoeri and +// not like Korean. Both sources swallow the chord and keep composing, and the byte drains at the +// commit. The two modifiers still reach the handler by different routes, and each case pins the +// route it recorded: +// +// - `Cmd+←` delivers no arrow keyup, at any listener position or at Chromium's own input +// dispatch. The `Cmd` release is the gesture's only end and arrives still marked composing. +// - `Option+←` delivers the arrow's own keyup and resolves through it. +// +// A fourth cell, Zhuyin `Option+←`, was recorded and then DELIBERATELY NOT INCLUDED. Under every +// synthesis this rig can produce it commits the composition on the chord, and a human at the same +// keyboard cannot reproduce that — the preedit block stays up for them, as it does for Pinyin and +// Kotoeri. Three timing regimes were tried (a flat 80ms with the modifier as a synthetic key +// event, then the hardware medians 190/110/160ms and 700/400/500ms with the modifier posted as a +// real flagsChanged) and all three committed on the press. Rather than freeze a recording no hand +// can reproduce, the cell is left out; the end-to-end byte order for that gesture is still +// asserted in terminal-macos-ime-cursor-chord-native.spec.ts, where it holds under both +// behaviours. The raw recordings and the human cross-check behind that call are attached to +// stablyai/orca#12732, where this round was measured. +import type { RecordedChordCase } from './keyboard-handlers.issue-12871-in-app-chord-traces' + +export const CHINESE_TRACE_CASES: RecordedChordCase[] = [ + { + // cn-zhuyin-command.json. Composition view read 你好 before and after the press; captured + // PTY line 你好\x01\n. The rows below stop at the Command release, so what is replayed is + // the gesture alone, without the commit that follows it. + name: 'Traditional Zhuyin, Cmd+ArrowLeft over a live 你好 preedit, ended by the Command release', + expectCalls: ['\x01'], + expectEmitted: ['\x01'], + commitsAfterCapture: '你好', + 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, exactly as in the Kotoeri recording. + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: false } + ] + }, + { + // cn-pinyin-nocand-command.json. Captured PTY line nihao\x01\n — Pinyin's Return commits the + // letters rather than the highlighted candidate, so the committed text is ASCII while the + // composition is unmistakably live (the view reads `ni hao`, segmented by the IME). + // + // The composition update between the press and the Command release is the point of keeping + // this case alongside the Zhuyin one: the carry has to survive the IME editing its own + // preedit mid-gesture. A recovery that disarmed on composition activity would drop the byte + // here and still pass every other case in this file. + name: 'Simplified Pinyin, Cmd+ArrowLeft over a live ni hao preedit updated mid-gesture', + expectCalls: ['\x01'], + expectEmitted: ['\x01'], + commitsAfterCapture: 'nihao', + 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: 'ni hao', value: 'ni hao' }, + { t: 'input', data: 'ni hao', value: 'ni hao' }, + { t: 'keyup', key: 'Meta', code: 'MetaLeft', keyCode: 91, isComposing: true, meta: false } + ] + }, + { + // cn-pinyin-nocand-option.json. Captured at the boundary as nihao\x1bb. The arrow's own keyup + // arrives here, still marked composing, and spends the carry before the Alt release can — + // the half that already worked, pinned so it fails if it stops. + name: 'Simplified Pinyin, Option+ArrowLeft over a live ni hao preedit, ended by the arrow keyup', + expectCalls: ['\x1bb'], + expectEmitted: ['\x1bb'], + commitsAfterCapture: 'nihao', + rows: [ + { 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: 'ni hao', value: 'ni hao' }, + { t: 'input', data: 'ni hao', value: 'ni hao' }, + { + t: 'keyup', + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 37, + isComposing: true, + alt: true + }, + { t: 'keyup', key: 'Alt', code: 'AltLeft', keyCode: 18, isComposing: false, alt: false } + ] + } +] 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 f5b022941ad0..fc13b04647e2 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 @@ -27,7 +27,7 @@ export const COMMAND_RELEASE_TRACE_CASES: RecordedChordCase[] = [ expectCalls: ['\x01'], expectEmitted: ['\x01'], // Kotoeri is still converting when the capture stops. - commitsAfterCapture: true, + commitsAfterCapture: 'さ', 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 c5adc8426770..a3b8994c8d0c 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,8 +44,12 @@ export type RecordedChordCase = { expectCalls: string[] expectEmitted: string[] rows: RecordedChordRow[] - /** Rows end mid-composition: assert nothing sent yet, then drive a commit and check again. */ - commitsAfterCapture?: true + /** + * Rows end mid-composition: assert nothing sent yet, then drive a commit and check again. The + * value is the text that source committed. xterm emits none of it — the capture opens after the + * session it would have to match — so it names the recording rather than feeding an assertion. + */ + commitsAfterCapture?: string } export const IN_APP_TRACE_CASES: RecordedChordCase[] = [ 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 69b217a54d39..835fac843158 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 @@ -39,6 +39,7 @@ 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 { CHINESE_TRACE_CASES } from './keyboard-handlers.issue-12871-chinese-chord-traces' import { COMMAND_RELEASE_TRACE_CASES } from './keyboard-handlers.issue-12871-command-release-traces' import { IN_APP_TRACE_CASES, @@ -442,7 +443,7 @@ afterEach(() => { describe('recorded macOS chord traces during an IME composition', () => { it.each( - [...CASES, ...IN_APP_TRACE_CASES, ...COMMAND_RELEASE_TRACE_CASES].map( + [...CASES, ...IN_APP_TRACE_CASES, ...COMMAND_RELEASE_TRACE_CASES, ...CHINESE_TRACE_CASES].map( (testCase) => [testCase.name, testCase] as const ) )('%s', async (_name, testCase) => { @@ -452,7 +453,7 @@ describe('recorded macOS chord traces during an IME composition', () => { 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, 'さ') + await commitComposition(rig.textarea, testCase.commitsAfterCapture) } expect(rig.inputCalls).toEqual(testCase.expectCalls) // Joined: the captures were taken when xterm flushed preedit and chord as one payload, and diff --git a/tests/e2e/macos-input-source-driver.ts b/tests/e2e/macos-input-source-driver.ts index ba62ac538bb0..374e878bf9bd 100644 --- a/tests/e2e/macos-input-source-driver.ts +++ b/tests/e2e/macos-input-source-driver.ts @@ -9,6 +9,12 @@ import path from 'node:path' 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' +// Chinese, both scripts. Each needs its parent enabled first for the same reason Kotoeri does: +// an enable-everything-for-this-id pass cannot reach a mode whose parent is still disabled. +export const SIMPLIFIED_PINYIN_ID = 'com.apple.inputmethod.SCIM.ITABC' +export const SIMPLIFIED_PINYIN_PARENT_ID = 'com.apple.inputmethod.SCIM' +export const TRADITIONAL_ZHUYIN_ID = 'com.apple.inputmethod.TCIM.Zhuyin' +export const TRADITIONAL_ZHUYIN_PARENT_ID = 'com.apple.inputmethod.TCIM' export const ABC_ID = 'com.apple.keylayout.ABC' export const KEY = { left: 123, backspace: 51, returnKey: 36, s: 1, a: 0 } as const 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 f6904ac01184..0690a0296f71 100644 --- a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts +++ b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts @@ -12,6 +12,10 @@ import { KOTOERI_ROMAJI_PARENT_ID, pressChordAsTyped, selectInputSource, + SIMPLIFIED_PINYIN_ID, + SIMPLIFIED_PINYIN_PARENT_ID, + TRADITIONAL_ZHUYIN_ID, + TRADITIONAL_ZHUYIN_PARENT_ID, TWO_SET_KOREAN_ID, typeKeyCodes } from './macos-input-source-driver' @@ -45,6 +49,9 @@ import { * 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. + * - Chinese (Zhuyin and Pinyin, measured 2026-08-09) behaves like Kotoeri and not like Korean: + * both swallow the chord, keep the preedit, and drain the byte at the commit. A Cmd chord + * there has no arrow keyup at all, so those two cells rest entirely on the Command release. * - ABC control: no IME anywhere, movement bytes flow alone. * * Run-validity guards follow terminal-macos-korean-chord-commit-native.spec.ts: the selected @@ -255,6 +262,176 @@ test.describe('Native macOS IME cursor chords during composition @headful', () = } }) + /** + * Chinese, both scripts, both chords. Measured on this build 2026-08-09, driven with the + * modifier as its own key event — every earlier Chinese capture folded it into the arrow's + * flags, which produces no modifier press or release at all, and so could not see the Command + * release that ends the gesture. The unit-layer half of the same recordings is in + * src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts. + * + * The four cells do not behave alike, and the table below records the differences rather than + * smoothing them: + * - `Cmd+←` on both sources delivers NO arrow keyup, at any listener position or at + * Chromium's own input dispatch. The Command release is the gesture's only end and arrives + * still marked composing. Both those cells pass only because that release is honoured; + * before it the byte was dropped and the line arrived without it. + * - `Option+←` delivers the arrow's own keyup and resolves through it, as it always did. + * Pinned so the half that already worked fails loudly if it stops. + * - Pinyin keeps composing through either chord, and so does Zhuyin through `Cmd+←`. + * `composesThroughChord` is that measurement, and it doubles as those cells' positive + * control. It is deliberately absent from the Zhuyin `Option+←` cell: synthesized chords + * make that source commit on the press at every timing this rig can produce, while a human + * at the same keyboard keeps the preedit. Since the two disagree, the intermediate state is + * not asserted there — only the byte contract below, which holds either way. + * In every cell the movement byte lands strictly after the committed text, exactly once. + */ + for (const cell of [ + { + script: 'Traditional Zhuyin', + id: TRADITIONAL_ZHUYIN_ID, + parentId: TRADITIONAL_ZHUYIN_PARENT_ID, + sourceIdPattern: /^com\.apple\.inputmethod\.TCIM(\.|$)/, + // Dachen layout: s=ㄋ u=ㄧ 3=ˇ then c=ㄏ l=ㄠ 3=ˇ. Zhuyin resolves bopomofo to hanzi inside + // the preedit, so the composition view already reads two characters before the chord. + keyCodes: [1, 32, 20, 8, 37, 20], + preedit: /[ㄅ-ㄩˇˊˋ˙一-鿿]/, + chord: 'Cmd+Left', + modifier: 'command' as const, + composesThroughChord: true, + commitReturns: 0, + // WHICH two hanzi is not this test's business and must not be: Zhuyin's candidate order + // adapts to use, and a run that committed 妳好 rather than 你好 pinned the byte just as + // well. The shape is the contract — the committed reading, then one movement byte, then + // the line end, with nothing before the reading and nothing after the byte. + byte: '\x01', + commit: /^[一-鿿]{2}$/ + }, + { + script: 'Traditional Zhuyin', + id: TRADITIONAL_ZHUYIN_ID, + parentId: TRADITIONAL_ZHUYIN_PARENT_ID, + sourceIdPattern: /^com\.apple\.inputmethod\.TCIM(\.|$)/, + keyCodes: [1, 32, 20, 8, 37, 20], + preedit: /[ㄅ-ㄩˇˊˋ˙一-鿿]/, + chord: 'Option+Left', + modifier: 'option' as const, + // No intermediate assertion here, on purpose. Driven by this rig the composition ends on + // the press; driven by a hand it survives and commits on the Return. Both routes put the + // same line on the pty, so the byte order below is asserted and the disputed state is not. + composesThroughChord: undefined, + commitReturns: 0, + byte: '\x1bb', + commit: /^[一-鿿]{2}$/ + }, + { + script: 'Simplified Pinyin', + id: SIMPLIFIED_PINYIN_ID, + parentId: SIMPLIFIED_PINYIN_PARENT_ID, + sourceIdPattern: /^com\.apple\.inputmethod\.SCIM(\.|$)/, + // nihao. The preedit reads back segmented as `ni hao`, and Return commits those LETTERS + // rather than the highlighted candidate (Space would take that), so the line is ASCII — + // which is why the composition view, not the committed text, is this cell's proof that an + // IME was engaged at all. + keyCodes: [45, 34, 4, 0, 31], + preedit: /^ni ?hao$/, + chord: 'Cmd+Left', + modifier: 'command' as const, + composesThroughChord: true, + commitReturns: 0, + byte: '\x01', + commit: /^nihao$/ + }, + { + script: 'Simplified Pinyin', + id: SIMPLIFIED_PINYIN_ID, + parentId: SIMPLIFIED_PINYIN_PARENT_ID, + sourceIdPattern: /^com\.apple\.inputmethod\.SCIM(\.|$)/, + keyCodes: [45, 34, 4, 0, 31], + preedit: /^ni ?hao$/, + chord: 'Option+Left', + modifier: 'option' as const, + composesThroughChord: true, + // This chord moves the caret between the preedit's segments, and the segmented preedit + // then costs one Return more than the Cmd cell: the first merges the segments, the next + // ends the composition. Spend that one here so flushLineToReader's own two presses mean + // the same thing in this cell as everywhere else in this file. + commitReturns: 1, + byte: '\x1bb', + commit: /^nihao$/ + } + ]) { + test(`${cell.script}: ${cell.chord} during composition puts its byte 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(cell.parentId) + enableInputSource(cell.id) + selectInputSource(cell.id) + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + // Chinese modes report back under the parent bundle id, the way Kotoeri reports under + // the legacy Japanese one. + await expect + .poll(() => readInputSourceId(orcaPage), { timeout: 10_000 }) + .toMatch(cell.sourceIdPattern) + + // Bounce timing can swallow the first keystrokes; one retry, as for Kotoeri. + typeKeyCodes(processId, cell.keyCodes) + try { + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 6_000 }) + .toMatch(cell.preedit) + } catch { + bounceFocus(processId) + await focusActiveTerminalInput(orcaPage) + typeKeyCodes(processId, cell.keyCodes) + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) + .toMatch(cell.preedit) + } + + pressChordAsTyped(processId, KEY.left, cell.modifier) + // The positive control, where the two drivers agree on it: a source that committed here + // would make the byte assertion below evidence about some other gesture. The one cell + // where synthesis and hardware disagree opts out rather than pinning the rig's answer. + await orcaPage.waitForTimeout(700) + if (cell.composesThroughChord === true) { + await expect + .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) + .toMatch(cell.preedit) + } + + for (let index = 0; index < cell.commitReturns; index += 1) { + pressChordAsTyped(processId, KEY.returnKey) + await orcaPage.waitForTimeout(400) + } + + // The commit drains the queued chord byte, then the line ends. Anchored end to end, so a + // byte ahead of the committed text (never queued) and a second copy of it (both releases + // fired) each fail here rather than passing as a substring. + const lines = await flushLineToReader(orcaPage, processId, setup.reader) + expect(lines).toHaveLength(1) + const line = Buffer.from(lines[0] ?? '', 'hex').toString('utf8') + expect(line.endsWith(`${cell.byte}\n`)).toBe(true) + const committed = line.slice(0, -(cell.byte.length + 1)) + expect(committed).toMatch(cell.commit) + // The same committed text has to be on screen, not merely in the byte stream. + expect(await getTerminalContent(orcaPage, 100_000)).toContain(committed) + } finally { + removeTerminalImeByteReader(setup.reader) + selectInputSource(TWO_SET_KOREAN_ID) + } + }) + } + test('ABC control: the same chords with no IME flow alone', async ({ electronApp, orcaPage, From fd71903ed67b56150b3635af7e8f6b7fc7b6c8bd Mon Sep 17 00:00:00 2001 From: hyeonho Date: Sat, 15 Aug 2026 23:32:29 +0900 Subject: [PATCH 7/9] test(e2e): stop a late preedit from passing the Chinese positive control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The positive control ran as a 10 s expect.poll immediately after the 700 ms settle, so it polled for a condition that must already hold. That hid a real failure two ways: a source that committed on the chord and then opened a fresh preedit satisfied the poll on the second one, which turns the byte assertion below into evidence about a different gesture; and when the composition was genuinely gone the run still burned the full timeout before reporting it. Read once instead — the shape the Kotoeri case already uses at the same point after the same settle. --- tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 0690a0296f71..334ffb7f3d52 100644 --- a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts +++ b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts @@ -402,11 +402,12 @@ test.describe('Native macOS IME cursor chords during composition @headful', () = // The positive control, where the two drivers agree on it: a source that committed here // would make the byte assertion below evidence about some other gesture. The one cell // where synthesis and hardware disagree opts out rather than pinning the rig's answer. + // Read once rather than polled, as in the Kotoeri test: the wait above is the settle, so + // the preedit is either still there now or the chord committed it. Polling would let a + // late one pass. await orcaPage.waitForTimeout(700) if (cell.composesThroughChord === true) { - await expect - .poll(() => readActiveComposition(orcaPage), { timeout: 10_000 }) - .toMatch(cell.preedit) + expect(await readActiveComposition(orcaPage)).toMatch(cell.preedit) } for (let index = 0; index < cell.commitReturns; index += 1) { From 84484e9264a7aabe9327eabaee1162f13b2e45e9 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:41:48 +0900 Subject: [PATCH 8/9] test(terminal): say what the re-homed cases pin, and close the empty-commit hole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the re-homing found three things worth changing, all in what the tests claim rather than in what they run. The macOS cases in keyboard-handlers-ime-composing-chord.test.tsx had a docstring narrating the whole gesture as though each step were asserted. Only the count is. Drop the marked press from those rows and they still pass — nothing is left that could fire twice — while dropping the arming makes them fail with the byte twice over. The docstring now says that, and points at the recorded-trace fixtures for which event carries the byte. commitsAfterCapture was read for truthiness, so a case committing the empty string would skip both the mid-gesture guard and the commit. Today that fails loudly, since every case expects a byte; a later case expecting none would have passed having asserted nothing. Read for presence instead. Its doc also now says plainly that the text is a label no assertion reads, so a wrong one misleads a reader rather than failing. The Chinese fixture claimed reproduction from a spec that checks those rows rather than producing them. The recorder, terminal-macos-chord-input-pipeline-probe.spec.ts, was never extended past Japanese, Korean and ABC, so these rows cannot be re-measured from this repo. Said so. And the deliberate Zhuyin Option+Left omission leaned on the e2e spec for that gesture's coverage without noting that no workflow sets ORCA_E2E_NATIVE_MACOS_KOREAN — so that gesture has no coverage that runs on its own, which the note now states. One review finding was not adopted: that the arrow keyup under a held Command contradicts the recorded "no keyup under Cmd". That claim belongs to the sources that swallow the chord, where the IME consumed the key. The Korean capture in keyboard-handlers.issue-12871-recorded-chord-traces.ts does carry an ArrowLeft keyup with metaKey set, right after compositionend. A comment now records the distinction where it was generalized. --- ...board-handlers-ime-composing-chord.test.tsx | 16 ++++++++++++---- ...andlers.issue-12871-chinese-chord-traces.ts | 18 +++++++++++------- ...handlers.issue-12871-in-app-chord-traces.ts | 7 ++++--- ...s.issue-12871-recorded-chord-traces.test.ts | 4 +++- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx index e1e7a90d062f..2630ca38d82f 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx @@ -231,10 +231,18 @@ describe('a cursor chord pressed during a composition', () => { }) /** - * The Korean 2-Set gesture end to end. The marked press is remembered rather than sent; the - * syllable commits while the key is still down, so the release arrives already unmarked and the - * recovery declines it; and the platform then replays the chord for the pane to resolve the - * ordinary way. One byte per press is the contract — a second one jumps two words. + * The Korean 2-Set gesture as recorded: marked press, commit, unmarked release, then the + * platform's replay of the chord. + * + * What the cases below pin is the count, not the route. The byte comes from the replay alone — + * drop the marked press from these rows and they still pass, because there is then nothing that + * could have fired a second time. Drop the arming instead and they fail with the byte twice, + * which is #12871's Korean half. So read them as "the remembered press adds nothing on top of + * the replay", and read the recorded-trace fixtures for which event carries the byte. + * + * The arrow keyup here is under a held Command, which the Korean capture does contain — the + * missing-keyup finding is about the sources that swallow the chord, where the IME consumed the + * key and only the Command release ends the gesture. */ function playCommittingChord( harness: ReturnType, diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts index e4bbcdca7422..6b6f9c666543 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts @@ -2,9 +2,11 @@ // Orca build (app commit 84517d74b268bca888a44b285627ec82ffde1361, macOS 26.5.2/25F84, darwin // arm64) at the xterm helper textarea, with PTY-side ground-truth bytes read by a child on the // pty. Replayed by keyboard-handlers.issue-12871-recorded-chord-traces.test.ts through the same -// rig as the other recordings. Reproduce with the Chinese cases in -// tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts, which drives the same gestures against -// the real input sources and asserts the resulting line end to end. +// rig as the other recordings. Unlike the Kotoeri and Korean families, these rows cannot be +// re-recorded from this repo: tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts is the +// recorder and was never extended past Japanese, Korean and ABC. The Chinese cases in +// tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts drive the same gestures and assert the +// resulting line end to end, but they check the rows rather than produce them. // // These replace an earlier Chinese recording entirely. That one was driven with the modifier // folded into the target key's flags, which produces no modifier press or release at all, so it @@ -26,10 +28,12 @@ // Kotoeri. Three timing regimes were tried (a flat 80ms with the modifier as a synthetic key // event, then the hardware medians 190/110/160ms and 700/400/500ms with the modifier posted as a // real flagsChanged) and all three committed on the press. Rather than freeze a recording no hand -// can reproduce, the cell is left out; the end-to-end byte order for that gesture is still -// asserted in terminal-macos-ime-cursor-chord-native.spec.ts, where it holds under both -// behaviours. The raw recordings and the human cross-check behind that call are attached to -// stablyai/orca#12732, where this round was measured. +// can reproduce, the cell is left out. Its byte order is still asserted in +// terminal-macos-ime-cursor-chord-native.spec.ts, where it holds under both behaviours — but that +// spec is hand-run behind ORCA_E2E_NATIVE_MACOS_KOREAN and no workflow sets it, so be plain about +// what that leaves: after this file, Zhuyin `Option+←` has no coverage that runs on its own. +// The raw recordings and the human cross-check behind the call are attached to stablyai/orca#12732, +// where this round was measured. import type { RecordedChordCase } from './keyboard-handlers.issue-12871-in-app-chord-traces' export const CHINESE_TRACE_CASES: RecordedChordCase[] = [ 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 a3b8994c8d0c..80b97c4d4109 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 @@ -45,9 +45,10 @@ export type RecordedChordCase = { expectEmitted: string[] rows: RecordedChordRow[] /** - * Rows end mid-composition: assert nothing sent yet, then drive a commit and check again. The - * value is the text that source committed. xterm emits none of it — the capture opens after the - * session it would have to match — so it names the recording rather than feeding an assertion. + * Rows end mid-composition: assert nothing sent yet, then drive a commit and check again. Holds + * the text that source committed, which no assertion reads — xterm emits none of it, the capture + * having opened after the session it would have to match. It is a label, so a wrong one here + * misleads a reader rather than failing; the PTY line in each case's comment is the record. */ commitsAfterCapture?: string } 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 835fac843158..cff97d4b537c 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 @@ -450,7 +450,9 @@ describe('recorded macOS chord traces during an IME composition', () => { const rig = openRig() await replay(rig.textarea, testCase.rows) - if (testCase.commitsAfterCapture) { + // Present, not truthy: a case that commits the empty string would otherwise skip both the + // mid-gesture guard and the commit, and assert nothing while still passing. + if (testCase.commitsAfterCapture !== undefined) { // Held, not dropped: nothing yet, and the same expectations must hold once it commits. expect(rig.inputCalls).toEqual([]) await commitComposition(rig.textarea, testCase.commitsAfterCapture) From 1bd326202b06ec2dd26805d9e2650a1025b4e61a Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:46:22 +0900 Subject: [PATCH 9/9] test(terminal): cut six claims down to what the assertions actually reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second review pass over the re-homed traces found six records claiming more than they measure. None of the assertions changed; the notes around them did. - The Zhuyin Cmd rows are byte-identical to the Kotoeri ones, so the header claim that each case pins the route it recorded was false for that case: replayed here it is Kotoeri under another name and cannot fail on its own. The identity is itself the measurement — a third input source producing the same shape — so it stays, said plainly, with the e2e cell named as the only place the real Zhuyin source is driven. - The Pinyin Cmd case justified itself by saying a recovery that disarmed on composition activity would drop its byte and pass everything else here. The Pinyin Option case carries the same compositionupdate rows and would fall with it. What the Cmd case adds is that route, not that property. - The multi-character Japanese preedit note read as though the glyph count were pinned. Shorten 日本語 to one character and the test still passes: where text lands on the grid is not visible to that harness. The note now says it records the reported symptom, not what the assertion reads. - The e2e header stated without qualification that both Chinese sources keep the preedit through the chord. Three of the four cells; the Zhuyin Option cell is the exception under this rig, which is why it opts out of the positive control. - `commitReturns: 1` was justified by "flushLineToReader's own two presses". It presses Return once and again only after a five-second wait fails. - The `cn-*.json` names have no path or digest, unlike the in-app family whose module carries a SHA-256 per file, and two of the three timing regimes cited for the Zhuyin omission were run out of tree. Both now say so rather than reading as things a reader could go and check. --- ...oard-handlers-ime-composing-chord.test.tsx | 6 +++-- ...ndlers.issue-12871-chinese-chord-traces.ts | 27 +++++++++++++------ ...inal-macos-ime-cursor-chord-native.spec.ts | 11 +++++--- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx index 2630ca38d82f..88a0d8d2c070 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx @@ -186,8 +186,10 @@ describe('a cursor chord pressed during a composition', () => { }) // The Japanese shape: still marked composing when the chord is resolved. The preedit spans - // several characters here on purpose — a relocated multi-character preedit also overwrote the - // glyph already at the destination cell, so the whole run has to commit in place first. + // several characters because that is the reported symptom — a relocated multi-character preedit + // also overwrote the glyph already at the destination cell. Only the order is asserted, though: + // shorten 日本語 to one character and this still passes, because where the text lands on the grid + // is not something this harness can see. it('holds the chord while the keydown is still marked composing', () => { const harness = createHarness() const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts index 6b6f9c666543..aabe0bdba53f 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.issue-12871-chinese-chord-traces.ts @@ -1,7 +1,10 @@ // Recorded IME chord traces for #12871 from the two Chinese input sources, taken inside a dev e2e // Orca build (app commit 84517d74b268bca888a44b285627ec82ffde1361, macOS 26.5.2/25F84, darwin // arm64) at the xterm helper textarea, with PTY-side ground-truth bytes read by a child on the -// pty. Replayed by keyboard-handlers.issue-12871-recorded-chord-traces.test.ts through the same +// pty. The per-case `cn-*.json` names below are the recording files, which live outside this repo +// on stablyai/orca#12732 — unlike the in-app family, whose sibling module carries a SHA-256 per +// file, there is no digest here to check a copy against. +// Replayed by keyboard-handlers.issue-12871-recorded-chord-traces.test.ts through the same // rig as the other recordings. Unlike the Kotoeri and Korean families, these rows cannot be // re-recorded from this repo: tests/e2e/terminal-macos-chord-input-pipeline-probe.spec.ts is the // recorder and was never extended past Japanese, Korean and ABC. The Chinese cases in @@ -15,19 +18,26 @@ // // What the three cases establish, measured rather than assumed: Chinese behaves like Kotoeri and // not like Korean. Both sources swallow the chord and keep composing, and the byte drains at the -// commit. The two modifiers still reach the handler by different routes, and each case pins the -// route it recorded: +// commit. The two modifiers still reach the handler by different routes: // // - `Cmd+←` delivers no arrow keyup, at any listener position or at Chromium's own input // dispatch. The `Cmd` release is the gesture's only end and arrives still marked composing. // - `Option+←` delivers the arrow's own keyup and resolves through it. // +// Read these as recordings first and tests second. The Zhuyin `Cmd+←` rows came out byte-identical +// to the Kotoeri ones in keyboard-handlers.issue-12871-command-release-traces.ts — that identity is +// the measurement, and it is why a third input source was worth recording at all. But it also means +// that case cannot fail on its own: replayed here it is the Kotoeri case under another name, and +// only the e2e cell below drives the real Zhuyin source. +// // A fourth cell, Zhuyin `Option+←`, was recorded and then DELIBERATELY NOT INCLUDED. Under every // synthesis this rig can produce it commits the composition on the chord, and a human at the same // keyboard cannot reproduce that — the preedit block stays up for them, as it does for Pinyin and // Kotoeri. Three timing regimes were tried (a flat 80ms with the modifier as a synthetic key // event, then the hardware medians 190/110/160ms and 700/400/500ms with the modifier posted as a -// real flagsChanged) and all three committed on the press. Rather than freeze a recording no hand +// real flagsChanged) and all three committed on the press. Only the first has a harness in this +// tree, tests/e2e/post-modifier-chord.swift; the other two were run out of tree, so that account +// is history rather than something you can re-run here. Rather than freeze a recording no hand // can reproduce, the cell is left out. Its byte order is still asserted in // terminal-macos-ime-cursor-chord-native.spec.ts, where it holds under both behaviours — but that // spec is hand-run behind ORCA_E2E_NATIVE_MACOS_KOREAN and no workflow sets it, so be plain about @@ -64,10 +74,11 @@ export const CHINESE_TRACE_CASES: RecordedChordCase[] = [ // letters rather than the highlighted candidate, so the committed text is ASCII while the // composition is unmistakably live (the view reads `ni hao`, segmented by the IME). // - // The composition update between the press and the Command release is the point of keeping - // this case alongside the Zhuyin one: the carry has to survive the IME editing its own - // preedit mid-gesture. A recovery that disarmed on composition activity would drop the byte - // here and still pass every other case in this file. + // The composition update between the press and the Command release is why this case earns its + // place next to the Zhuyin one, whose rows carry no composition activity at all: the carry has + // to survive the IME editing its own preedit mid-gesture. The Option case below has the same + // shape, so a recovery that disarmed on composition activity would take both of them down — + // what this one adds is that it happens on the Command route too. name: 'Simplified Pinyin, Cmd+ArrowLeft over a live ni hao preedit updated mid-gesture', expectCalls: ['\x01'], expectEmitted: ['\x01'], 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 334ffb7f3d52..627ee391931b 100644 --- a/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts +++ b/tests/e2e/terminal-macos-ime-cursor-chord-native.spec.ts @@ -50,8 +50,10 @@ import { * 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. * - Chinese (Zhuyin and Pinyin, measured 2026-08-09) behaves like Kotoeri and not like Korean: - * both swallow the chord, keep the preedit, and drain the byte at the commit. A Cmd chord - * there has no arrow keyup at all, so those two cells rest entirely on the Command release. + * both swallow the chord and drain the byte at the commit. A Cmd chord there has no arrow + * keyup at all, so those two cells rest entirely on the Command release. The preedit survives + * the press in three of the four cells; Zhuyin's Option cell is the exception under this rig + * and is described where it is defined. * - ABC control: no IME anywhere, movement bytes flow alone. * * Run-validity guards follow terminal-macos-korean-chord-commit-native.spec.ts: the selected @@ -353,8 +355,9 @@ test.describe('Native macOS IME cursor chords during composition @headful', () = composesThroughChord: true, // This chord moves the caret between the preedit's segments, and the segmented preedit // then costs one Return more than the Cmd cell: the first merges the segments, the next - // ends the composition. Spend that one here so flushLineToReader's own two presses mean - // the same thing in this cell as everywhere else in this file. + // ends the composition. Spend that one here so the Return inside flushLineToReader still + // means "flush the line" in this cell, as it does everywhere else in this file, rather + // than being eaten by the merge. commitReturns: 1, byte: '\x1bb', commit: /^nihao$/