mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
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 <neil@stably.ai> Co-authored-by: kunsanglee <85242378+kunsanglee@users.noreply.github.com>
This commit is contained in:
co-authored by
Neil
kunsanglee
parent
0a92d71874
commit
625d284e4d
+42
@@ -229,6 +229,48 @@ describe('a cursor chord pressed during a composition', () => {
|
||||
harness.dispose()
|
||||
})
|
||||
|
||||
function pressOptionArrowLeft(
|
||||
harness: ReturnType<typeof createHarness>,
|
||||
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))
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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<KeyboardEvent, 'code' | 'timeStamp'>
|
||||
export type TerminalImeKeyIdentity = Pick<KeyboardEvent, 'code' | 'timeStamp'>
|
||||
|
||||
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<string, Map<number, DeferredNewlineState>>()
|
||||
|
||||
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)
|
||||
|
||||
+1
-1
@@ -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() },
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user