Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof useTerminalKeyboardShortcuts>[0]
Expand All @@ -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<EventListenerOrEventListenerObject>()
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
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -110,6 +135,7 @@ function createHarness(): {
terminalElement,
terminalInput,
wire,
deferralListenerCount,
startComposition: () => {
terminalElement.dispatchEvent(
new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1 } })
Expand Down Expand Up @@ -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()
})
})
14 changes: 8 additions & 6 deletions src/renderer/src/components/terminal-pane/keyboard-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -986,6 +986,7 @@ export function useTerminalKeyboardShortcuts({
terminalImeEnterModifierKeydowns.clear()
modifiedEnterChordOwner.clear()
deferredNewlineSender.clearRedispatchedEnters()
deferredChordSender.cancelPending()
observedEnterKeydownTimeStamps.clear()
}

Expand All @@ -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 })
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
})
})
Original file line number Diff line number Diff line change
@@ -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()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
createTerminalImeModifiedEnterChordOwner,
isTerminalImeEnterKeyUp,
isTerminalImeProcessEnter,
sendTerminalInputAfterComposition
sendTerminalInputAfterComposition,
TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS
} from './terminal-ime-deferred-newline'
import {
installTerminalImeCompositionRoute,
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
Loading
Loading