mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
fix(ime): stop dropping and splitting composition commits in the terminal
Two ways a committed composition was lost or corrupted. The composition route called preventDefault() before checking whether it owned the session. A route installed mid-composition never saw the matching session start, so it suppressed xterm's own insertion and then returned without delivering anything — the commit vanished. Ownership is now decided first; preventDefault stays for sessions the route does own, including the ones it deliberately drops after a transport swap, since that drop is its decision to make. The deferred-newline fallback fired `finish` directly, bypassing the pending-composition re-check that the event path uses. A composition still open at 200 ms got a newline sent into the middle of its preedit. The deadline now re-arms while a composition is pending, and its 2 s ceiling is measured from the last compositionupdate rather than from the start. A slow candidate window no longer splits the preedit, and neither does multi-segment bunsetsu conversion, which stays legitimately pending well past any fixed total-elapsed ceiling. A composition that stops progressing still cannot strand the keystroke. Co-authored-by: Orca <help@stably.ai> The re-arm also treats a live compositionupdate as evidence in its own right, not just a session the route recorded. A route installed while a composition was already in flight never saw that session start, so it has nothing to report as pending and the deadline would otherwise split that composition too.
This commit is contained in:
@@ -149,6 +149,35 @@ describe('installTerminalImeCompositionRoute', () => {
|
||||
expect(harness.input).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('leaves an uncaptured session to xterm when installed mid-composition', () => {
|
||||
const harness = createHarness()
|
||||
|
||||
// No start was seen, so nothing here can deliver the text. Cancelling the event
|
||||
// would suppress xterm's own insertion and drop the commit on the floor.
|
||||
const notPrevented = harness.end(1, '한')
|
||||
|
||||
expect(notPrevented).toBe(true)
|
||||
expect(harness.input).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('suppresses xterm insertion only for a session it captured', () => {
|
||||
const harness = createHarness()
|
||||
harness.start(1)
|
||||
|
||||
expect(harness.end(1, '한')).toBe(false)
|
||||
expect(harness.input).toHaveBeenCalledWith('한')
|
||||
})
|
||||
|
||||
it('keeps suppressing insertion for a captured session it deliberately drops', () => {
|
||||
const harness = createHarness()
|
||||
harness.start(1)
|
||||
harness.state.currentTransport = createTransport('pty-replacement')
|
||||
|
||||
// Owned, so xterm must still stand down — the drop is this route's decision.
|
||||
expect(harness.end(1, '한')).toBe(false)
|
||||
expect(harness.input).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each(['terminal close', 'tab unmount'])(
|
||||
'releases the captured session and listeners on %s',
|
||||
() => {
|
||||
|
||||
@@ -82,11 +82,13 @@ export function installTerminalImeCompositionRoute(args: {
|
||||
if (!detail) {
|
||||
return
|
||||
}
|
||||
event.preventDefault()
|
||||
const captured = sessions.get(detail.id)
|
||||
if (!captured) {
|
||||
// Not our session — the route was installed mid-composition, so no start was seen.
|
||||
// Preventing default here would suppress xterm's insertion with nothing to replace it.
|
||||
return
|
||||
}
|
||||
event.preventDefault()
|
||||
sessions.delete(detail.id)
|
||||
adjustPendingCompositionCount(terminalElement, -1)
|
||||
if (
|
||||
|
||||
@@ -116,6 +116,114 @@ describe('sendTerminalInputAfterComposition', () => {
|
||||
expect(send).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('re-arms past the fallback rather than splitting a still-pending composition', () => {
|
||||
const el = document.createElement('div')
|
||||
const send = vi.fn()
|
||||
const transport = { getPtyId: () => 'pty-1' } as unknown as PtyTransport
|
||||
const route = installTerminalImeCompositionRoute({
|
||||
terminalElement: el,
|
||||
terminal: { input: vi.fn() },
|
||||
capturedTransport: transport,
|
||||
getCurrentTransport: () => transport
|
||||
})
|
||||
el.dispatchEvent(
|
||||
new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1, data: '한' } })
|
||||
)
|
||||
|
||||
sendTerminalInputAfterComposition(el, send, { fallbackMs: 50, maxIdleMs: 1000 })
|
||||
vi.advanceTimersByTime(500)
|
||||
|
||||
// A slow candidate window is not a reason to send Enter into the middle of the preedit.
|
||||
expect(send).not.toHaveBeenCalled()
|
||||
|
||||
el.dispatchEvent(
|
||||
new CustomEvent(XTERM_COMPOSITION_SESSION_END_EVENT, { detail: { id: 1, data: '한' } })
|
||||
)
|
||||
vi.advanceTimersByTime(0)
|
||||
expect(send).toHaveBeenCalledTimes(1)
|
||||
|
||||
route.dispose()
|
||||
})
|
||||
|
||||
it('holds the newline through a long conversion that keeps updating its preedit', () => {
|
||||
const el = document.createElement('div')
|
||||
const send = vi.fn()
|
||||
const transport = { getPtyId: () => 'pty-1' } as unknown as PtyTransport
|
||||
const route = installTerminalImeCompositionRoute({
|
||||
terminalElement: el,
|
||||
terminal: { input: vi.fn() },
|
||||
capturedTransport: transport,
|
||||
getCurrentTransport: () => transport
|
||||
})
|
||||
el.dispatchEvent(
|
||||
new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1, data: 'にほん' } })
|
||||
)
|
||||
|
||||
sendTerminalInputAfterComposition(el, send, { fallbackMs: 50, maxIdleMs: 200 })
|
||||
|
||||
// Multi-segment bunsetsu conversion: each segment takes longer than the ceiling
|
||||
// in total, but never leaves the preedit idle for a full ceiling's worth.
|
||||
for (let segment = 0; segment < 6; segment += 1) {
|
||||
vi.advanceTimersByTime(150)
|
||||
el.dispatchEvent(new Event('compositionupdate'))
|
||||
}
|
||||
expect(send).not.toHaveBeenCalled()
|
||||
|
||||
el.dispatchEvent(
|
||||
new CustomEvent(XTERM_COMPOSITION_SESSION_END_EVENT, { detail: { id: 1, data: '日本' } })
|
||||
)
|
||||
vi.advanceTimersByTime(0)
|
||||
expect(send).toHaveBeenCalledTimes(1)
|
||||
|
||||
route.dispose()
|
||||
})
|
||||
|
||||
it('holds for a composition already in flight when nothing recorded its session start', () => {
|
||||
const el = document.createElement('div')
|
||||
const send = vi.fn()
|
||||
|
||||
// No route installed, so there is no pending session to report — the live
|
||||
// compositionupdate is the only evidence the composition exists.
|
||||
sendTerminalInputAfterComposition(el, send, { fallbackMs: 50, maxIdleMs: 200 })
|
||||
for (let tick = 0; tick < 5; tick += 1) {
|
||||
vi.advanceTimersByTime(30)
|
||||
el.dispatchEvent(new Event('compositionupdate'))
|
||||
}
|
||||
expect(send).not.toHaveBeenCalled()
|
||||
|
||||
el.dispatchEvent(new Event('compositionend'))
|
||||
vi.advanceTimersByTime(0)
|
||||
expect(send).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('sends once at the ceiling when a composition never ends', () => {
|
||||
const el = document.createElement('div')
|
||||
const send = vi.fn()
|
||||
const transport = { getPtyId: () => 'pty-1' } as unknown as PtyTransport
|
||||
const route = installTerminalImeCompositionRoute({
|
||||
terminalElement: el,
|
||||
terminal: { input: vi.fn() },
|
||||
capturedTransport: transport,
|
||||
getCurrentTransport: () => transport
|
||||
})
|
||||
el.dispatchEvent(
|
||||
new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1, data: '한' } })
|
||||
)
|
||||
|
||||
sendTerminalInputAfterComposition(el, send, { fallbackMs: 50, maxIdleMs: 200 })
|
||||
vi.advanceTimersByTime(150)
|
||||
expect(send).not.toHaveBeenCalled()
|
||||
|
||||
// Bounded: a composition that stops progressing must not strand the keystroke.
|
||||
vi.advanceTimersByTime(100)
|
||||
expect(send).toHaveBeenCalledTimes(1)
|
||||
|
||||
vi.runAllTimers()
|
||||
expect(send).toHaveBeenCalledTimes(1)
|
||||
|
||||
route.dispose()
|
||||
})
|
||||
|
||||
it('still delivers the input on the next macrotask without a terminal element', () => {
|
||||
const send = vi.fn()
|
||||
|
||||
|
||||
@@ -5,18 +5,29 @@ import {
|
||||
|
||||
export const TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS = 200
|
||||
|
||||
// Ceiling for a composition that stops making progress. Measured from the last
|
||||
// compositionupdate, not from the start: multi-segment bunsetsu conversion keeps a
|
||||
// Japanese composition legitimately pending far longer than this, and finishing on a
|
||||
// total-elapsed ceiling would split it with the newline this deferral exists to hold back.
|
||||
export const TERMINAL_IME_DEFERRED_NEWLINE_MAX_IDLE_MS = 2000
|
||||
|
||||
export function sendTerminalInputAfterComposition(
|
||||
terminalElement: HTMLElement | null | undefined,
|
||||
send: () => void,
|
||||
options?: { fallbackMs?: number }
|
||||
options?: { fallbackMs?: number; maxIdleMs?: number }
|
||||
): void {
|
||||
if (!terminalElement) {
|
||||
window.setTimeout(send, 0)
|
||||
return
|
||||
}
|
||||
|
||||
const fallbackMs = options?.fallbackMs ?? TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS
|
||||
const fallbackMs = Math.max(1, options?.fallbackMs ?? TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS)
|
||||
const maxIdleMs = Math.max(
|
||||
fallbackMs,
|
||||
options?.maxIdleMs ?? TERMINAL_IME_DEFERRED_NEWLINE_MAX_IDLE_MS
|
||||
)
|
||||
let done = false
|
||||
let idleMs = 0
|
||||
|
||||
const finish = (): void => {
|
||||
if (done) {
|
||||
@@ -24,6 +35,7 @@ export function sendTerminalInputAfterComposition(
|
||||
}
|
||||
done = true
|
||||
terminalElement.removeEventListener('compositionend', onCompositionEnd)
|
||||
terminalElement.removeEventListener('compositionupdate', onCompositionUpdate)
|
||||
terminalElement.removeEventListener(
|
||||
XTERM_COMPOSITION_SESSION_END_EVENT,
|
||||
onCompositionSessionEnd
|
||||
@@ -40,9 +52,33 @@ export function sendTerminalInputAfterComposition(
|
||||
}
|
||||
const onCompositionEnd = (): void => finishAfterPendingComposition()
|
||||
const onCompositionSessionEnd = (): void => finishAfterPendingComposition()
|
||||
// Each preedit change proves the composition is still progressing, so the idle
|
||||
// ceiling only ever expires on a composition that is genuinely stuck.
|
||||
let sawRecentUpdate = false
|
||||
const onCompositionUpdate = (): void => {
|
||||
idleMs = 0
|
||||
sawRecentUpdate = true
|
||||
}
|
||||
terminalElement.addEventListener('compositionend', onCompositionEnd)
|
||||
terminalElement.addEventListener('compositionupdate', onCompositionUpdate)
|
||||
terminalElement.addEventListener(XTERM_COMPOSITION_SESSION_END_EVENT, onCompositionSessionEnd)
|
||||
const fallbackTimer = window.setTimeout(finish, fallbackMs)
|
||||
|
||||
// Re-arm rather than sending: firing mid-composition splits the preedit with a newline.
|
||||
const onFallbackDeadline = (): void => {
|
||||
idleMs += fallbackMs
|
||||
// A live compositionupdate counts as evidence on its own: a route installed while a
|
||||
// composition was already in flight never saw that session start, so it has nothing
|
||||
// recorded to report as pending.
|
||||
const compositionInProgress =
|
||||
hasPendingTerminalImeComposition(terminalElement) || sawRecentUpdate
|
||||
sawRecentUpdate = false
|
||||
if (!compositionInProgress || idleMs >= maxIdleMs) {
|
||||
finish()
|
||||
return
|
||||
}
|
||||
fallbackTimer = window.setTimeout(onFallbackDeadline, fallbackMs)
|
||||
}
|
||||
let fallbackTimer = window.setTimeout(onFallbackDeadline, fallbackMs)
|
||||
}
|
||||
|
||||
export type TerminalImeDeferredNewlineSender = {
|
||||
|
||||
Reference in New Issue
Block a user