mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
fix(terminal): gate IME-owned-key suppression to CJK input sources
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 <help@stably.ai>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
@@ -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))
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user