From 625d284e4dba9eeeb898540c9f3a940fbbc18136 Mon Sep 17 00:00:00 2001 From: "Hyeonho Lee (Ethan)" Date: Mon, 26 Oct 2026 23:39:48 +0900 Subject: [PATCH] fix(terminal): send a composing cursor chord once, not twice (#17624) On macOS with a Korean IME, pressing Option+Left while a syllable was in the preedit moved the shell cursor two words. Chromium forwards a composing keydown for the chord, commits the composition, then replays the same native key event (same timeStamp). Orca deferred the first copy until the composition settled and then sent the replay too. The deferred chord sender now remembers the deferred keydown's code and timeStamp and absorbs exactly one matching replay. The absorb check runs before the composing-defer branch, so a replay arriving before xterm ends the composition session is covered as well. Any other checked keydown, blur, or teardown forfeits the slot. Fixes #17616 Co-authored-by: Neil Co-authored-by: kunsanglee <85242378+kunsanglee@users.noreply.github.com> --- ...oard-handlers-ime-composing-chord.test.tsx | 42 +++++++++++++++++++ .../terminal-ime-deferred-chord.test.ts | 10 ++--- .../terminal-ime-deferred-chord.ts | 24 +++++++++-- .../terminal-ime-deferred-newline.ts | 14 +++---- ...inal-keyboard-event-handlers-focus.test.ts | 2 +- .../terminal-keyboard-event-handlers.ts | 6 ++- 6 files changed, 81 insertions(+), 17 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 9627ecd31b8..322123c4dba 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 @@ -229,6 +229,48 @@ describe('a cursor chord pressed during a composition', () => { harness.dispose() }) + function pressOptionArrowLeft( + harness: ReturnType, + press: { isComposing: boolean; keyCode: number; timeStamp: number } + ): void { + const event = keyboardEvent('keydown', { + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: press.keyCode, + altKey: true, + isComposing: press.isComposing + }) + Object.defineProperty(event, 'timeStamp', { value: press.timeStamp }) + harness.terminalInput.dispatchEvent(event) + } + + // Chromium (crbug.com/48161): the IME's composing keydown, the commit, then the same NSEvent replayed unmarked. + it.each([ + ['before xterm ends the session', false], + ['after xterm ends the session', true] + ])('sends a chord the IME replays %s once (#17616)', (_label, sessionEndedFirst) => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.startComposition() + pressOptionArrowLeft(harness, { isComposing: true, keyCode: 229, timeStamp: 23884 }) + harness.terminalElement.dispatchEvent(new Event('compositionend')) + if (sessionEndedFirst) { + harness.endComposition('사') + } + pressOptionArrowLeft(harness, { isComposing: false, keyCode: 37, timeStamp: 23884 }) + if (!sessionEndedFirst) { + harness.endComposition('사') + } + vi.runAllTimers() + expect(harness.wire).toEqual(['사', '\x1bb']) + + pressOptionArrowLeft(harness, { isComposing: false, keyCode: 37, timeStamp: 23999 }) + expect(harness.wire, 'a genuine next press is not a replay').toEqual(['사', '\x1bb', '\x1bb']) + hook.unmount() + harness.dispose() + }) + it('sends immediately when no composition is in flight', () => { const harness = createHarness() const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.test.ts index 51f94019fcd..bcba11c8a52 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.test.ts @@ -20,7 +20,7 @@ describe('createTerminalImeDeferredChordSender', () => { const send = vi.fn() const sender = createTerminalImeDeferredChordSender() - sender.defer(el, send) + sender.defer({ code: 'ArrowLeft', timeStamp: 1 }, el, send) expect(send).not.toHaveBeenCalled() el.dispatchEvent(new Event('compositionend')) @@ -35,8 +35,8 @@ describe('createTerminalImeDeferredChordSender', () => { const second = vi.fn() const sender = createTerminalImeDeferredChordSender() - sender.defer(el, first) - sender.defer(el, second) + sender.defer({ code: 'ArrowLeft', timeStamp: 1 }, el, first) + sender.defer({ code: 'ArrowLeft', timeStamp: 1 }, el, second) sender.cancelPending() expect(removeEventListener).toHaveBeenCalledTimes(4) @@ -53,7 +53,7 @@ describe('createTerminalImeDeferredChordSender', () => { const send = vi.fn() const sender = createTerminalImeDeferredChordSender() - sender.defer(el, send) + sender.defer({ code: 'ArrowLeft', timeStamp: 1 }, el, send) vi.advanceTimersByTime(TERMINAL_IME_DEFERRED_CHORD_ABANDON_MS - 1) expect(send).not.toHaveBeenCalled() @@ -68,7 +68,7 @@ describe('createTerminalImeDeferredChordSender', () => { const send = vi.fn() const sender = createTerminalImeDeferredChordSender() - sender.defer(el, send) + sender.defer({ code: 'ArrowLeft', timeStamp: 1 }, el, send) el.dispatchEvent(new Event('compositionend')) vi.advanceTimersByTime(0) sender.cancelPending() diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.ts index 8dcb0893f91..267d81f26f0 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.ts @@ -1,19 +1,31 @@ -import { sendTerminalInputAfterComposition } from './terminal-ime-deferred-newline' +import { + sendTerminalInputAfterComposition, + type TerminalImeKeyIdentity +} from './terminal-ime-deferred-newline' /** Allow long conversion windows, then discard instead of firing mid-preedit and corrupting text (#12871). */ export const TERMINAL_IME_DEFERRED_CHORD_ABANDON_MS = 10_000 export type TerminalImeDeferredChordSender = { - defer: (terminalElement: HTMLElement | null | undefined, send: () => void) => void + defer: ( + chord: TerminalImeKeyIdentity, + terminalElement: HTMLElement | null | undefined, + send: () => void + ) => void + /** True only for the replay of the last deferred chord; any other checked keydown forfeits it. */ + absorbReplay: (chord: TerminalImeKeyIdentity) => boolean cancelPending: () => void } /** Own deferred chords so blur/teardown can stop waits even without compositionend and prevent sends into a rebound terminal. */ export function createTerminalImeDeferredChordSender(): TerminalImeDeferredChordSender { const pendingStops = new Set<() => void>() + // Chromium replays an IME-committing chord as the very next keydown, copied from the same NSEvent (crbug.com/48161). + let replayableChord: TerminalImeKeyIdentity | null = null return { - defer: (terminalElement, send) => { + defer: (chord, terminalElement, send) => { + replayableChord = { code: chord.code, timeStamp: chord.timeStamp } let abandonTimer: number | undefined let stopComposingWait: (() => void) | null = null const stopWaiting = (): void => { @@ -32,7 +44,13 @@ export function createTerminalImeDeferredChordSender(): TerminalImeDeferredChord { fallbackMs: null } ) }, + absorbReplay: (chord) => { + const owed = replayableChord + replayableChord = null + return owed?.code === chord.code && owed.timeStamp === chord.timeStamp + }, cancelPending: () => { + replayableChord = null for (const stopWaiting of pendingStops) { stopWaiting() } diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts index 3e996275e9e..6b6ce0d0712 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts @@ -64,23 +64,23 @@ export function sendTerminalInputAfterComposition( export type TerminalImeDeferredNewlineSender = { defer: ( - enter: TerminalImeEnterIdentity, + enter: TerminalImeKeyIdentity, terminalElement: HTMLElement | null | undefined, send: () => void ) => void - absorbRedispatchedEnter: (enter: TerminalImeEnterIdentity) => boolean + absorbRedispatchedEnter: (enter: TerminalImeKeyIdentity) => boolean releaseRedispatchedEnter: ( - enter: TerminalImeEnterIdentity, - originatingEnter?: TerminalImeEnterIdentity + enter: TerminalImeKeyIdentity, + originatingEnter?: TerminalImeKeyIdentity ) => void clearRedispatchedEnters: () => void } -export type TerminalImeEnterIdentity = Pick +export type TerminalImeKeyIdentity = Pick export type TerminalImeModifiedEnterKind = 'shift' | 'ctrl' -export type TerminalImeModifiedEnterChord = TerminalImeEnterIdentity & { +export type TerminalImeModifiedEnterChord = TerminalImeKeyIdentity & { kind: TerminalImeModifiedEnterKind terminalModifierKeyDownObserved?: boolean } @@ -168,7 +168,7 @@ type DeferredNewlineState = { export function createTerminalImeDeferredNewlineSender(): TerminalImeDeferredNewlineSender { const statesByEnterCode = new Map>() - const cleanUpIfSettled = (enter: TerminalImeEnterIdentity, state: DeferredNewlineState): void => { + const cleanUpIfSettled = (enter: TerminalImeKeyIdentity, state: DeferredNewlineState): void => { if (state.inFlightSends <= 0 && state.absorbCredits <= 0) { const statesByTimeStamp = statesByEnterCode.get(enter.code) statesByTimeStamp?.delete(enter.timeStamp) diff --git a/src/renderer/src/components/terminal-pane/terminal-keyboard-event-handlers-focus.test.ts b/src/renderer/src/components/terminal-pane/terminal-keyboard-event-handlers-focus.test.ts index dde159c33c6..2195c81c6d4 100644 --- a/src/renderer/src/components/terminal-pane/terminal-keyboard-event-handlers-focus.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-keyboard-event-handlers-focus.test.ts @@ -42,7 +42,7 @@ describe('terminal keyboard pane ownership', () => { absorbRedispatchedEnter: () => false, defer: vi.fn() }, - deferredChordSender: { defer: vi.fn() }, + deferredChordSender: { defer: vi.fn(), absorbReplay: () => false }, getModifiedEnterChord: () => null, reconcileHeldImeEnterModifiers: vi.fn(), optionKittyReleases: { arm: vi.fn(), armNativeDeadKey: vi.fn() }, diff --git a/src/renderer/src/components/terminal-pane/terminal-keyboard-event-handlers.ts b/src/renderer/src/components/terminal-pane/terminal-keyboard-event-handlers.ts index 724a177a67c..7c61947b4fc 100644 --- a/src/renderer/src/components/terminal-pane/terminal-keyboard-event-handlers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-keyboard-event-handlers.ts @@ -275,9 +275,13 @@ export function createTerminalKeyboardEventHandlers(context: EventContext) { deferredNewlineSender.defer(e, pane.terminal.element, sendResolvedInput) return } + // The held copy already sends a chord the IME replays on commit, even while xterm is still finalizing (#17616). + if (deferredChordSender.absorbReplay(e)) { + return + } // Defer cursor chords until composed text commits or they overtake it (#12871); unlike Enter, timeout must drop rather than fire mid-preedit. Sender owns cancellation on blur/teardown. if (e.isComposing || hasPendingImeComposition) { - deferredChordSender.defer(pane.terminal.element, sendResolvedInput) + deferredChordSender.defer(e, pane.terminal.element, sendResolvedInput) return } sendResolvedInput()