diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.test.ts b/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.test.ts index 16fb09252e0..3d8d114aff6 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.test.ts @@ -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', () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.ts b/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.ts index 788fa9b83ba..5809c9c74af 100644 --- a/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.ts +++ b/src/renderer/src/components/terminal-pane/terminal-ime-composition-route.ts @@ -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 ( 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 fe7860e35fe..786102ed132 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 @@ -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() 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 992ae182698..221ba710f0f 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 @@ -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 = {