mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
fix(ime): stop terminal shortcuts firing on keystrokes the IME owns
The window-capture keydown handler resolved and dispatched terminal chords without checking for a live composition. It runs ahead of the pane handler and stops propagation on a match, so the chord was the only thing the keystroke did. Ctrl+Backspace mid-preedit reached the PTY as \x17 and erased a word of the real command line behind the candidate window, leaving the composition untouched. Alt+Backspace, Alt and Ctrl arrows, and the macOS Cmd line-edit chords have the same shape. Separately, the non-Latin fallback re-derives bindings from event.code, so a keystroke reported as Process still matched Ctrl+W and closed the pane out from under a composition. The only composition check was inside the sendInput branch and covered Enter alone, which is the one key that was already safe. Enter and the input-source switch stay exempt: the send path defers Enter until the commit lands, and the switch only hands the key back to the OS.
This commit is contained in:
@@ -31,6 +31,7 @@ function keyboardEvent(
|
||||
function createHarness(): {
|
||||
deps: KeyboardHandlersDeps
|
||||
editable: HTMLInputElement
|
||||
sendInput: ReturnType<typeof vi.fn>
|
||||
startComposition: () => void
|
||||
terminalInput: HTMLTextAreaElement
|
||||
dispose: () => void
|
||||
@@ -97,6 +98,7 @@ function createHarness(): {
|
||||
return {
|
||||
deps,
|
||||
editable,
|
||||
sendInput,
|
||||
terminalInput,
|
||||
startComposition: () => {
|
||||
terminalElement.dispatchEvent(
|
||||
@@ -275,3 +277,79 @@ describe('Windows IME keyboard ownership', () => {
|
||||
harness.dispose()
|
||||
})
|
||||
})
|
||||
|
||||
describe('terminal shortcuts yield to a composition', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue('X11; Linux x86_64')
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.clearAllTimers()
|
||||
vi.useRealTimers()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
// The chord resolves to a real control sequence, so the only thing standing between a
|
||||
// preedit keystroke and the PTY is a composition check above the dispatch.
|
||||
it.each([
|
||||
{ name: 'Ctrl+Backspace', init: { key: 'Backspace', code: 'Backspace', ctrlKey: true } },
|
||||
{ name: 'Alt+Backspace', init: { key: 'Backspace', code: 'Backspace', altKey: true } },
|
||||
{ name: 'Alt+ArrowLeft', init: { key: 'ArrowLeft', code: 'ArrowLeft', altKey: true } }
|
||||
])('does not send $name to the PTY while composing', ({ init }) => {
|
||||
const harness = createHarness()
|
||||
const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps))
|
||||
harness.startComposition()
|
||||
|
||||
harness.terminalInput.dispatchEvent(
|
||||
keyboardEvent('keydown', { ...init, keyCode: 229, timeStamp: 10, isComposing: true })
|
||||
)
|
||||
|
||||
expect(harness.sendInput).not.toHaveBeenCalled()
|
||||
hook.unmount()
|
||||
harness.dispose()
|
||||
})
|
||||
|
||||
it('still sends Ctrl+Backspace when nothing is composing', () => {
|
||||
const harness = createHarness()
|
||||
const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps))
|
||||
|
||||
harness.terminalInput.dispatchEvent(
|
||||
keyboardEvent('keydown', {
|
||||
key: 'Backspace',
|
||||
code: 'Backspace',
|
||||
keyCode: 8,
|
||||
timeStamp: 10,
|
||||
ctrlKey: true
|
||||
})
|
||||
)
|
||||
|
||||
expect(harness.sendInput).toHaveBeenCalled()
|
||||
hook.unmount()
|
||||
harness.dispose()
|
||||
})
|
||||
|
||||
// 'Process' means the IME already consumed this keystroke, but the non-Latin fallback
|
||||
// re-derives the binding from event.code, so the chord matches by physical key anyway.
|
||||
it('does not close the pane on a Ctrl+W the IME consumed', () => {
|
||||
const harness = createHarness()
|
||||
const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps))
|
||||
harness.startComposition()
|
||||
|
||||
harness.terminalInput.dispatchEvent(
|
||||
keyboardEvent('keydown', {
|
||||
key: 'Process',
|
||||
code: 'KeyW',
|
||||
keyCode: 229,
|
||||
timeStamp: 10,
|
||||
isComposing: true,
|
||||
ctrlKey: true
|
||||
})
|
||||
)
|
||||
|
||||
expect(harness.deps.onRequestClosePane).not.toHaveBeenCalled()
|
||||
hook.unmount()
|
||||
harness.dispose()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -9,6 +9,7 @@ import { safeFind } from '../terminal-search-safe-find'
|
||||
import { resolveTerminalShortcutAction } from './terminal-shortcut-policy'
|
||||
import type { MacOptionAsAlt } from './terminal-shortcut-policy'
|
||||
import { createTerminalNativeOnlyShortcutTracker } from './terminal-native-only-shortcut'
|
||||
import { terminalShortcutIsOwnedByIme } from './terminal-shortcut-composition-guard'
|
||||
import {
|
||||
createTerminalImeDeferredNewlineSender,
|
||||
createTerminalImeModifiedEnterChordOwner,
|
||||
@@ -456,6 +457,10 @@ export function useTerminalKeyboardShortcuts({
|
||||
return
|
||||
}
|
||||
|
||||
if (terminalShortcutIsOwnedByIme(e, resolveShortcutEvent)) {
|
||||
return
|
||||
}
|
||||
|
||||
if (matchFileSearchShortcut(e, shortcutPlatform, keybindings, terminalShortcutPolicy)) {
|
||||
const pane = manager.getActivePane() ?? manager.getPanes()[0]
|
||||
const selectedText = normalizeSelectedTextForFileSearch(pane?.terminal.getSelection())
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { terminalShortcutIsOwnedByIme } from './terminal-shortcut-composition-guard'
|
||||
|
||||
function keydown(init: KeyboardEventInit & { keyCode?: number }): KeyboardEvent {
|
||||
const event = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, ...init })
|
||||
if (init.keyCode !== undefined) {
|
||||
Object.defineProperty(event, 'keyCode', { value: init.keyCode })
|
||||
}
|
||||
return event
|
||||
}
|
||||
|
||||
const sendInput = (): { type: string } => ({ type: 'sendInput' })
|
||||
const switchInputSource = (): { type: string } => ({ type: 'switchInputSource' })
|
||||
const noAction = (): null => null
|
||||
|
||||
describe('terminalShortcutIsOwnedByIme', () => {
|
||||
it.each([
|
||||
['isComposing', { key: 'Backspace', code: 'Backspace', ctrlKey: true, isComposing: true }],
|
||||
['keyCode 229', { key: 'Backspace', code: 'Backspace', ctrlKey: true, keyCode: 229 }],
|
||||
['key Process', { key: 'Process', code: 'KeyW', ctrlKey: true }]
|
||||
])('claims a chord marked by %s', (_marker, init) => {
|
||||
expect(terminalShortcutIsOwnedByIme(keydown(init), sendInput)).toBe(true)
|
||||
})
|
||||
|
||||
it('leaves an unmarked chord alone', () => {
|
||||
const event = keydown({ key: 'Backspace', code: 'Backspace', ctrlKey: true, keyCode: 8 })
|
||||
expect(terminalShortcutIsOwnedByIme(event, sendInput)).toBe(false)
|
||||
})
|
||||
|
||||
// Both exemptions matter in the same direction: claiming them would break a path that
|
||||
// already handles composition, rather than merely declining to fix one.
|
||||
it('releases Enter so the send path can defer it to the commit', () => {
|
||||
const event = keydown({ key: 'Process', code: 'Enter', keyCode: 229, isComposing: true })
|
||||
expect(terminalShortcutIsOwnedByIme(event, sendInput)).toBe(false)
|
||||
})
|
||||
|
||||
it('releases the input-source switch so the OS still receives it', () => {
|
||||
const event = keydown({ key: 'Process', code: 'Space', ctrlKey: true, keyCode: 229 })
|
||||
expect(terminalShortcutIsOwnedByIme(event, switchInputSource)).toBe(false)
|
||||
})
|
||||
|
||||
it('claims a marked chord that resolves to nothing, so no later matcher sees it', () => {
|
||||
const event = keydown({ key: 'Process', code: 'KeyF', ctrlKey: true, keyCode: 229 })
|
||||
expect(terminalShortcutIsOwnedByIme(event, noAction)).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,35 @@
|
||||
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
|
||||
|
||||
type ResolvedShortcutAction = { readonly type: string } | null
|
||||
|
||||
/**
|
||||
* True when an IME owns this keydown, so no terminal shortcut may act on it.
|
||||
*
|
||||
* The window-capture handler runs before the pane's own key handler and stops
|
||||
* propagation on a match, so a chord resolved here is the only thing the keystroke
|
||||
* will ever do. Ctrl+Backspace mid-preedit reaches the PTY as `\x17` and erases a word
|
||||
* of the real command line behind the candidate window; the IME's buffer is untouched.
|
||||
* The non-Latin fallback makes it worse by re-deriving bindings from `event.code`, so
|
||||
* even a keystroke reported as `Process` still matches Ctrl+W and closes the pane.
|
||||
*
|
||||
* Two exemptions, both paths that already handle a live composition:
|
||||
*
|
||||
* - **Enter** — the send path defers it until the commit lands, so it must get through.
|
||||
* - **Input-source switch** — it only hands the key back to the OS. Dropping it here
|
||||
* would let xterm see the chord instead and write its control byte to the PTY, which
|
||||
* is the very failure this guard exists to prevent.
|
||||
*
|
||||
* Resolution is pure, so asking for the action before deciding costs nothing.
|
||||
*/
|
||||
export function terminalShortcutIsOwnedByIme(
|
||||
event: KeyboardEvent,
|
||||
resolveAction: (event: KeyboardEvent) => ResolvedShortcutAction
|
||||
): boolean {
|
||||
if (!isImeCompositionKeyDown(event)) {
|
||||
return false
|
||||
}
|
||||
if (event.code === 'Enter') {
|
||||
return false
|
||||
}
|
||||
return resolveAction(event)?.type !== 'switchInputSource'
|
||||
}
|
||||
Reference in New Issue
Block a user