diff --git a/src/web/public/input-cjk.js b/src/web/public/input-cjk.js index 7db12e6b2..d2bf72bf7 100644 --- a/src/web/public/input-cjk.js +++ b/src/web/public/input-cjk.js @@ -27,8 +27,8 @@ * * Solution: outside composition, flush is DEBOUNCED (200ms). The entire * delete→reinsert cycle collapses into one flush of the final textarea value. - * Keyboard typing of single printable characters still goes through the - * keydown handler (immediate, no debounce). + * Physical-keyboard commits are flushed immediately after the input event + * exposes the final browser/IME text; keydown never guesses that text. * * ## Phantom character for Android backspace * @@ -56,8 +56,7 @@ const CjkInput = (() => { let _compositionFlushTimer = null; let _dictationActive = false; let _dictationDecayTimer = null; - let _keydownSentAt = 0; - let _keydownSentText = ''; + let _printableKeydownAt = null; const _listeners = {}; const PHANTOM = '​'; @@ -197,6 +196,7 @@ const CjkInput = (() => { _send = send; _composing = false; + _printableKeydownAt = null; _flushTimer = null; _textarea = document.getElementById('cjkInput'); if (!_textarea) return this; @@ -234,6 +234,7 @@ const CjkInput = (() => { }; _listeners.blur = () => { _t(`blur composing=${_composing} ${_vdesc(_textarea.value)}`); + _printableKeydownAt = null; // Keep cjkActive while CJK input is visible — iOS dictation and system // UI may steal focus temporarily, and clearing the flag during that // window lets xterm's onData process duplicated input. @@ -253,6 +254,7 @@ const CjkInput = (() => { _listeners.compositionstart = () => { _t(`compstart ${_vdesc(_textarea.value)}`); _composing = true; + _printableKeydownAt = null; _cancelDebouncedFlush(); // Leave textarea.value untouched — programmatic changes during // compositionstart cancel the IME composition on iOS Safari. @@ -277,6 +279,7 @@ const CjkInput = (() => { // ── Keydown: special keys work REGARDLESS of composition state ── _listeners.keydown = (e) => { _t(`keydown ${_kdesc(e.key)} kc=${e.keyCode} ic=${e.isComposing} c=${_composing}`); + _printableKeydownAt = null; if (e.key === 'Enter') { e.preventDefault(); _composing = false; @@ -325,16 +328,11 @@ const CjkInput = (() => { return; } - // Single printable character: send immediately to PTY. - // Third-party IMEs on iOS may ignore preventDefault, so the char - // still enters the textarea and fires an input event — _keydownSentAt - // tells the input handler to skip that echo. + // A printable KeyboardEvent.key is the physical key, not necessarily + // the committed text. Let the browser/IME produce the input event so + // full-width punctuation and other layout transforms are preserved. if (e.key.length === 1 && !e.ctrlKey && !e.altKey && !e.metaKey && _isEffectivelyEmpty()) { - e.preventDefault(); - _send(e.key); - _keydownSentAt = performance.now(); - _keydownSentText = e.key; - _resetToPhantom(); + _printableKeydownAt = performance.now(); return; } }; @@ -343,6 +341,8 @@ const CjkInput = (() => { // ── Input event: primary path for virtual keyboards + dictation ── _listeners.input = (e) => { _t(`input ${e.inputType || '?'} ic=${e.isComposing} c=${_composing} ${_vdesc(_textarea.value)}`); + const printableKeydownAt = _printableKeydownAt; + _printableKeydownAt = null; // ── Stuck-composition recovery ── // Some IMEs (WeChat/Sogou keyboards) fire compositionstart without a // matching compositionend. A stale _composing=true blocks every flush @@ -388,18 +388,18 @@ const CjkInput = (() => { if (_composing) return; - // Keydown handler already sent this character — clear the textarea - // echo that the IME inserted despite preventDefault. Content-checked: - // only a value matching the sent char is an echo. Anything else (e.g. - // an IME committing CJK text right after a keydown-sent char) is real - // input and must flow through to the debounced flush, not be dropped. - if (performance.now() - _keydownSentAt < 100) { - const cur = _strip(_textarea.value); - if (cur === '' || cur === _keydownSentText) { - _t('echo-drop'); - _resetToPhantom(); - return; - } + // A recent physical printable key makes this insertText a keyboard + // commit, so keep the old zero-latency path. Send the textarea's final + // Unicode value, never KeyboardEvent.key, because the IME may have + // transformed punctuation or the active layout may differ. + if ( + e.inputType === 'insertText' && + printableKeydownAt !== null && + performance.now() - printableKeydownAt < 100 + ) { + _cancelDebouncedFlush(); + _flush(); + return; } // Outside composition: keyboard typing or voice dictation. @@ -425,6 +425,7 @@ const CjkInput = (() => { clearTimeout(_compositionFlushTimer); _compositionFlushTimer = null; _composing = false; + _printableKeydownAt = null; _resetToPhantom(); }, @@ -446,6 +447,7 @@ const CjkInput = (() => { } window.cjkActive = false; _composing = false; + _printableKeydownAt = null; for (const key of Object.keys(_listeners)) delete _listeners[key]; _initialized = false; }, diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index ed5079353..19fc93366 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -287,11 +287,12 @@ Object.assign(CodemanApp.prototype, { this._installMobileTapMouseGuard(); this._installTouchSelectionFocusGuard(); - // Suppress xterm key handling during CJK IME composition. - // Without this, xterm processes raw keyDown events (e.g., "Process" key) - // during composition, causing duplicate or garbled input. + // Let xterm's CompositionHelper own IME key events. In particular, a + // non-composing keyCode 229 is how an active IME commits numbers and + // punctuation; returning false here would stop xterm before it can diff + // the helper textarea and emit the committed Unicode text. this.terminal.attachCustomKeyEventHandler((ev) => { - if (ev.isComposing || ev.keyCode === 229) return false; + if (ev.isComposing || ev.key === 'Process' || ev.keyCode === 229) return true; // Let the app's Alt/Option session-nav and Command Palette shortcuts reach the document keydown handler // (app.js switches tabs by PHYSICAL e.code) instead of xterm injecting ESC into @@ -399,72 +400,6 @@ Object.assign(CodemanApp.prototype, { return true; }); - // Android virtual keyboard fix: catch non-composition input events. - // On Android Chrome, typing symbols (e.g., "/" from Gboard's symbol keyboard) - // sends keyCode 229 + input event WITHOUT compositionstart/end wrapping. - // The custom key handler above returns false for keyCode 229, telling xterm - // to ignore the keydown. xterm.js expects the character to arrive via - // composition events, but since there's no composition, the character is lost. - // This listener catches those orphaned input events and forwards them to onData. - { - const xtermTextarea = container.querySelector('.xterm-helper-textarea'); - if (xtermTextarea && MobileDetection.isTouchDevice()) { - let composing = false; - let lastKeydownHandled = 0; - xtermTextarea.addEventListener('compositionstart', () => { composing = true; }); - xtermTextarea.addEventListener('compositionend', () => { composing = false; }); - // Track when xterm handles a keydown normally (non-229 keyCode). - // If xterm processed the keydown, it will emit onData itself -- - // the input event handler below must NOT re-send the character. - xtermTextarea.addEventListener('keydown', (e) => { - if (!e.isComposing && e.keyCode !== 229) { - lastKeydownHandled = Date.now(); - } - }); - xtermTextarea.addEventListener('input', (e) => { - // Only handle insertText events outside of composition -- these are - // the ones xterm.js misses on Android virtual keyboards. - if (composing || e.isComposing) return; - if (e.inputType !== 'insertText' || !e.data) return; - // If xterm just handled a keydown (within 50ms), it already sent the - // char via onData. Skip to avoid double-send (e.g., Shift+A => AA). - if (Date.now() - lastKeydownHandled < 50) return; - // xterm.js may have already processed this via its own input handler. - // Check if the textarea was cleared by xterm (value is empty or just - // whitespace) -- if so, xterm handled it and we should not double-send. - // Use a microtask to check after xterm's own handlers have run. - const data = e.data; - const pendingBefore = this._localEchoOverlay?.pendingText || ''; - Promise.resolve().then(() => { - if ( - this._lastTerminalData?.data === data && - performance.now() - this._lastTerminalData.time < 100 - ) { - xtermTextarea.value = ''; - return; - } - const pendingAfter = this._localEchoOverlay?.pendingText || ''; - if ( - this._localEchoEnabled && - pendingAfter.length > pendingBefore.length && - pendingAfter.endsWith(data) - ) { - xtermTextarea.value = ''; - return; - } - // If xterm cleared the textarea, it processed the input -- skip. - const val = xtermTextarea.value; - if (!val || (val.trim() === '' && data !== ' ')) return; - // xterm didn't process it -- forward to terminal as if typed. - // Emit via onData path by writing to terminal's input handler. - this.terminal._core.coreService.triggerDataEvent(data, true); - // Clear the textarea to prevent xterm from processing it later. - xtermTextarea.value = ''; - }); - }); - } - } - // WebGL renderer for GPU-accelerated terminal rendering. // Previously caused "page unresponsive" crashes from synchronous GPU stalls, // but the mode-aware 32/64KB frame cap in flushPendingWrites() now prevents diff --git a/test/input-cjk.test.ts b/test/input-cjk.test.ts index 54b250b7a..993ad5a39 100644 --- a/test/input-cjk.test.ts +++ b/test/input-cjk.test.ts @@ -5,7 +5,7 @@ * composition/keydown/input event sequences against a stub textarea. * Focus: the intermittent "Chinese characters silently lost" failure modes — * stuck composition state, deferred flush racing the next composition, and - * the keydown-echo suppression window swallowing a real IME commit. + * physical-key punctuation transformed into full-width IME output. */ import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; @@ -116,6 +116,36 @@ describe('CJK input module', () => { expect(textarea.value).toBe(PHANTOM); }); + it('lets the IME transform printable keys before sending full-width punctuation', () => { + const { textarea, sent } = loadCjkHarness(); + const committed = Array.from(',。!?;:“”、《》、()'); + const physicalKeys = [',', '.', '!', '?', ';', ':', '"', '"', '\\', '<', '>', '\\', '(', ')']; + + textarea.fire('compositionstart'); + textarea.value = PHANTOM + '中文'; + textarea.fire('input', { isComposing: true, inputType: 'insertCompositionText' }); + textarea.fire('compositionend'); + vi.advanceTimersByTime(10); + vi.advanceTimersByTime(1000); + + for (const [index, punctuation] of committed.entries()) { + const preventDefault = vi.fn(); + textarea.fire('keydown', { + key: physicalKeys[index], + ctrlKey: false, + altKey: false, + metaKey: false, + preventDefault, + }); + expect(preventDefault).not.toHaveBeenCalled(); + + textarea.value = PHANTOM + punctuation; + textarea.fire('input', { isComposing: false, inputType: 'insertText' }); + } + + expect(sent).toEqual(['中文', ...committed]); + }); + it('recovers committed text when compositionend never fires (stuck composition)', () => { const { textarea, sent } = loadCjkHarness(); @@ -124,6 +154,7 @@ describe('CJK input module', () => { textarea.value = PHANTOM + '你好'; // The commit arrives as a plain input event outside composition. textarea.fire('input', { isComposing: false, inputType: 'insertText' }); + expect(sent).toEqual([]); vi.advanceTimersByTime(200); expect(sent).toEqual(['你好']); @@ -151,30 +182,28 @@ describe('CJK input module', () => { expect(sent).toEqual(['你好世界']); }); - it('does not discard an IME commit landing inside the keydown echo window', () => { + it('sends the transformed IME commit that follows a printable keydown', () => { const { textarea, sent } = loadCjkHarness(); - // English char goes out immediately via keydown. + vi.advanceTimersByTime(1000); + // The physical key is not committed text and must not be sent by itself. textarea.fire('keydown', { key: 'a', ctrlKey: false, altKey: false, metaKey: false }); - expect(sent).toEqual(['a']); + expect(sent).toEqual([]); - // Within 100ms the IME commits Chinese via a bare input event. + // The browser/IME supplies the canonical text in the following input. vi.advanceTimersByTime(50); textarea.value = PHANTOM + '你好'; textarea.fire('input', { isComposing: false, inputType: 'insertText' }); - vi.advanceTimersByTime(200); - expect(sent).toEqual(['a', '你好']); + expect(sent).toEqual(['你好']); }); - it('still suppresses the true textarea echo of a keydown-sent character', () => { + it('sends a printable physical key exactly once after its input event', () => { const { textarea, sent } = loadCjkHarness(); textarea.fire('keydown', { key: 'a', ctrlKey: false, altKey: false, metaKey: false }); - expect(sent).toEqual(['a']); + expect(sent).toEqual([]); - // Third-party IME ignored preventDefault — the same char echoes into - // the textarea. It must be dropped, not sent twice. vi.advanceTimersByTime(10); textarea.value = PHANTOM + 'a'; textarea.fire('input', { isComposing: false, inputType: 'insertText' }); @@ -242,6 +271,8 @@ describe('CJK input module', () => { vi.advanceTimersByTime(10); textarea.fire('keydown', { key: '囍', ctrlKey: false, altKey: false, metaKey: false }); + textarea.value = PHANTOM + '囍'; + textarea.fire('input', { isComposing: false, inputType: 'insertText' }); textarea.value = PHANTOM + '秘密'; textarea.fire('blur'); expect(sent).toEqual(['秘密口令', '囍']); diff --git a/test/terminal-copy-shortcut.test.ts b/test/terminal-copy-shortcut.test.ts index 00446fcef..f719ed3cd 100644 --- a/test/terminal-copy-shortcut.test.ts +++ b/test/terminal-copy-shortcut.test.ts @@ -12,7 +12,7 @@ * emitted through onData (the bytes that would reach the PTY). * * Browser-driven, so it is excluded from `npm run test:ci` like the other - * Playwright suites. Run locally: npm test -- test/terminal-copy-shortcut.test.ts + * Playwright suites. Run locally: npm run test:browser -- test/terminal-copy-shortcut.test.ts * * Port: 3174 (per MEMORY.md, ports 3150+ for tests) */ @@ -23,6 +23,7 @@ import { WebServer } from '../src/web/server.js'; const PORT = 3174; const BASE_URL = `http://localhost:${PORT}`; +const IME_PUNCTUATION = ',。!?;:“”、《》、()'; describe('terminal Ctrl+C smart copy', () => { let server: WebServer; @@ -102,6 +103,59 @@ describe('terminal Ctrl+C smart copy', () => { })); } + async function captureImeInput(targetPage: Page) { + await targetPage.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 }); + await targetPage.evaluate(() => { + const term = (window as any).app.terminal; + (window as any).__data = []; + if (!(window as any).__dataHooked) { + term.onData((d: string) => (window as any).__data.push(d)); + (window as any).__dataHooked = true; + } + (document.querySelector('.xterm-helper-textarea') as HTMLElement).focus(); + }); + + const cdp = await targetPage.context().newCDPSession(targetPage); + await cdp.send('Input.imeSetComposition', { text: '中文', selectionStart: 2, selectionEnd: 2 }); + await cdp.send('Input.insertText', { text: '中文' }); + await targetPage.waitForFunction(() => (window as any).__data.join('') === '中文', null, { polling: 10 }); + + let expected = '中文'; + for (const punctuation of Array.from(IME_PUNCTUATION)) { + // Keep keydown -> DOM mutation -> input in one browser task, as a + // native key default action does. Separate CDP calls can let xterm's + // zero-delay textarea diff run before Input.insertText reaches the page. + await targetPage.evaluate((text) => { + const textarea = document.querySelector('.xterm-helper-textarea') as HTMLTextAreaElement; + const down = new KeyboardEvent('keydown', { + key: 'Process', + code: 'Unidentified', + bubbles: true, + cancelable: true, + composed: true, + }); + Object.defineProperties(down, { keyCode: { value: 229 }, which: { value: 229 } }); + textarea.dispatchEvent(down); + if (!document.execCommand('insertText', false, text)) throw new Error('browser rejected insertText'); + const up = new KeyboardEvent('keyup', { + key: 'Process', + code: 'Unidentified', + bubbles: true, + cancelable: true, + composed: true, + }); + Object.defineProperties(up, { keyCode: { value: 229 }, which: { value: 229 } }); + textarea.dispatchEvent(up); + }, punctuation); + expected += punctuation; + await targetPage.waitForFunction((text) => (window as any).__data.join('') === text, expected, { + polling: 10, + }); + } + + return targetPage.evaluate(() => (window as any).__data as string[]); + } + it('copies the selection and sends nothing to the PTY', async () => { await setup('COPY-CASE-SELECTED', true); await page.keyboard.press('Control+c'); @@ -163,4 +217,20 @@ describe('terminal Ctrl+C smart copy', () => { expect(res.data.join('')).toContain('SENTINEL'); // pasted text, not ^V expect(res.data.join('')).not.toContain('\x16'); }); + + it('forwards full-width punctuation after a Chinese IME composition', async () => { + await setup('IME-PUNCTUATION', false); + const desktopChunks = await captureImeInput(page); + expect(desktopChunks.join('')).toBe('中文' + IME_PUNCTUATION); + + const touchContext = await browser.newContext({ hasTouch: true }); + try { + const touchPage = await touchContext.newPage(); + await touchPage.goto(BASE_URL, { waitUntil: 'domcontentloaded' }); + const touchChunks = await captureImeInput(touchPage); + expect(touchChunks.join('')).toBe('中文' + IME_PUNCTUATION); + } finally { + await touchContext.close(); + } + }); });