From 5ea424565d5b374b6418f413844b535835bccbb7 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 12 Jul 2026 12:42:14 +0200 Subject: [PATCH] fix(review): content-free IME traces, guarded onData self-heal, Android-only retap recovery (PR #143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - BLOCKER (privacy): the CJK diagnostic trace logged typed CONTENT — _esc(e.key) per keystroke, up to 24 chars of textarea value on focus/blur/compstart/ compend/input, and the flushed text — mirrored into _crashDiag, which persists to localStorage and beacons to POST /api/crash-diag. Traces are now content-free: key CLASS via _kdesc (any single code point → 'printable', named keys pass through), value lengths + phantom presence via _vdesc (len=N[+ph]), and 'flush send len=N'. _esc removed. - MAJOR: the onData self-heal refocused the CJK field whenever gated data arrived with focus elsewhere — but onData also fires for xterm's SELF-GENERATED query replies (DA/DSR/CPR/OSC during Ink redraws), so it stole focus from rename/search/settings inputs while output streamed. Now requires document.activeElement === this.terminal.textarea (genuine typed input) and bails when shouldSuppressTerminalQueryResponse(data) matches. - MAJOR: the pointerdown blur→setTimeout(focus,0) wedged-IME recovery ran on ALL platforms; on iOS tapping the focused empty field is normal and the async refocus is outside the user-gesture stack. The listener is now only registered when /Android/i.test(navigator.userAgent). - tests: trace-privacy test (no typed character or textarea value ever appears in the trace; lengths/key classes still recorded), iOS harness asserts the pointerdown recovery never cycles, self-heal source guard asserts both new conditions; vm harness gained a ua option (navigator injected, Android UA default so the existing wedged-IME test still exercises the recovery). Co-Authored-By: Claude Fable 5 --- src/web/public/input-cjk.js | 57 ++++++++++++++++++++------------ src/web/public/terminal-ui.js | 11 ++++++- test/input-cjk.test.ts | 61 ++++++++++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/web/public/input-cjk.js b/src/web/public/input-cjk.js index fa5ab85e..7db12e6b 100644 --- a/src/web/public/input-cjk.js +++ b/src/web/public/input-cjk.js @@ -64,13 +64,23 @@ const CjkInput = (() => { // ── Diagnostic trace (intermittent CJK-loss investigation) ── // In-memory ring buffer of every IME event + flush decision. Mirrored into - // the crash-diag breadcrumbs (app.js), which beacon to the server every 2s — - // after a repro, `GET /api/crash-diag` shows the exact event sequence. + // the crash-diag breadcrumbs (app.js), which persist to localStorage and + // beacon to the server every 2s — after a repro, `GET /api/crash-diag` + // shows the exact event sequence. + // PRIVACY: because the trace leaves the page, it must stay CONTENT-FREE — + // event types, booleans, key classes, and value LENGTHS only. Never log a + // typed character or the textarea value (pasted secrets would be captured). const TRACE_MAX = 200; const _trace = []; - function _esc(v) { - const s = String(v == null ? '' : v).replace(/​/g, '∅'); - return JSON.stringify(s.length > 24 ? s.slice(0, 24) + '…' + s.length : s); + /** Content-free value descriptor: real-text length + phantom presence. */ + function _vdesc(v) { + const s = String(v == null ? '' : v); + return `len=${_strip(s).length}${s.includes(PHANTOM) ? '+ph' : ''}`; + } + /** Content-free key descriptor: named keys (Enter, Process…) pass through; any single code point is typed content. */ + function _kdesc(key) { + const k = String(key == null ? '' : key); + return [...k].length === 1 ? 'printable' : k; } function _t(msg) { _trace.push(`${Date.now() % 1000000} ${msg}`); @@ -146,7 +156,7 @@ const CjkInput = (() => { return; } const val = _strip(_textarea.value); - _t(`flush ${val ? 'send ' + _esc(val) : 'empty'}`); + _t(`flush ${val ? 'send len=' + val.length : 'empty'}`); if (val) { _send(val); } @@ -197,27 +207,33 @@ const CjkInput = (() => { _listeners.mousedown = (e) => { e.stopPropagation(); }; - // ── Wedged-IME recovery (Android) ── + // ── Wedged-IME recovery (Android ONLY) ── // Some Android IMEs (esp. 9-key Sogou/Xiaomi/Baidu) can wedge their // InputConnection: the keyboard composes in its own candidate bar but // delivers ZERO DOM events to the focused textarea. JS cannot detect // this (nothing fires) — but re-tapping the already-focused empty field // is the user's natural "it's stuck" gesture. A blur→focus cycle forces // the browser to restart the IME input session, which un-wedges it. - _listeners.pointerdown = () => { - if (document.activeElement === _textarea && !_composing && _isEffectivelyEmpty()) { - _t('ime-reset (retap)'); - _textarea.blur(); - setTimeout(() => _textarea.focus(), 0); - } - }; + // iOS is excluded: tapping the focused empty field there is normal + // (paste callout, habitual tap), and the setTimeout refocus runs outside + // the user-gesture stack, so the cycle would just misbehave. + if (/Android/i.test(navigator.userAgent)) { + _listeners.pointerdown = () => { + if (document.activeElement === _textarea && !_composing && _isEffectivelyEmpty()) { + _t('ime-reset (retap)'); + _textarea.blur(); + setTimeout(() => _textarea.focus(), 0); + } + }; + _textarea.addEventListener('pointerdown', _listeners.pointerdown); + } _listeners.focus = () => { - _t(`focus val=${_esc(_textarea.value)}`); + _t(`focus ${_vdesc(_textarea.value)}`); window.cjkActive = true; if (!_textarea.value) _resetToPhantom(); }; _listeners.blur = () => { - _t(`blur composing=${_composing} val=${_esc(_textarea.value)}`); + _t(`blur composing=${_composing} ${_vdesc(_textarea.value)}`); // 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. @@ -230,20 +246,19 @@ const CjkInput = (() => { _composing = false; }; _textarea.addEventListener('mousedown', _listeners.mousedown); - _textarea.addEventListener('pointerdown', _listeners.pointerdown); _textarea.addEventListener('focus', _listeners.focus); _textarea.addEventListener('blur', _listeners.blur); // ── Composition tracking (keyboard IME — works for CJK typing) ── _listeners.compositionstart = () => { - _t(`compstart val=${_esc(_textarea.value)}`); + _t(`compstart ${_vdesc(_textarea.value)}`); _composing = true; _cancelDebouncedFlush(); // Leave textarea.value untouched — programmatic changes during // compositionstart cancel the IME composition on iOS Safari. }; _listeners.compositionend = () => { - _t(`compend val=${_esc(_textarea.value)}`); + _t(`compend ${_vdesc(_textarea.value)}`); _composing = false; _cancelDebouncedFlush(); // Defer flush: some Android IMEs haven't committed text to textarea @@ -261,7 +276,7 @@ const CjkInput = (() => { // ── Keydown: special keys work REGARDLESS of composition state ── _listeners.keydown = (e) => { - _t(`keydown ${_esc(e.key)} kc=${e.keyCode} ic=${e.isComposing} c=${_composing}`); + _t(`keydown ${_kdesc(e.key)} kc=${e.keyCode} ic=${e.isComposing} c=${_composing}`); if (e.key === 'Enter') { e.preventDefault(); _composing = false; @@ -327,7 +342,7 @@ const CjkInput = (() => { // ── Input event: primary path for virtual keyboards + dictation ── _listeners.input = (e) => { - _t(`input ${e.inputType || '?'} ic=${e.isComposing} c=${_composing} val=${_esc(_textarea.value)}`); + _t(`input ${e.inputType || '?'} ic=${e.isComposing} c=${_composing} ${_vdesc(_textarea.value)}`); // ── Stuck-composition recovery ── // Some IMEs (WeChat/Sogou keyboards) fire compositionstart without a // matching compositionend. A stale _composing=true blocks every flush diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 421fcf5d..6c81eb6f 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -652,8 +652,17 @@ Object.assign(CodemanApp.prototype, { // typed lands HERE and is swallowed — keyboard shows the IME composing // while both the CJK field and the terminal stay empty. Route focus // back so the very next keystroke lands in the CJK field again. + // Only GENUINE typed input qualifies: onData also fires for xterm's + // self-generated query replies (DA/DSR/CPR/OSC during Ink redraws), + // which arrive no matter what has focus — so require focus to be on + // xterm's own textarea and bail on query replies, or this would steal + // focus from the rename/search/settings inputs while output streams. const cjkEl = document.getElementById('cjkInput'); - if (cjkEl?.classList.contains('cjk-input-visible') && document.activeElement !== cjkEl) { + if ( + cjkEl?.classList.contains('cjk-input-visible') && + document.activeElement === this.terminal.textarea && + !window.CodemanTerminalInput?.shouldSuppressTerminalQueryResponse(data) + ) { _crashDiag.log('CJK regain-focus (onData swallowed input)'); cjkEl.focus(); } diff --git a/test/input-cjk.test.ts b/test/input-cjk.test.ts index 9405ae9c..54b250b7 100644 --- a/test/input-cjk.test.ts +++ b/test/input-cjk.test.ts @@ -60,7 +60,10 @@ function makeTextarea(): FakeTextarea { }; } -function loadCjkHarness() { +const ANDROID_UA = 'Mozilla/5.0 (Linux; Android 14; Pixel 8) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0'; +const IOS_UA = 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_5 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko)'; + +function loadCjkHarness({ ua = ANDROID_UA }: { ua?: string } = {}) { const textarea = makeTextarea(); const sent: string[] = []; const windowObj: Record = {}; @@ -74,6 +77,7 @@ function loadCjkHarness() { const context = vm.createContext({ window: windowObj, document: documentObj, + navigator: { userAgent: ua }, setTimeout: (fn: () => void, ms?: number) => setTimeout(fn, ms), clearTimeout: (t: ReturnType) => clearTimeout(t), performance: { now: () => performance.now() }, @@ -203,6 +207,52 @@ describe('CJK input module', () => { expect(blurred).toBe(1); }); + it('does not run the wedged-IME pointerdown recovery on iOS (Android-only)', () => { + // On iOS, tapping the focused empty field is normal (paste callout, + // habitual tap), and the async refocus runs outside the user-gesture + // stack — the blur→focus cycle must never fire there. + const { textarea, documentObj } = loadCjkHarness({ ua: IOS_UA }); + documentObj.activeElement = textarea; + let blurred = 0; + let focused = 0; + textarea.blur = () => { + blurred++; + }; + textarea.focus = () => { + focused++; + }; + + textarea.fire('pointerdown'); + vi.advanceTimersByTime(10); + expect(blurred).toBe(0); + expect(focused).toBe(0); + }); + + it('never records typed content in the diagnostic trace (privacy)', () => { + // The trace mirrors into crash-diag, which persists to localStorage and + // beacons to the server — it must stay content-free: lengths, key + // classes, and event names only. Drive every path that formerly embedded + // the value or key literal (composition, keydown send, input, blur). + const { CjkInput, textarea, sent } = loadCjkHarness(); + + textarea.fire('compositionstart'); + textarea.value = PHANTOM + '秘密口令'; + textarea.fire('input', { isComposing: true, inputType: 'insertCompositionText' }); + textarea.fire('compositionend'); + vi.advanceTimersByTime(10); + + textarea.fire('keydown', { key: '囍', ctrlKey: false, altKey: false, metaKey: false }); + textarea.value = PHANTOM + '秘密'; + textarea.fire('blur'); + expect(sent).toEqual(['秘密口令', '囍']); + + const trace = CjkInput.getTrace().join('\n'); + for (const ch of '秘密口令囍') expect(trace).not.toContain(ch); + // Lengths and key classes are still traced for diagnostics. + expect(trace).toContain('flush send len=4'); + expect(trace).toContain('keydown printable'); + }); + it('skips redundant textarea writes when already reset (Android IME desync guard)', () => { const { CjkInput, textarea } = loadCjkHarness(); const before = textarea.valueWrites; @@ -227,6 +277,15 @@ describe('CJK input module', () => { expect(src).toMatch(/this\.terminal\.focus = \(\) => \{/); expect(src).toContain("cjkEl?.classList.contains('cjk-input-visible')"); expect(src).toContain('CJK regain-focus (onData swallowed input)'); + + // The self-heal must only fire for GENUINE typed input: onData also fires + // for xterm's self-generated query replies (DA/DSR/CPR/OSC during Ink + // redraws), which arrive while e.g. the rename input or search box has + // focus — refocusing on those steals focus mid-typing. Guard: focus must + // be on xterm's own textarea AND the data must not be a query reply. + const selfHeal = src.slice(src.indexOf('Self-heal'), src.indexOf('CJK regain-focus')); + expect(selfHeal).toContain('document.activeElement === this.terminal.textarea'); + expect(selfHeal).toContain('shouldSuppressTerminalQueryResponse(data)'); }); it('sends text plus carriage return on Enter', () => {