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 new file mode 100644 index 000000000000..2200f2d5c9ee --- /dev/null +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime-composing-chord.test.tsx @@ -0,0 +1,223 @@ +// @vitest-environment happy-dom +// #12871: a cursor chord pressed while a syllable is still composing reached the pty ahead of the +// text it was typed after. With `가나` on the line, typing `가나다` and pressing Cmd+Left left +// `다가나` — the composing `다` landed at the cursor's destination. +// +// The composed glyph reaches the pty from the composition session-end handler, which runs after +// the chord's keydown. Only Enter was held for that; every other chord went straight out. +import { cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import type { PtyTransport } from './pty-transport' +import { + installTerminalImeCompositionRoute, + XTERM_COMPOSITION_SESSION_END_EVENT, + XTERM_COMPOSITION_SESSION_START_EVENT +} from './terminal-ime-composition-route' +import { useTerminalKeyboardShortcuts } from './keyboard-handlers' + +type KeyboardHandlersDeps = Parameters[0] + +function keyboardEvent( + type: string, + overrides: { isComposing?: boolean; keyCode?: number } & KeyboardEventInit +): KeyboardEvent { + const event = new KeyboardEvent(type, { bubbles: true, cancelable: true, ...overrides }) + Object.defineProperties(event, { + isComposing: { value: overrides.isComposing ?? false }, + keyCode: { value: overrides.keyCode ?? 0 } + }) + return event +} + +function createHarness(): { + deps: KeyboardHandlersDeps + terminalElement: HTMLDivElement + terminalInput: HTMLTextAreaElement + /** Every byte reaching the pty, in arrival order, whichever route it took. */ + wire: string[] + startComposition: () => void + endComposition: (data: string) => void + dispose: () => void +} { + const scope = document.createElement('div') + const terminalElement = document.createElement('div') + const terminalInput = document.createElement('textarea') + terminalInput.className = 'xterm-helper-textarea' + terminalElement.append(terminalInput) + scope.append(terminalElement) + document.body.append(scope) + + const wire: string[] = [] + const transport = { + getPtyId: () => 'pty-1', + sendInput: (data: string) => { + wire.push(data) + return true + } + } as unknown as PtyTransport + const pane = { + id: 1, + leafId: '00000000-0000-4000-8000-000000000001', + terminal: { + element: terminalElement, + focus: vi.fn(), + getSelection: vi.fn(() => '') + } + } + const manager = { + getActivePane: () => pane, + getPanes: () => [pane] + } as unknown as PaneManager + const route = installTerminalImeCompositionRoute({ + terminalElement, + // The committed glyph takes this route; the chord takes the transport. Both land in `wire`, + // so the assertion is about their order rather than about either one alone. + terminal: { input: (data: string) => void wire.push(data) }, + capturedTransport: transport, + getCurrentTransport: () => transport + }) + + const deps: KeyboardHandlersDeps = { + tabId: 'tab-1', + worktreeId: 'worktree-1', + isActive: true, + keyboardScopeRef: { current: scope }, + managerRef: { current: manager }, + paneTransportsRef: { current: new Map([[pane.id, transport]]) }, + panePtyBindingsRef: { current: new Map() }, + paneCwdRef: { current: new Map() }, + fallbackCwd: '', + expandedPaneIdRef: { current: null }, + setExpandedPane: vi.fn(), + restoreExpandedLayout: vi.fn(), + refreshPaneSizes: vi.fn(), + persistLayoutSnapshot: vi.fn(), + toggleExpandPane: vi.fn(), + setSearchOpen: vi.fn(), + onSearchSelectedText: vi.fn(), + onRequestClosePane: vi.fn(), + onClearPaneScrollback: vi.fn(), + onSetTitle: vi.fn(), + onClearPaneTitle: vi.fn(), + searchOpenRef: { current: false }, + searchStateRef: { current: { query: '', caseSensitive: false, regex: false } }, + macOptionAsAltRef: { current: 'false' } + } + + return { + deps, + terminalElement, + terminalInput, + wire, + startComposition: () => { + terminalElement.dispatchEvent( + new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1 } }) + ) + }, + endComposition: (data: string) => { + terminalElement.dispatchEvent( + new CustomEvent(XTERM_COMPOSITION_SESSION_END_EVENT, { + cancelable: true, + detail: { id: 1, data } + }) + ) + }, + dispose: () => { + route.dispose() + scope.remove() + } + } +} + +describe('a cursor chord pressed during a composition', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue( + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)' + ) + }) + + afterEach(() => { + cleanup() + vi.clearAllTimers() + vi.useRealTimers() + vi.restoreAllMocks() + }) + + function pressCmdArrowLeft( + harness: ReturnType, + isComposing: boolean + ): void { + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'ArrowLeft', + code: 'ArrowLeft', + keyCode: 37, + metaKey: true, + isComposing + }) + ) + } + + // The Korean 2-Set shape: the platform replays the chord unmarked after keyup, so `isComposing` + // is already false — but xterm has not yet emitted the session end that writes the syllable. + it('sends the composed syllable before the chord, not after it', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.startComposition() + pressCmdArrowLeft(harness, false) + expect(harness.wire, 'chord must not reach the pty while the glyph is pending').toEqual([]) + + harness.endComposition('다') + vi.runAllTimers() + + expect(harness.wire).toEqual(['다', '\x01']) + hook.unmount() + harness.dispose() + }) + + // The Japanese shape: still marked composing when the chord is resolved. + it('holds the chord while the keydown is still marked composing', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.startComposition() + pressCmdArrowLeft(harness, true) + expect(harness.wire).toEqual([]) + + harness.endComposition('日本語') + vi.runAllTimers() + + expect(harness.wire).toEqual(['日本語', '\x01']) + hook.unmount() + harness.dispose() + }) + + // A conversion can hold its candidate window open for seconds. A timer that fired mid-preedit + // would put the chord back ahead of the text, which is the whole defect. + it('does not fall back to a timer while the composition is still open', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.startComposition() + pressCmdArrowLeft(harness, true) + vi.advanceTimersByTime(30_000) + + expect(harness.wire).toEqual([]) + hook.unmount() + harness.dispose() + }) + + it('sends immediately when no composition is in flight', () => { + const harness = createHarness() + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + pressCmdArrowLeft(harness, false) + + expect(harness.wire).toEqual(['\x01']) + 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 8353f4201bf1..c796e58ed049 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -15,7 +15,8 @@ import { getTerminalImeModifiedEnterKind, isTerminalImeConsumedKey, isTerminalImeEnterKeyUp, - isTerminalImeProcessEnter + isTerminalImeProcessEnter, + sendTerminalInputAfterComposition } from './terminal-ime-deferred-newline' import { hasPendingTerminalImeComposition } from './terminal-ime-composition-route' import { @@ -629,6 +630,17 @@ export function useTerminalKeyboardShortcuts({ deferredNewlineSender.defer(e, pane.terminal.element, sendResolvedInput) return } + // Why: the composed glyph reaches the pty from the composition session-end handler, which + // 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. + if (e.isComposing || hasPendingImeComposition) { + sendTerminalInputAfterComposition(pane.terminal.element, sendResolvedInput, { + fallbackMs: null + }) + return + } sendResolvedInput() return } 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 d1ef2b882ebe..5b8613c1f7fd 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 @@ -37,6 +37,22 @@ describe('sendTerminalInputAfterComposition', () => { expect(send).toHaveBeenCalledTimes(1) }) + // #12871: a newline arriving late still arrives, so the fallback timer is right for it. A + // cursor chord arriving mid-preedit is the corruption the wait exists to prevent, and a + // conversion can hold its candidate window open for seconds — far past the fallback. + it('never fires on a timer when the caller opts out of the fallback', () => { + const el = document.createElement('div') + const send = vi.fn() + + sendTerminalInputAfterComposition(el, send, { fallbackMs: null }) + vi.advanceTimersByTime(60_000) + expect(send).not.toHaveBeenCalled() + + el.dispatchEvent(new Event('compositionend')) + vi.runAllTimers() + expect(send).toHaveBeenCalledTimes(1) + }) + it('falls back to sending when no compositionend arrives', () => { 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 2fd141fe3ce0..6eb2ce0e7e66 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,17 +5,26 @@ import { export const TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS = 200 +/** + * `fallbackMs: null` waits indefinitely. A newline arriving late still arrives, so a timer is + * 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. + */ export function sendTerminalInputAfterComposition( terminalElement: HTMLElement | null | undefined, send: () => void, - options?: { fallbackMs?: number } + options?: { fallbackMs?: number | null } ): void { if (!terminalElement) { window.setTimeout(send, 0) return } - const fallbackMs = options?.fallbackMs ?? TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS + const fallbackMs = + options?.fallbackMs === null + ? null + : (options?.fallbackMs ?? TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS) let done = false const finish = (): void => { @@ -28,7 +37,9 @@ export function sendTerminalInputAfterComposition( XTERM_COMPOSITION_SESSION_END_EVENT, onCompositionSessionEnd ) - window.clearTimeout(fallbackTimer) + if (fallbackTimer !== undefined) { + window.clearTimeout(fallbackTimer) + } // xterm flushes the committed glyph after compositionend. window.setTimeout(send, 0) } @@ -42,7 +53,7 @@ export function sendTerminalInputAfterComposition( const onCompositionSessionEnd = (): void => finishAfterPendingComposition() terminalElement.addEventListener('compositionend', onCompositionEnd) terminalElement.addEventListener(XTERM_COMPOSITION_SESSION_END_EVENT, onCompositionSessionEnd) - const fallbackTimer = window.setTimeout(finish, fallbackMs) + const fallbackTimer = fallbackMs === null ? undefined : window.setTimeout(finish, fallbackMs) } export type TerminalImeDeferredNewlineSender = { diff --git a/tests/e2e/terminal-korean-composing-chord-order.spec.ts b/tests/e2e/terminal-korean-composing-chord-order.spec.ts new file mode 100644 index 000000000000..1217bc9d34df --- /dev/null +++ b/tests/e2e/terminal-korean-composing-chord-order.spec.ts @@ -0,0 +1,87 @@ +/** + * #12871: a cursor chord pressed while a syllable is still composing reached the pty ahead of the + * text it was typed after. With `가나` on the line, typing `가나다` and pressing Cmd+Left left + * `다가다` — the composing `다` landed at the cursor's destination. + * + * The unit coverage in `keyboard-handlers-ime-composing-chord.test.tsx` pins the handler's + * ordering against a synthetic transport. This asserts the same ordering at the pty, where the + * committed glyph and the chord arrive by two different routes — the composition session-end + * handler and the transport — and only their merged order is observable. + */ +import { expect, test } from './helpers/orca-app' +import { closeTerminalImePaneArena, openTerminalImePaneArena } from './terminal-ime-pane-arena' +import { + commitImeText, + composeHangulSyllable, + dispatchPlainEnter, + type ImeKeyIdentity +} from './terminal-ime-cdp-composition' +import { + createTerminalImeByteReader, + removeTerminalImeByteReader, + startTerminalImeByteReader, + waitForTerminalImeBytes +} from './terminal-ime-byte-reader' +import { applyImePlatformPolicy, expectImePlatformPolicy } from './terminal-ime-platform-policy' + +const JAMO: Record = { + ㄷ: { key: 'ㄷ', code: 'KeyE', keyCode: 229 }, + ㅏ: { key: 'ㅏ', code: 'KeyK', keyCode: 229 } +} + +/** ㄷ → ㅏ assembles 다, which stays composing because a final consonant could still follow. */ +const DA_FRAMES = [ + { jamoKey: JAMO['ㄷ'], preedit: 'ㄷ' }, + { jamoKey: JAMO['ㅏ'], preedit: '다' } +] as const + +/** Cmd+Left resolves to Ctrl+A (readline start-of-line) — the chord from the report. */ +const CMD_LEFT_BYTE = '\x01' + +test.describe('Terminal 2-Set Korean composing-chord order', () => { + test('sends the composing syllable before a Cmd+Left pressed during it', async ({ + orcaPage, + testRepoPath + }, testInfo) => { + // Cmd+Left resolves to \x01 only under the macOS branch of the shortcut policy, so a Linux + // runner produces no chord byte at all and the spec would pass by measuring nothing. Pinning + // the renderer's platform is what lets the reported chord run on any shard; the assertion + // below fails loudly if the override did not take. + await applyImePlatformPolicy(orcaPage, 'mac') + await expectImePlatformPolicy(orcaPage, 'mac') + + const arena = await openTerminalImePaneArena(orcaPage) + const reader = createTerminalImeByteReader(testRepoPath, 1) + let completed = false + try { + await startTerminalImeByteReader(orcaPage, arena.ptyId, reader) + + await composeHangulSyllable(arena.session, orcaPage, DA_FRAMES) + + // Pressed while the syllable is still in preedit. Before the fix this reached the pty + // immediately, ahead of the 다 that had been typed first. + await arena.session.send('Input.dispatchKeyEvent', { + type: 'rawKeyDown', + key: 'ArrowLeft', + code: 'ArrowLeft', + windowsVirtualKeyCode: 37, + nativeVirtualKeyCode: 37, + modifiers: 4 + }) + + await commitImeText(arena.session, '다') + // The held chord flushes a macrotask after the composition session ends. Enter is only here + // to terminate the line for the reader, so let the chord land before adding it — otherwise + // the newline overtakes it and the assertion measures the wrong pair. + await orcaPage.waitForTimeout(250) + await dispatchPlainEnter(arena.session) + + const received = await waitForTerminalImeBytes(orcaPage, reader) + expect(received).toEqual([Buffer.from(`다${CMD_LEFT_BYTE}\n`).toString('hex')]) + completed = true + } finally { + await closeTerminalImePaneArena(arena, testInfo, 'korean-composing-chord-order', !completed) + removeTerminalImeByteReader(reader) + } + }) +})