From e4f27d136f1e0fafef482c7b1331580a4ba1ec4f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:16:17 -0700 Subject: [PATCH] fix(terminal): gate IME-owned-key suppression to CJK input sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Chinese IME fix (#6396) added a third clause to shouldSuppressTerminalImeKeyboardEvent that swallows IME-owned editing keys (Backspace, Enter, arrows, etc.) whenever a composition is active, for ANY IME. This is correct for CJK candidate-window editing, but it breaks Vietnamese Telex/VNI, which compose inline and use Backspace mid-composition to rebuild a base char into its accented form (aa -> â, chaof -> chào). Those Backspaces were swallowed before reaching xterm/PTY, dropping or garbling diacritics. Gate clause 3 to CJK input sources via the existing macCjkInputSourceTracker (null on non-Mac, so Windows/Linux fall through to the universal isComposing / keyCode===229 paths unchanged). Clauses 1 and 2 are Chromium per-event flags valid for all IMEs and are left untouched, so Chinese Pinyin candidate selection is preserved. Fixes #6494 Co-authored-by: Orca --- .../use-terminal-pane-lifecycle.ts | 3 +- .../xterm-bypass-policy-non-mac.test.ts | 35 ++++++++++--- .../terminal-pane/xterm-bypass-policy.test.ts | 50 +++++++++++++++++-- .../terminal-pane/xterm-bypass-policy.ts | 20 ++++++-- 4 files changed, 90 insertions(+), 18 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index fe67c1ad83a..b0a41342384 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -715,7 +715,8 @@ export function useTerminalPaneLifecycle({ pane.terminal.attachCustomKeyEventHandler((e) => { if ( shouldSuppressTerminalImeKeyboardEvent(e, { - compositionActive: imeCompositionTracker.isActive() + compositionActive: imeCompositionTracker.isActive(), + isCjkCompositionActive: macCjkInputSourceTracker?.isActive() === true }) ) { return false diff --git a/src/renderer/src/components/terminal-pane/xterm-bypass-policy-non-mac.test.ts b/src/renderer/src/components/terminal-pane/xterm-bypass-policy-non-mac.test.ts index dbd19be8e54..f47b83f8c21 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy-non-mac.test.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy-non-mac.test.ts @@ -181,23 +181,44 @@ describe('shouldSuppressTerminalImeKeyboardEvent — Windows/Linux', () => { ).toBe(false) }) - it('suppresses IME-owned editing keys while composition is active', () => { + it('does NOT suppress IME-owned editing keys on non-Mac (no CJK input-source gate)', () => { + // Why: the CJK input-source tracker is Mac-only and is null on Windows/Linux, + // so isCjkCompositionActive is always false here. The candidate-window + // swallow therefore never fires; inline composition keys reach xterm. The + // Windows IME Process-key path (keyCode 229) still covers active preedit. + for (const key of ['Backspace', 'Enter', 'ArrowDown']) { + expect( + shouldSuppressTerminalImeKeyboardEvent(event({ key, code: key }), { + compositionActive: true, + isCjkCompositionActive: false + }) + ).toBe(false) + } + }) + + it('still suppresses universal Chromium IME signals regardless of CJK gate', () => { expect( - shouldSuppressTerminalImeKeyboardEvent(event({ key: 'Backspace', code: 'Backspace' }), { - compositionActive: true + shouldSuppressTerminalImeKeyboardEvent(event({ key: 'Backspace', isComposing: true }), { + compositionActive: false, + isCjkCompositionActive: false }) ).toBe(true) expect( - shouldSuppressTerminalImeKeyboardEvent(event({ key: 'ArrowDown', code: 'ArrowDown' }), { - compositionActive: true - }) + shouldSuppressTerminalImeKeyboardEvent( + event({ key: 'Process', code: 'KeyN', keyCode: 229 }), + { + compositionActive: true, + isCjkCompositionActive: false + } + ) ).toBe(true) }) it('does not suppress ordinary text keys solely because composition is active', () => { expect( shouldSuppressTerminalImeKeyboardEvent(event({ key: 'a', code: 'KeyA' }), { - compositionActive: true + compositionActive: true, + isCjkCompositionActive: false }) ).toBe(false) }) diff --git a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts index c5a1e2310d9..7c5f0b29651 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts @@ -184,23 +184,63 @@ describe('shouldSuppressTerminalImeKeyboardEvent — macOS', () => { ).toBe(false) }) - it('suppresses IME-owned editing keys while composition is active', () => { + it('suppresses IME-owned editing keys while a CJK composition is active', () => { + // Why: CJK candidate-window editing — Backspace/arrows belong to the + // preedit session and must not delete committed text in a TUI (#6396). expect( shouldSuppressTerminalImeKeyboardEvent(event({ key: 'Backspace', code: 'Backspace' }), { - compositionActive: true + compositionActive: true, + isCjkCompositionActive: true }) ).toBe(true) expect( shouldSuppressTerminalImeKeyboardEvent(event({ key: 'ArrowDown', code: 'ArrowDown' }), { - compositionActive: true + compositionActive: true, + isCjkCompositionActive: true }) ).toBe(true) }) - it('does not suppress ordinary text keys solely because composition is active', () => { + it('does NOT suppress IME-owned editing keys for inline non-CJK IMEs (Vietnamese Telex/VNI)', () => { + // Why: Telex/VNI compose inline and use Backspace/Enter mid-composition to + // rebuild a base char into its accented form (aa -> â). Swallowing those + // drops/garbles diacritics (#6494). Only CJK candidate windows need the + // swallow, so a non-CJK active composition must fall through to xterm. + for (const key of ['Backspace', 'Enter', 'ArrowDown', 'ArrowLeft']) { + expect( + shouldSuppressTerminalImeKeyboardEvent(event({ key, code: key }), { + compositionActive: true, + isCjkCompositionActive: false + }) + ).toBe(false) + } + }) + + it('still suppresses universal Chromium IME signals regardless of CJK gate', () => { + // Why: clauses 1-2 (isComposing, keyCode 229) are per-event Chromium flags + // valid for every IME and must not depend on the CJK input-source gate. + expect( + shouldSuppressTerminalImeKeyboardEvent(event({ key: 'Backspace', isComposing: true }), { + compositionActive: false, + isCjkCompositionActive: false + }) + ).toBe(true) + expect( + shouldSuppressTerminalImeKeyboardEvent( + event({ key: 'Process', code: 'KeyN', keyCode: 229 }), + { + compositionActive: true, + isCjkCompositionActive: false + } + ) + ).toBe(true) + }) + + it('does not suppress ordinary text keys solely because a CJK composition is active', () => { expect( shouldSuppressTerminalImeKeyboardEvent(event({ key: 'a', code: 'KeyA' }), { - compositionActive: true + compositionActive: true, + isCjkCompositionActive: true }) ).toBe(false) }) diff --git a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts index 79c0c3caa2b..110b3a15750 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts @@ -35,6 +35,10 @@ export type XtermBypassOptions = { export type XtermImeKeyboardOptions = { compositionActive: boolean + /** True only when the active macOS input source is CJK. Gates the + * IME-owned-key swallow to candidate-window IMEs and away from inline + * IMEs (Vietnamese Telex/VNI), where mid-composition Backspace is real. */ + isCjkCompositionActive: boolean } export const TERMINAL_INTERRUPT_INPUT = '\x03' @@ -69,18 +73,24 @@ function isXtermHandledKeyEvent(type: string): boolean { export function shouldSuppressTerminalImeKeyboardEvent( event: XtermBypassEvent, - options: XtermImeKeyboardOptions = { compositionActive: false } + options: XtermImeKeyboardOptions = { compositionActive: false, isCjkCompositionActive: false } ): boolean { if (!isXtermHandledKeyEvent(event.type)) { return false } - // Why: IMEs own Process-key / composing keystrokes. Letting xterm translate - // Backspace/Enter/etc. into PTY bytes makes TUIs delete committed CJK text - // while the user is only editing the preedit candidate. + // Why: clauses 1-2 are Chromium per-event flags safe for all IMEs. Clause 3 + // is a CJK candidate-window workaround only: a candidate keystroke can carry + // isComposing=false yet still belong to the preedit session, so we swallow + // editing keys to stop TUIs deleting committed CJK text. It must NOT fire for + // inline IMEs (Vietnamese Telex/VNI) where mid-composition Backspace + // legitimately rebuilds a base char into its accented form (aa -> â), hence + // the CJK input-source gate. return ( event.isComposing === true || event.keyCode === 229 || - (options.compositionActive && TERMINAL_IME_OWNED_KEYS.has(event.key)) + (options.compositionActive && + options.isCjkCompositionActive && + TERMINAL_IME_OWNED_KEYS.has(event.key)) ) }