From e5568d899b1649822e25415e3aeaec90e376ab6d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 1 Aug 2026 20:44:43 -0700 Subject: [PATCH] fix(ime): stop a drifted composition flag from swallowing Escape An Escape the IME owns always arrives marked, so an unmarked one means the document flag has drifted. compositionend is not guaranteed, and eleven call sites guard Escape above their key dispatch, so the drift left rename fields, search boxes and pickers with no way to cancel. Matches the rule xterm-bypass-policy.ts already applies to the terminal. Co-authored-by: Orca --- .../WorkspaceKanbanSearchField.test.tsx | 15 +++++++++++ .../ime-composition-keyboard-event.test.ts | 18 +++++++++++++ .../src/lib/ime-composition-keyboard-event.ts | 25 +++++++++++++------ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.test.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.test.tsx index ab5170c8749..da2591826bf 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.test.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.test.tsx @@ -195,6 +195,21 @@ describe('WorkspaceKanbanSearchField', () => { expect(onClose).not.toHaveBeenCalled() }) + it('still clears on an unmarked Escape after a composition that never ended', () => { + // Why: compositionend is not guaranteed, and this field is the board's only + // Escape handler, so trusting the document flag alone would strand the query. + renderField({ query: '検索', matchCount: 1, totalCount: 12 }) + + act(() => { + input().dispatchEvent(new CompositionEvent('compositionstart', { bubbles: true })) + input().dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }) + ) + }) + + expect(onClear).toHaveBeenCalledTimes(1) + }) + it('keeps focus in the field after the clear button unmounts itself', () => { renderField({ query: 'orca', matchCount: 3, totalCount: 12 }) act(() => { diff --git a/src/renderer/src/lib/ime-composition-keyboard-event.test.ts b/src/renderer/src/lib/ime-composition-keyboard-event.test.ts index e6618e85d01..edbe06b43c2 100644 --- a/src/renderer/src/lib/ime-composition-keyboard-event.test.ts +++ b/src/renderer/src/lib/ime-composition-keyboard-event.test.ts @@ -86,6 +86,24 @@ describe('document composition tracking', () => { expect(isImeCompositionKeyDown(keyEvent({ isComposing: false, keyCode: 13 }))).toBe(true) }) + it('still suppresses an Escape the IME actually marked', () => { + dispatchOnBody('compositionstart') + + expect( + isImeCompositionKeyDown(keyEvent({ isComposing: true, key: 'Escape', keyCode: 27 })) + ).toBe(true) + expect(isImeCompositionKeyDown(keyEvent({ key: 'Escape', keyCode: 229 }))).toBe(true) + }) + + it('lets an unmarked Escape through so a drifted flag cannot strand the cancel path', () => { + // No compositionend is guaranteed, so nothing else would clear this. Every + // rename field, search box and picker in the app cancels on Escape. + dispatchOnBody('compositionstart') + + expect(isImeCompositionKeyDown(keyEvent({ key: 'Escape', keyCode: 27 }))).toBe(false) + expect(isImeCompositionKeyDown(keyEvent({ key: 'Enter', keyCode: 13 }))).toBe(true) + }) + it('tracks composition anywhere in the document', () => { dispatchOnBody('compositionstart') expect(isDocumentImeCompositionActive()).toBe(true) diff --git a/src/renderer/src/lib/ime-composition-keyboard-event.ts b/src/renderer/src/lib/ime-composition-keyboard-event.ts index 393952250f6..78c4deaff16 100644 --- a/src/renderer/src/lib/ime-composition-keyboard-event.ts +++ b/src/renderer/src/lib/ime-composition-keyboard-event.ts @@ -84,21 +84,30 @@ export function _setDocumentImeCompositionActiveForTests(active: boolean): void * Call this above the key dispatch, not inside each key branch, so every action * (send, history navigation, completion, cancel) is suppressed together. * - * The 229 / 'Process' markers are unconditional here — there is deliberately no - * opt-out argument. The only surface that needs a marked key to pass through is - * the terminal, so it can reach xterm's CompositionHelper, and that decision - * lives in xterm-bypass-policy.ts instead. Every caller of this predicate is - * ordinary UI, so an opt-out would only ever be dead API. + * The 229 / 'Process' markers are unconditional here, and deliberately stricter + * than xterm-bypass-policy.ts, which lets a bare 229 keydown through on + * macOS/Linux. That is not a contradiction: the terminal needs the keystroke + * because xterm's CompositionHelper reads the committed glyph off the helper + * textarea. Callers of this predicate receive committed text through `input`, so + * a marked keydown carries nothing they could act on — its `key` is 'Process' or + * 'Unidentified', never the glyph. Suppressing it here costs no input. */ export function isImeCompositionKeyDown(event: AnyKeyboardEvent): boolean { const nativeEvent = toNativeKeyboardEvent(event) if (!nativeEvent) { return isDocumentImeCompositionActive() } - return ( - isDocumentImeCompositionActive() || + if ( nativeEvent.isComposing === true || nativeEvent.keyCode === 229 || nativeEvent.key === 'Process' - ) + ) { + return true + } + // Why: an Escape the IME owns is marked above, so an unmarked one means this + // derived flag has drifted. compositionend is not guaranteed, so swallowing it + // would strand every cancel path behind a flag nothing is going to clear. Same + // rule as TERMINAL_IME_OWNED_KEYS in xterm-bypass-policy.ts, which omits Escape + // for exactly this reason. + return nativeEvent.key !== 'Escape' && isDocumentImeCompositionActive() }