Merge pull request #327 from dignfei/fix/terminal-ime-punctuation

fix(terminal): preserve IME punctuation input

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;
     },

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<char> 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

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(['秘密口令', '囍']);

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();
+    }
+  });
 });