diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx index 2200f2d5c9ee..f13a57115b73 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx @@ -14,6 +14,7 @@ import { XTERM_COMPOSITION_SESSION_END_EVENT, XTERM_COMPOSITION_SESSION_START_EVENT } from './terminal-ime-composition-route' +import { TERMINAL_IME_DEFERRED_CHORD_ABANDON_MS } from './terminal-ime-deferred-chord' import { useTerminalKeyboardShortcuts } from './keyboard-handlers' type KeyboardHandlersDeps = Parameters[0] @@ -30,12 +31,34 @@ function keyboardEvent( return event } +/** Live registrations on the terminal element, so a deferral that never disposes is visible. */ +function trackCompositionListeners(element: HTMLElement): () => number { + const live = new Set() + const watched = new Set(['compositionend', XTERM_COMPOSITION_SESSION_END_EVENT]) + const { addEventListener, removeEventListener } = element + element.addEventListener = function (type, listener, options): void { + if (watched.has(type) && listener) { + live.add(listener) + } + addEventListener.call(this, type, listener, options) + } + element.removeEventListener = function (type, listener, options): void { + if (watched.has(type) && listener) { + live.delete(listener) + } + removeEventListener.call(this, type, listener, options) + } + return () => live.size +} + function createHarness(): { deps: KeyboardHandlersDeps terminalElement: HTMLDivElement terminalInput: HTMLTextAreaElement /** Every byte reaching the pty, in arrival order, whichever route it took. */ wire: string[] + /** Registrations added after the composition route's own, i.e. the deferrals still waiting. */ + deferralListenerCount: () => number startComposition: () => void endComposition: (data: string) => void dispose: () => void @@ -77,6 +100,8 @@ function createHarness(): { capturedTransport: transport, getCurrentTransport: () => transport }) + // Installed after the route so only the deferral's own registrations are counted. + const deferralListenerCount = trackCompositionListeners(terminalElement) const deps: KeyboardHandlersDeps = { tabId: 'tab-1', @@ -110,6 +135,7 @@ function createHarness(): { terminalElement, terminalInput, wire, + deferralListenerCount, startComposition: () => { terminalElement.dispatchEvent( new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1 } }) @@ -220,4 +246,57 @@ describe('a cursor chord pressed during a composition', () => { hook.unmount() harness.dispose() }) + + // STA-4476: an indefinite wait has no exit of its own. Without a disposer the listeners outlive + // the pane and a later composition on the same element flushes the stale chord. + it('drops the held chord and its listeners when the pane tears down', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.startComposition() + pressCmdArrowLeft(harness, true) + expect(harness.deferralListenerCount()).toBeGreaterThan(0) + + hook.unmount() + harness.endComposition('다') + vi.runAllTimers() + + expect(harness.wire).toEqual(['다']) + expect(harness.deferralListenerCount()).toBe(0) + harness.dispose() + }) + + it('drops the held chord and its listeners on blur', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.startComposition() + pressCmdArrowLeft(harness, true) + window.dispatchEvent(new Event('blur')) + harness.endComposition('다') + vi.runAllTimers() + + expect(harness.wire).toEqual(['다']) + expect(harness.deferralListenerCount()).toBe(0) + hook.unmount() + harness.dispose() + }) + + // A composition that never commits must not strand the wait forever. The chord is discarded, + // never sent late — a late send is #12871 again. + it('abandons a chord whose composition never ends', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.startComposition() + pressCmdArrowLeft(harness, true) + vi.advanceTimersByTime(TERMINAL_IME_DEFERRED_CHORD_ABANDON_MS + 1) + harness.endComposition('다') + vi.runAllTimers() + + expect(harness.wire).toEqual(['다']) + expect(harness.deferralListenerCount()).toBe(0) + hook.unmount() + harness.dispose() + }) }) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 0a5fe0ac5b93..3614af3b888f 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -15,9 +15,9 @@ import { getTerminalImeModifiedEnterKind, isTerminalImeConsumedKey, isTerminalImeEnterKeyUp, - isTerminalImeProcessEnter, - sendTerminalInputAfterComposition + isTerminalImeProcessEnter } from './terminal-ime-deferred-newline' +import { createTerminalImeDeferredChordSender } from './terminal-ime-deferred-chord' import { hasPendingTerminalImeComposition } from './terminal-ime-composition-route' import { requestCapturedTerminalReconfirmation, @@ -290,6 +290,7 @@ export function useTerminalKeyboardShortcuts({ const terminalImeEnterModifierKeydowns = new Set<'shift' | 'ctrl'>() const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker() const deferredNewlineSender = createTerminalImeDeferredNewlineSender() + const deferredChordSender = createTerminalImeDeferredChordSender() const modifiedEnterChordOwner = createTerminalImeModifiedEnterChordOwner() const reconcileHeldImeEnterModifiers = ( event: KeyboardEvent, @@ -651,11 +652,10 @@ export function useTerminalKeyboardShortcuts({ // runs after this keydown. Sending now puts a cursor chord ahead of the text it was typed // after — `가나다` then Cmd+Left leaves `다가나` (#12871). Enter is handled above, where a // fallback timer is right because a newline arriving late still arrives; a chord arriving - // mid-preedit is the corruption itself, so this one waits without a deadline. + // mid-preedit is the corruption itself, so this one waits on the composition rather than a + // deadline. The sender owns the wait so blur and teardown can drop it. if (e.isComposing || hasPendingImeComposition) { - sendTerminalInputAfterComposition(pane.terminal.element, sendResolvedInput, { - fallbackMs: null - }) + deferredChordSender.defer(pane.terminal.element, sendResolvedInput) return } sendResolvedInput() @@ -986,6 +986,7 @@ export function useTerminalKeyboardShortcuts({ terminalImeEnterModifierKeydowns.clear() modifiedEnterChordOwner.clear() deferredNewlineSender.clearRedispatchedEnters() + deferredChordSender.cancelPending() observedEnterKeydownTimeStamps.clear() } @@ -1000,6 +1001,7 @@ export function useTerminalKeyboardShortcuts({ optionKittyReleases.clear() modifiedEnterChordOwner.clear() deferredNewlineSender.clearRedispatchedEnters() + deferredChordSender.cancelPending() observedEnterKeydownTimeStamps.clear() window.removeEventListener('keydown', onModifierDown, { capture: true }) window.removeEventListener('keyup', onKeyUp, { capture: true }) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.test.ts new file mode 100644 index 000000000000..d54ab6de4556 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.test.ts @@ -0,0 +1,81 @@ +// @vitest-environment happy-dom +// STA-4476: the composing-chord deferral waits on the composition, not a deadline (#12871), so it +// is the sender that has to guarantee an exit — otherwise the wait and its listeners outlive the +// pane and a later composition flushes a stale chord. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createTerminalImeDeferredChordSender, + TERMINAL_IME_DEFERRED_CHORD_ABANDON_MS +} from './terminal-ime-deferred-chord' +import { XTERM_COMPOSITION_SESSION_END_EVENT } from './terminal-ime-composition-route' + +describe('createTerminalImeDeferredChordSender', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + afterEach(() => { + vi.useRealTimers() + }) + + it('sends once the composition commits', () => { + const el = document.createElement('div') + const send = vi.fn() + const sender = createTerminalImeDeferredChordSender() + + sender.defer(el, send) + expect(send).not.toHaveBeenCalled() + + el.dispatchEvent(new Event('compositionend')) + vi.advanceTimersByTime(0) + expect(send).toHaveBeenCalledTimes(1) + }) + + it('cancels every pending chord and detaches its listeners', () => { + const el = document.createElement('div') + const removeEventListener = vi.spyOn(el, 'removeEventListener') + const first = vi.fn() + const second = vi.fn() + const sender = createTerminalImeDeferredChordSender() + + sender.defer(el, first) + sender.defer(el, second) + sender.cancelPending() + + expect(removeEventListener).toHaveBeenCalledTimes(4) + + el.dispatchEvent(new Event('compositionend')) + el.dispatchEvent(new CustomEvent(XTERM_COMPOSITION_SESSION_END_EVENT)) + vi.runAllTimers() + expect(first).not.toHaveBeenCalled() + expect(second).not.toHaveBeenCalled() + }) + + it('abandons a chord whose composition never ends rather than sending it late', () => { + const el = document.createElement('div') + const send = vi.fn() + const sender = createTerminalImeDeferredChordSender() + + sender.defer(el, send) + vi.advanceTimersByTime(TERMINAL_IME_DEFERRED_CHORD_ABANDON_MS - 1) + expect(send).not.toHaveBeenCalled() + + vi.advanceTimersByTime(2) + el.dispatchEvent(new Event('compositionend')) + vi.runAllTimers() + expect(send).not.toHaveBeenCalled() + }) + + it('stops tracking a chord that already sent, so a later cancel is inert', () => { + const el = document.createElement('div') + const send = vi.fn() + const sender = createTerminalImeDeferredChordSender() + + sender.defer(el, send) + el.dispatchEvent(new Event('compositionend')) + vi.advanceTimersByTime(0) + sender.cancelPending() + vi.runAllTimers() + + expect(send).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.ts new file mode 100644 index 000000000000..8598a0ec1f8f --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-chord.ts @@ -0,0 +1,51 @@ +import { sendTerminalInputAfterComposition } from './terminal-ime-deferred-newline' + +/** + * Ceiling on the otherwise indefinite wait. Generous enough for a conversion candidate window, + * which can stay open for seconds; a chord that outlives it is discarded rather than sent late, + * because firing mid-preedit is the corruption the wait exists to prevent (#12871). + */ +export const TERMINAL_IME_DEFERRED_CHORD_ABANDON_MS = 10_000 + +export type TerminalImeDeferredChordSender = { + defer: (terminalElement: HTMLElement | null | undefined, send: () => void) => void + cancelPending: () => void +} + +/** + * Owns every chord held for a live composition so blur and pane teardown can drop them. An + * unowned deferral has no exit when compositionend never arrives: its listeners outlive the pane + * and a later composition flushes the stale chord against a rebound terminal. + */ +export function createTerminalImeDeferredChordSender(): TerminalImeDeferredChordSender { + const pendingStops = new Set<() => void>() + + return { + defer: (terminalElement, send) => { + let abandonTimer: number | undefined + let stopComposingWait: (() => void) | null = null + const stopWaiting = (): void => { + window.clearTimeout(abandonTimer) + pendingStops.delete(stopWaiting) + stopComposingWait?.() + } + abandonTimer = window.setTimeout(stopWaiting, TERMINAL_IME_DEFERRED_CHORD_ABANDON_MS) + pendingStops.add(stopWaiting) + stopComposingWait = sendTerminalInputAfterComposition( + terminalElement, + () => { + stopWaiting() + send() + }, + { fallbackMs: null } + ) + }, + cancelPending: () => { + // Each stop removes itself; deleting the visited entry is safe for a Set iterator. + for (const stopWaiting of pendingStops) { + stopWaiting() + } + pendingStops.clear() + } + } +} diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.test.ts index 5b8613c1f7fd..0ac8ce17377c 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.test.ts @@ -6,7 +6,8 @@ import { createTerminalImeModifiedEnterChordOwner, isTerminalImeEnterKeyUp, isTerminalImeProcessEnter, - sendTerminalInputAfterComposition + sendTerminalInputAfterComposition, + TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS } from './terminal-ime-deferred-newline' import { installTerminalImeCompositionRoute, @@ -53,6 +54,27 @@ describe('sendTerminalInputAfterComposition', () => { expect(send).toHaveBeenCalledTimes(1) }) + // STA-4476: `fallbackMs: null` has no exit of its own, so the disposer is the only thing that + // can detach the listeners when compositionend never arrives. + it('detaches its listeners and never sends once disposed', () => { + const el = document.createElement('div') + const send = vi.fn() + const removeEventListener = vi.spyOn(el, 'removeEventListener') + + const dispose = sendTerminalInputAfterComposition(el, send, { fallbackMs: null }) + dispose() + + expect(removeEventListener).toHaveBeenCalledWith('compositionend', expect.any(Function)) + expect(removeEventListener).toHaveBeenCalledWith( + XTERM_COMPOSITION_SESSION_END_EVENT, + expect.any(Function) + ) + + el.dispatchEvent(new Event('compositionend')) + vi.runAllTimers() + expect(send).not.toHaveBeenCalled() + }) + it('falls back to sending when no compositionend arrives', () => { const el = document.createElement('div') const send = vi.fn() @@ -63,6 +85,22 @@ describe('sendTerminalInputAfterComposition', () => { expect(send).toHaveBeenCalledTimes(1) }) + // STA-4476: why the Enter path needs no disposer of its own — its fallback always runs, so the + // listeners cannot outlive it even when the composition never ends. + it('detaches its listeners on the fallback, not only on compositionend', () => { + const el = document.createElement('div') + const removeEventListener = vi.spyOn(el, 'removeEventListener') + + sendTerminalInputAfterComposition(el, vi.fn()) + vi.advanceTimersByTime(TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS) + + expect(removeEventListener).toHaveBeenCalledWith('compositionend', expect.any(Function)) + expect(removeEventListener).toHaveBeenCalledWith( + XTERM_COMPOSITION_SESSION_END_EVENT, + expect.any(Function) + ) + }) + it('finishes from the captured xterm transaction when deferral starts after compositionend', () => { const el = document.createElement('div') const send = vi.fn() diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts index 6eb2ce0e7e66..b27f25e4742d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-deferred-newline.ts @@ -10,15 +10,19 @@ export const TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS = 200 * right for it; a cursor chord arriving mid-preedit reproduces the corruption the wait exists to * prevent, and a conversion can hold its candidate window open for seconds. Dropping the chord * costs one keypress, firing early costs a mangled line. + * + * Returns a disposer that stops waiting without sending. An indefinite wait has no other exit, so + * a caller that can outlive the composition must hold it — otherwise the listeners stay on the + * terminal element and a later composition flushes the stale send. */ export function sendTerminalInputAfterComposition( terminalElement: HTMLElement | null | undefined, send: () => void, options?: { fallbackMs?: number | null } -): void { +): () => void { if (!terminalElement) { - window.setTimeout(send, 0) - return + const immediateTimer = window.setTimeout(send, 0) + return () => window.clearTimeout(immediateTimer) } const fallbackMs = @@ -27,7 +31,7 @@ export function sendTerminalInputAfterComposition( : (options?.fallbackMs ?? TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS) let done = false - const finish = (): void => { + const stopWaiting = (): void => { if (done) { return } @@ -40,6 +44,13 @@ export function sendTerminalInputAfterComposition( if (fallbackTimer !== undefined) { window.clearTimeout(fallbackTimer) } + } + + const finish = (): void => { + if (done) { + return + } + stopWaiting() // xterm flushes the committed glyph after compositionend. window.setTimeout(send, 0) } @@ -54,6 +65,8 @@ export function sendTerminalInputAfterComposition( terminalElement.addEventListener('compositionend', onCompositionEnd) terminalElement.addEventListener(XTERM_COMPOSITION_SESSION_END_EVENT, onCompositionSessionEnd) const fallbackTimer = fallbackMs === null ? undefined : window.setTimeout(finish, fallbackMs) + + return stopWaiting } export type TerminalImeDeferredNewlineSender = {