From 2d96472dbe5f19e90bfd7db3075969fcdcb268cd Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sat, 26 Sep 2026 22:47:48 -0400 Subject: [PATCH] fix(terminal): address review of the iOS IME preview - Observe keydown in the capture phase on terminal.element, an ancestor of the helper textarea, so the controller sees it before xterm's own capture listener finalizes the composition and emits the commit through onData. Finalize on exactly the keys CompositionHelper.keydown does (every keyCode except 20/229/16/17/18), ignoring isComposing and key as xterm does. - Bound awaitingCommit with the same 2 s fallback as the committed phase, so a composition whose commit never reaches onData cannot turn the next unrelated keystroke or paste into an IME commit. - pagehide resets the controller instead of destroying it, so a back-forward cache restore keeps the preview working. - Give the preview an opaque background from the terminal theme. - Route an IME commit through the ordinary printable/paste local echo branch and complete the commit afterwards; drop the send-on-throw fallback. - Pin the event order with an xterm stand-in registered in the capture phase ahead of the controller, and against real xterm in a browser test. - CLAUDE.md: note the IME commit routing and the z-index 6 preview layer. --- CLAUDE.md | 4 +- config/test-suites.ts | 1 + src/web/public/mobile-ime-preview.js | 93 ++++++--- src/web/public/terminal-ui.js | 54 +++--- test/mobile-ime-preview-structure.test.ts | 67 ++++++- test/mobile-ime-preview.browser.test.ts | 128 +++++++++++++ test/mobile-ime-preview.test.ts | 220 +++++++++++++++++++--- 7 files changed, 479 insertions(+), 88 deletions(-) create mode 100644 test/mobile-ime-preview.browser.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 6c8a8dc9..02cde906 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -312,7 +312,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph ### Frontend -Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. Load order: `constants.js`(1) → `i18n.js`(1.5) → `mobile-handlers.js`(2) → `voice-input.js`(3) → `notification-manager.js`(4) → `keyboard-accessory.js`(5) → `input-cjk.js`(5.5) → `mobile-ime-preview.js`(5.52) → `terminal-keycode229-recovery.js`(5.55) → `sanitize-html.js`(5.6) → `app.js`(6) → `tab-rail-resize.js`(6.5) → `terminal-ui.js`(7) → `terminal-split.js`(7.5) → `respawn-ui.js`(8) → `ralph-panel.js`(9) → `orchestrator-panel.js`(9.5) → `cron-ui.js`(9.7) → `settings-ui.js`(10) → `panels-ui.js`(11) → `readmymind-ui.js`(11.3) → `ultracode-panel.js`(11.5) → `approvals-ui.js`(11.6) → `reboot-restore-ui.js`(11.65) → `admin-ui.js`(11.7) → `session-ui.js`(12) → `host-wake-ui.js`(12.2) → `webview-tabs.js`(12.5) → `mobile-overview.js`(12.55) → `home-sessions.js`(12.56) → `entrance-animations.js`(12.6) → `ralph-wizard.js`(13) → `api-client.js`(14) → `subagent-windows.js`(15) → `ultracode-windows.js`(15.5) → `session-lineage.js`(15.6) → `image-input.js`(16). `i18n.js` translates static + newly inserted application DOM while skipping terminal/response/file/user-name surfaces; `input-cjk.js` handles CJK IME composition via an always-visible textarea below the terminal (`window.cjkActive` blocks xterm's onData). `terminal-keycode229-recovery.js` forwards a committed `input` event that xterm's `_inputEvent` guard drops (Chrome-on-Android soft keyboards send `composed: true` after a keydown), and only when xterm emitted no canonical data for that keystroke. ⚠️ **That decision is settled at the NEXT keydown as well as on its own zero-delay timer** (#441): the drain runs from xterm's custom key handler, which fires BEFORE xterm processes that key, so a soft keyboard that commits the last character and sends Enter in one InputConnection transaction puts the character on the wire ahead of the `\r`. On the timer alone that character is not merely late, it is LOST: xterm emits the `\r` first and bumps the canonical counter past the candidate's snapshot, so the candidate stands down (measured, `hell\r` where the user typed `hello`). The trade is that a keydown decides with less evidence than the timer did, since xterm's own keyCode-229 rescue has not run yet; that is safe for Enter, which clears the textarea so the pending diff emits nothing. Ordering is pinned by `test/terminal-keycode229-recovery.browser.test.ts`, which the CI gate does NOT run. +Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. Load order: `constants.js`(1) → `i18n.js`(1.5) → `mobile-handlers.js`(2) → `voice-input.js`(3) → `notification-manager.js`(4) → `keyboard-accessory.js`(5) → `input-cjk.js`(5.5) → `mobile-ime-preview.js`(5.52) → `terminal-keycode229-recovery.js`(5.55) → `sanitize-html.js`(5.6) → `app.js`(6) → `tab-rail-resize.js`(6.5) → `terminal-ui.js`(7) → `terminal-split.js`(7.5) → `respawn-ui.js`(8) → `ralph-panel.js`(9) → `orchestrator-panel.js`(9.5) → `cron-ui.js`(9.7) → `settings-ui.js`(10) → `panels-ui.js`(11) → `readmymind-ui.js`(11.3) → `ultracode-panel.js`(11.5) → `approvals-ui.js`(11.6) → `reboot-restore-ui.js`(11.65) → `admin-ui.js`(11.7) → `session-ui.js`(12) → `host-wake-ui.js`(12.2) → `webview-tabs.js`(12.5) → `mobile-overview.js`(12.55) → `home-sessions.js`(12.56) → `entrance-animations.js`(12.6) → `ralph-wizard.js`(13) → `api-client.js`(14) → `subagent-windows.js`(15) → `ultracode-windows.js`(15.5) → `session-lineage.js`(15.6) → `image-input.js`(16). `i18n.js` translates static + newly inserted application DOM while skipping terminal/response/file/user-name surfaces; `input-cjk.js` handles CJK IME composition via an always-visible textarea below the terminal (`window.cjkActive` blocks xterm's onData). `terminal-keycode229-recovery.js` forwards a committed `input` event that xterm's `_inputEvent` guard drops (Chrome-on-Android soft keyboards send `composed: true` after a keydown), and only when xterm emitted no canonical data for that keystroke. ⚠️ **That decision is settled at the NEXT keydown as well as on its own zero-delay timer** (#441): the drain runs from xterm's custom key handler, which fires BEFORE xterm processes that key, so a soft keyboard that commits the last character and sends Enter in one InputConnection transaction puts the character on the wire ahead of the `\r`. On the timer alone that character is not merely late, it is LOST: xterm emits the `\r` first and bumps the canonical counter past the candidate's snapshot, so the candidate stands down (measured, `hell\r` where the user typed `hello`). The trade is that a keydown decides with less evidence than the timer did, since xterm's own keyCode-229 rescue has not run yet; that is safe for Enter, which clears the textarea so the pending diff emits nothing. Ordering is pinned by `test/terminal-keycode229-recovery.browser.test.ts`, which the CI gate does NOT run. `mobile-ime-preview.js` (iOS WebKit only) paints the text an IME is composing: an iOS IME commit is routed into the local-echo overlay through the ordinary printable/paste branch and then `_transferMobileImeCommitToLocalEcho`, and without local echo the preview clears only on output parsed AFTER the commit (or its 2 s fallback). ⚠️ It watches keydown in the capture phase on `terminal.element`, never on the textarea, because xterm finalizes the composition and emits the commit in its own capture listener on the textarea. **Entrance animations** (`entrance-animations.js`, all OFF by default): opt-in animations for tabs, terminal, windows and connection lines, chosen via `data-tab-anim` / `data-term-anim` / `data-win-anim` / `data-line-anim` on ``; the default `legacy` theme short-circuits every hook. ⚠️ Tabs and lines are destroyed mid-animation on re-render, so re-apply to the fresh element by id with a negative `animation-delay` (resume, never restart). ⚠️ Terminal-pane styles may animate only transform / opacity / clip-path (anything else resizes the PTY via FitAddon); `blur` is the ONE sanctioned `filter` exception, do not generalise it. ⚠️ Line glow lives in `--line-glow` so blur keyframes interpolate. Persisted per-device in `codeman:*Anim` localStorage keys, never in `SettingsUpdateSchema`; lab at `?animlab=1`. Test: `test/entrance-animations.test.ts`. → [architecture-invariants#entrance-animations](docs/architecture-invariants.md#entrance-animations) @@ -372,7 +372,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **SSE staleness watchdog** (`computeSseStale()` in constants.js, `_checkSseStale()` + a 5s interval in app.js): an `EventSource` can stop delivering without erroring, so the client forces a reconnect when nothing arrives. ⚠️ The server keepalive must stay the named `sse:heartbeat` event (`cleanupDeadClients()`, sse-stream-manager.ts), never an SSE comment, which `EventSource` cannot observe; its no-op client listener must stay registered. ⚠️ Judge staleness only while `connected` and online (the loop breaker). ⚠️ The liveness stamp lives inside `addListener`. ⚠️ Clear the interval only at the top of `connectSSE()`, or intervals stack. → [architecture-invariants#sse-staleness-watchdog](docs/architecture-invariants.md#sse-staleness-watchdog) -**Z-index layers** (keep new overlays consistent with this stack): local echo overlay (7), terminal touch-selection bar (900, below floating agent windows), subagent windows + split picker menu (1000), plan agents (1100), mobile/tablet fixed header (1200), modals on ≤768px (1300, must beat the fixed header), log viewers (2000), connection-loss overlay (2500), image popups (3000), response viewer (5000, backdrop 4999), file-preview overlay (5100, must outrank the response viewer that launches it), toasts/path picker (10000+), custom-model center-status banner (10001; its `[hidden]` must re-assert `display: none` or `dismiss()` leaves an invisible click-blocker), custom-model swap-confirm/context-warning modals (10010). → [architecture-invariants#z-index-layers](docs/architecture-invariants.md#z-index-layers) +**Z-index layers** (keep new overlays consistent with this stack): iOS IME composition preview (6, inside `.xterm-helpers`, just under the local echo overlay), local echo overlay (7), terminal touch-selection bar (900, below floating agent windows), subagent windows + split picker menu (1000), plan agents (1100), mobile/tablet fixed header (1200), modals on ≤768px (1300, must beat the fixed header), log viewers (2000), connection-loss overlay (2500), image popups (3000), response viewer (5000, backdrop 4999), file-preview overlay (5100, must outrank the response viewer that launches it), toasts/path picker (10000+), custom-model center-status banner (10001; its `[hidden]` must re-assert `display: none` or `dismiss()` leaves an invisible click-blocker), custom-model swap-confirm/context-warning modals (10010). → [architecture-invariants#z-index-layers](docs/architecture-invariants.md#z-index-layers) **Respawn presets**: `solo-work` (3s/60min), `subagent-workflow` (45s/240min), `team-lead` (90s/480min), `ralph-todo` (8s/480min), `overnight-autonomous` (10s/480min). diff --git a/config/test-suites.ts b/config/test-suites.ts index 9f3ed295..0d1eac00 100644 --- a/config/test-suites.ts +++ b/config/test-suites.ts @@ -33,6 +33,7 @@ export const BROWSER_TEST_GLOBS = [ 'test/split-pane-terminal.browser.test.ts', 'test/split-pane-orchestration.browser.test.ts', 'test/split-pane-auto-collapse.browser.test.ts', + 'test/mobile-ime-preview.browser.test.ts', ]; /** diff --git a/src/web/public/mobile-ime-preview.js b/src/web/public/mobile-ime-preview.js index c18ece58..42826c8a 100644 --- a/src/web/public/mobile-ime-preview.js +++ b/src/web/public/mobile-ime-preview.js @@ -10,7 +10,18 @@ * onData, the caller hands it to `consumeTerminalData()`, which switches the * preview to `phase: 'committed'` until something else shows the text: the * local echo overlay or a prediction (`completeCommit`), authoritative - * terminal output (`noteAuthoritativeOutput`), or a 2 s fallback timer. + * terminal output (`noteAuthoritativeOutput`), or a 2 s fallback timer. The + * same 2 s bound applies while waiting for a commit that never reaches onData + * (the user deleted the whole composition), so a later unrelated chunk is never + * mistaken for it. + * + * Keydown ordering: xterm registers its textarea keydown listener in the + * capture phase inside terminal.open() and finalizes the composition there + * (CompositionHelper.keydown), emitting the commit through onData + * synchronously. The controller therefore observes keydown in the capture + * phase on an ANCESTOR (`keydownTarget`, the terminal element), which runs + * before any listener on the textarea itself, and finalizes on exactly the + * keys xterm does. * * VISUAL ONLY: the controller never sends, consumes or reorders input bytes, * and every callback is wrapped so a failing render cannot block the wire. @@ -25,6 +36,10 @@ const COMMITTED_VISUAL_TTL = 2000; const PREVIEW_CAP = 2048; + // keyCodes on which xterm 6's CompositionHelper.keydown keeps composing + // (CapsLock, the IME "composition character", Shift/Ctrl/Alt). Any other + // keydown during a composition finalizes it. + const KEEP_COMPOSING_KEYCODES = new Set([20, 229, 16, 17, 18]); const CONTROL_OR_LINE_BREAK = /[\u0000-\u001f\u007f-\u009f\u2028\u2029]/; function isIosWebKitTouch(nav = navigator) { @@ -38,6 +53,9 @@ function create(options) { const textarea = options.textarea; + // Must be the textarea or an ancestor of it, so its capture listener runs + // before xterm's capture listener on the textarea. + const keydownTarget = options.keydownTarget || textarea; const render = typeof options.render === 'function' ? options.render : function () {}; const clear = typeof options.clear === 'function' ? options.clear : function () {}; const onCommit = typeof options.onCommit === 'function' ? options.onCommit : function () {}; @@ -135,6 +153,25 @@ updateComposition({ data: event.data == null ? textarea.value : event.data }); } + function armFallbackTimer(isCurrent) { + const token = { generation, id: undefined }; + timerToken = token; + const callback = function () { + if (destroyed || timerToken !== token || token.generation !== generation || !isCurrent()) return; + timerToken = null; + cleanup(); + }; + const id = safely(setTimer, callback, COMMITTED_VISUAL_TTL); + if (timerToken === token) { + if (id === undefined) { + timerToken = null; + return false; + } + token.id = id; + } + return true; + } + function finalizeComposition(value, fromKeydown) { if (!composing) return; composing = false; @@ -143,6 +180,13 @@ finalizedByKeydown = fromKeydown; latestValue = value == null ? latestValue : String(value); scheduleLatestPreview('provisional'); + // A commit that never reaches onData (the composition was deleted, so + // xterm emits nothing) must not leave the controller waiting forever. + cancelCommittedTimer(); + const owner = generation; + armFallbackTimer(function () { + return awaitingCommit && generation === owner; + }); } function onCompositionEnd(event) { @@ -153,10 +197,15 @@ finalizeComposition(event.data, false); } + // Mirrors CompositionHelper.keydown in @xterm/xterm 6.0.0 + // (src/browser/input/CompositionHelper.ts:94-108): while composing, keyCode + // 20/229 and 16/17/18 keep the composition open and every other keyCode + // finalizes it. `isComposing` and `key` are deliberately not consulted, + // because xterm does not consult them. function onKeydown(event) { - if (composing && event.isComposing === false && event.key !== 'Process' && event.key !== 'Unidentified') { - finalizeComposition(latestValue, true); - } + if (keydownTarget !== textarea && event.target !== textarea) return; + if (!composing || KEEP_COMPOSING_KEYCODES.has(event.keyCode)) return; + finalizeComposition(latestValue, true); } function reset() { @@ -190,22 +239,10 @@ scheduleLatestPreview('committed'); if (destroyed || generation !== owner || !committed) return true; - const token = { generation, id: undefined }; - timerToken = token; - const callback = function () { - if (destroyed || timerToken !== token || token.generation !== generation || !committed) return; - timerToken = null; - cleanup(); - }; - const id = safely(setTimer, callback, COMMITTED_VISUAL_TTL); - if (timerToken === token) { - if (id === undefined) { - timerToken = null; - if (!destroyed && generation === owner && committed) cleanup(); - } else { - token.id = id; - } - } + const armed = armFallbackTimer(function () { + return committed; + }); + if (!armed && !destroyed && generation === owner && committed) cleanup(); return true; } @@ -220,19 +257,19 @@ } const listeners = [ - ['compositionstart', beginComposition], - ['compositionupdate', updateComposition], - ['input', onComposingInput], - ['compositionend', onCompositionEnd], - ['keydown', onKeydown, true], - ['blur', reset], + [textarea, 'compositionstart', beginComposition], + [textarea, 'compositionupdate', updateComposition], + [textarea, 'input', onComposingInput], + [textarea, 'compositionend', onCompositionEnd], + [keydownTarget, 'keydown', onKeydown, true], + [textarea, 'blur', reset], ]; - for (const [type, listener, capture] of listeners) textarea.addEventListener(type, listener, capture); + for (const [target, type, listener, capture] of listeners) target.addEventListener(type, listener, capture); function destroy() { if (destroyed) return; destroyed = true; - for (const [type, listener, capture] of listeners) textarea.removeEventListener(type, listener, capture); + for (const [target, type, listener, capture] of listeners) target.removeEventListener(type, listener, capture); cleanup(); } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 9413522d..7942255a 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -319,15 +319,22 @@ Object.assign(CodemanApp.prototype, { const value = style?.[property] || compositionView.style?.[property]; if (value) preview.style[property] = value; } - let foreground = this.terminal?.options?.theme?.foreground; - if (!foreground) { + const theme = this.terminal?.options?.theme; + let foreground = theme?.foreground; + let background = theme?.background; + if (!foreground || !background) { try { - foreground = window.codemanCurrentXtermTheme?.()?.foreground; + const current = window.codemanCurrentXtermTheme?.(); + foreground = foreground || current?.foreground; + background = background || current?.background; } catch { // Theme lookup is best-effort; retain the safe terminal fallback. } } preview.style.color = foreground || '#e0e0e0'; + // Opaque, like xterm's own composition view, so the preview does not + // overprint whatever sits at the cursor (a dim composer placeholder). + preview.style.backgroundColor = background || '#0d0d0d'; } catch { // Typography matching is visual-only and must not block input. } @@ -348,6 +355,10 @@ Object.assign(CodemanApp.prototype, { }; const controller = MobileImePreview.create({ textarea, + // An ancestor of the textarea: its capture-phase keydown listener runs + // before xterm's capture listener on the textarea, which finalizes the + // composition and emits the commit synchronously. + keydownTarget: this.terminal.element, render: ({ text, phase }) => { try { syncPreviewTypography(); @@ -364,6 +375,9 @@ Object.assign(CodemanApp.prototype, { this._mobileImePreview = controller; this._mobileImePreviewSessionId = this.activeSessionId; + // Offline and pagehide only reset: initTerminal() runs once per page + // load, so destroying on pagehide would leave the preview off for good + // after a back-forward cache restore (iOS Safari keeps pages there). this._mobileImePreviewOfflineHandler = () => { try { this._mobileImePreview?.reset?.(); @@ -371,7 +385,7 @@ Object.assign(CodemanApp.prototype, { // Disconnect cleanup is visual-only. } }; - this._mobileImePreviewPagehideHandler = () => this._destroyMobileImePreview(); + this._mobileImePreviewPagehideHandler = this._mobileImePreviewOfflineHandler; window.addEventListener('offline', this._mobileImePreviewOfflineHandler); window.addEventListener('pagehide', this._mobileImePreviewPagehideHandler); } catch { @@ -414,23 +428,18 @@ Object.assign(CodemanApp.prototype, { } }, - /** Hand a committed IME chunk to the local echo overlay. False = not taken. */ - _transferMobileImeCommitToLocalEcho(data) { - const overlay = this._localEchoOverlay; - try { - const update = data.length === 1 ? overlay?.addChar : overlay?.appendText; - if (typeof update !== 'function') return false; - update.call(overlay, data); - } catch { - return false; - } + /** + * The local echo overlay has just taken a committed IME chunk through the + * ordinary printable/paste branch, so it now shows the text: release the + * preview instead of waiting for terminal output. + */ + _transferMobileImeCommitToLocalEcho() { this._mobileImeCommitOutputSeq = null; try { this._mobileImePreview?.completeCommit?.({ predicted: true }); } catch { // Ownership transfer is visual-only. } - return true; }, initTerminal() { @@ -1474,16 +1483,9 @@ Object.assign(CodemanApp.prototype, { // When enabled, keystrokes are buffered locally in the overlay for // instant visual feedback. Nothing is sent to the PTY until Enter // (or a control char) is pressed — avoids out-of-order char delivery. - // An IME commit moves into the overlay, which then shows it in place of - // the preview. The charCode check skips a commit the one-shot Ctrl above - // turned into a control byte. If the overlay cannot take it, the text is - // sent directly rather than dropped. - let imeCommitBypassesEcho = false; - if (isImeCommit && this._localEchoEnabled && !echoPassthrough && data.charCodeAt(0) >= 32) { - if (this._transferMobileImeCommitToLocalEcho(data)) return; - imeCommitBypassesEcho = true; - } - if (this._localEchoEnabled && !echoPassthrough && !imeCommitBypassesEcho) { + // An IME commit takes the same printable/paste branch as typed text, + // and the overlay then shows it in place of the preview. + if (this._localEchoEnabled && !echoPassthrough) { if (data === '\x7f') { const source = this._localEchoOverlay?.removeChar(); if (source === 'flushed') { @@ -1539,6 +1541,7 @@ Object.assign(CodemanApp.prototype, { if (data.length > 1 && data.charCodeAt(0) >= 32) { // Paste: append to overlay only (sent on Enter) this._localEchoOverlay?.appendText(data); + if (isImeCommit && this._localEchoOverlay) this._transferMobileImeCommitToLocalEcho(); return; } if (data.charCodeAt(0) < 32) { @@ -1684,6 +1687,7 @@ Object.assign(CodemanApp.prototype, { if (data.length === 1 && data.charCodeAt(0) >= 32) { // Printable char: add to overlay only (sent on Enter) this._localEchoOverlay?.addChar(data); + if (isImeCommit && this._localEchoOverlay) this._transferMobileImeCommitToLocalEcho(); return; } } diff --git a/test/mobile-ime-preview-structure.test.ts b/test/mobile-ime-preview-structure.test.ts index 5daa5c5c..d673fa67 100644 --- a/test/mobile-ime-preview-structure.test.ts +++ b/test/mobile-ime-preview-structure.test.ts @@ -72,6 +72,7 @@ function createPreviewHarness( createThrows?: boolean; omitGlobal?: boolean; themeForeground?: string; + themeBackground?: string; themeGetterThrows?: boolean; } = {} ) { @@ -140,12 +141,17 @@ function createPreviewHarness( }); windowStub.codemanCurrentXtermTheme = () => { if (options.themeGetterThrows) throw new Error('theme unavailable'); - return { foreground: '#334455' }; + return { foreground: '#334455', background: '#223344' }; }; const app: App = Object.assign(Object.create(mixin), { terminal: { textarea: {}, - options: { theme: options.themeForeground ? { foreground: options.themeForeground } : undefined }, + options: { + theme: + options.themeForeground || options.themeBackground + ? { foreground: options.themeForeground, background: options.themeBackground } + : undefined, + }, element: { querySelector: (selector: string) => (selector === '.xterm-helpers' ? helpers : null) }, }, activeSessionId: 'session-a', @@ -202,6 +208,9 @@ describe('mobile IME preview lifecycle', () => { app._initMobileImePreview(); expect(mobileImePreview?.create).toHaveBeenCalledOnce(); expect(mobileImePreview?.create.mock.calls[0][0].textarea).toBe(app.terminal.textarea); + // Keydown is observed on the terminal element (an ancestor of the + // textarea), so it runs before xterm's own capture listener finalizes. + expect(mobileImePreview?.create.mock.calls[0][0].keydownTarget).toBe(app.terminal.element); expect(helpers.children).toEqual([previewNodes[0]]); expect(previewNodes[0]).toMatchObject({ className: 'codeman-ime-preview', hidden: true }); expect(previewNodes[0].attributes['aria-hidden']).toBe('true'); @@ -233,6 +242,21 @@ describe('mobile IME preview lifecycle', () => { expect(windowStub.removeEventListener).toHaveBeenCalledTimes(2); }); + it('pagehide resets the controller instead of destroying it, so a bfcache restore keeps the preview', () => { + const { app, createdControllers, previewNodes, windowStub } = createPreviewHarness(); + app._initMobileImePreview(); + const pagehide = (windowStub.addEventListener as Fn).mock.calls.find((call) => call[0] === 'pagehide')?.[1]; + expect(pagehide).toBeTypeOf('function'); + pagehide(); + expect(createdControllers[0].reset).toHaveBeenCalledOnce(); + expect(createdControllers[0].destroy).not.toHaveBeenCalled(); + expect(previewNodes[0].remove).not.toHaveBeenCalled(); + expect(app._mobileImePreview).toBe(createdControllers[0]); + // A second hide after the page came back from the cache still works. + pagehide(); + expect(createdControllers[0].reset).toHaveBeenCalledTimes(2); + }); + it('resets the controller exactly once when the active session changes', () => { const { app, createdControllers } = createPreviewHarness(); app._initMobileImePreview(); @@ -256,9 +280,10 @@ describe('mobile IME preview lifecycle', () => { expect(helpers.classList.contains('codeman-ime-preview-owned')).toBe(false); }); - it('uses the terminal foreground while mirroring native composition font metrics', () => { + it('uses the terminal foreground and opaque background while mirroring native composition font metrics', () => { const { app, compositionView, previewNodes, createdControllers } = createPreviewHarness({ themeForeground: '#1f2328', + themeBackground: '#fafafa', }); app._initMobileImePreview(); createdControllers[0].callbacks.render({ text: '入力', phase: 'provisional' }); @@ -270,15 +295,24 @@ describe('mobile IME preview lifecycle', () => { lineHeight: compositionView.style.lineHeight, height: compositionView.style.height, color: '#1f2328', + backgroundColor: '#fafafa', }); }); + it('falls back to the current skin theme when the terminal options carry no theme', () => { + const { app, previewNodes, createdControllers } = createPreviewHarness(); + app._initMobileImePreview(); + createdControllers[0].callbacks.render({ text: '入力', phase: 'provisional' }); + expect(previewNodes[0].style).toMatchObject({ color: '#334455', backgroundColor: '#223344' }); + }); + it('keeps rendering with a safe foreground when the theme getter throws', () => { const { app, previewNodes, createdControllers } = createPreviewHarness({ themeGetterThrows: true }); app._initMobileImePreview(); expect(() => createdControllers[0].callbacks.render({ text: '安全', phase: 'provisional' })).not.toThrow(); expect(previewNodes[0].textContent).toBe('安全'); expect(previewNodes[0].style.color).toBe('#e0e0e0'); + expect(previewNodes[0].style.backgroundColor).toBe('#0d0d0d'); }); it.each(['query', 'create', 'append', 'className', 'hidden'] as const)( @@ -464,17 +498,34 @@ describe('mobile IME commit onData routing', () => { }); it.each([ - ['the overlay is missing', { overlayMissing: true }, '日本'], ['appendText throws', { appendThrows: true }, '失敗'], ['addChar throws', { addThrows: true }, '字'], - ])('sends the committed text exactly once when %s', (_label, extra, text) => { + ])('never sends a commit the overlay may already hold when %s', (_label, extra, text) => { + // One code path with typed text: no send-on-throw fallback, which would + // double-send if the overlay threw after appending. const { controller, sent, handle } = onDataApp({ localEcho: true, ...extra }); - expect(() => handle(text)).not.toThrow(); - expect(sent).toEqual([text]); - // Nothing else shows the text yet, so the preview keeps it. + expect(() => handle(text)).toThrow('overlay failed'); + expect(sent).toEqual([]); + // Nothing shows the text, so the preview keeps it until its fallback. expect(controller.completeCommit).not.toHaveBeenCalled(); }); + it('keeps the preview when the overlay is missing, exactly like typed text', () => { + const { controller, sent, handle } = onDataApp({ localEcho: true, overlayMissing: true }); + expect(() => handle('日本')).not.toThrow(); + expect(sent).toEqual([]); + expect(controller.completeCommit).not.toHaveBeenCalled(); + }); + + it('completes the commit only after the printable branch has put it in the overlay', () => { + const { controller, overlay, handle } = onDataApp({ localEcho: true }); + controller.completeCommit.mockImplementation(() => { + expect(overlay.pendingText).toBe('界'); + }); + handle('界'); + expect(controller.completeCommit).toHaveBeenCalledOnce(); + }); + it('keeps an untagged paste on the existing local echo path', () => { const { controller, overlay, sent, handle } = onDataApp({ localEcho: true, tagged: false }); handle('plain paste'); diff --git a/test/mobile-ime-preview.browser.test.ts b/test/mobile-ime-preview.browser.test.ts new file mode 100644 index 00000000..504282d1 --- /dev/null +++ b/test/mobile-ime-preview.browser.test.ts @@ -0,0 +1,128 @@ +/** + * The iOS IME preview controller against a REAL xterm 6 instance. + * + * The controller's logic is unit-tested in test/mobile-ime-preview.test.ts + * with a stand-in for xterm. What only real xterm proves is the event ORDER: + * `terminal.open()` registers xterm's keydown listener in the capture phase on + * the helper textarea, and CompositionHelper.keydown finalizes a composition + * there and emits the commit through onData synchronously. The controller must + * observe that keydown first (capture phase on `terminal.element`), and must + * finalize on exactly the keys xterm does. + * + * No server: a blank page loads the vendored xterm bundle and the controller. + * Browser-driven, so it is excluded from `npm run test:ci` like the other + * Playwright suites. Run locally: + * npm run test:browser -- test/mobile-ime-preview.browser.test.ts + */ + +import { resolve } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { chromium, type Browser, type Page } from 'playwright'; + +const root = resolve(import.meta.dirname, '..'); + +type Step = + | ['start'] + | ['update', string] + | ['end', string] + | ['key', number, string, boolean] + | ['wait', number] + | ['consume', string]; + +describe('mobile IME preview with real xterm', () => { + let browser: Browser; + let page: Page; + + beforeAll(async () => { + browser = await chromium.launch({ headless: true }); + page = await browser.newPage(); + await page.setContent('
'); + await page.addScriptTag({ path: resolve(root, 'node_modules/@xterm/xterm/lib/xterm.js') }); + await page.addScriptTag({ path: resolve(root, 'src/web/public/mobile-ime-preview.js') }); + }, 60000); + + afterAll(async () => { + if (browser) await browser.close(); + }); + + async function drive(steps: Step[]) { + return page.evaluate(async (steps: Step[]) => { + const w = window as any; + const host = document.getElementById('t') as HTMLElement; + host.innerHTML = ''; + const term = new w.Terminal(); + term.open(host); + const textarea = term.textarea as HTMLTextAreaElement; + const renders: Array<{ text: string; phase: string }> = []; + const onData: Array<{ data: string; consumed: boolean }> = []; + const controller = w.MobileImePreview.create({ + textarea, + keydownTarget: term.element, + render: (r: { text: string; phase: string }) => renders.push(r), + clear: () => {}, + }); + term.onData((data: string) => onData.push({ data, consumed: controller.consumeTerminalData(data) })); + textarea.focus(); + const tick = (ms: number) => new Promise((r) => setTimeout(r, ms)); + for (const step of steps) { + if (step[0] === 'start') textarea.dispatchEvent(new CompositionEvent('compositionstart', { data: '' })); + if (step[0] === 'update') { + textarea.value = step[1]; + textarea.dispatchEvent(new CompositionEvent('compositionupdate', { data: step[1] })); + } + if (step[0] === 'end') textarea.dispatchEvent(new CompositionEvent('compositionend', { data: step[1] })); + if (step[0] === 'key') { + const [, keyCode, key, isComposing] = step; + const event = new KeyboardEvent('keydown', { key, isComposing, bubbles: true, cancelable: true }); + Object.defineProperty(event, 'keyCode', { get: () => keyCode }); + textarea.dispatchEvent(event); + } + if (step[0] === 'wait') await tick(step[1]); + if (step[0] === 'consume') onData.push({ data: step[1], consumed: controller.consumeTerminalData(step[1]) }); + } + await tick(20); + const { composing, awaitingCommit, committed, latest } = controller.state; + const result = { onData, state: { composing, awaitingCommit, committed, latest }, lastRender: renders.at(-1) }; + controller.destroy(); + term.dispose(); + return result; + }, steps); + } + + it('Enter mid-composition: xterm emits the commit and the controller takes it as committed', async () => { + // compositionupdate's textarea end offset is recorded by xterm on a 0 ms timer. + const result = await drive([['start'], ['update', '確定'], ['wait', 10], ['key', 13, 'Enter', false]]); + expect(result.onData).toEqual([ + { data: '確定', consumed: true }, + { data: '\r', consumed: false }, + ]); + expect(result.state).toMatchObject({ awaitingCommit: false, committed: true, latest: '確定' }); + expect(result.lastRender).toEqual({ text: '確定', phase: 'committed' }); + }); + + it('keyCode 229 with isComposing false: xterm keeps composing, so the preview keeps following', async () => { + const result = await drive([ + ['start'], + ['update', 'か'], + ['wait', 10], + ['key', 229, 'k', false], + ['update', 'かな'], + ]); + expect(result.onData).toEqual([]); + expect(result.state).toMatchObject({ composing: true, awaitingCommit: false, latest: 'かな' }); + expect(result.lastRender).toEqual({ text: 'かな', phase: 'provisional' }); + }); + + it('a deleted composition stops waiting after 2 s, so a later paste is not taken as its commit', async () => { + const result = await drive([ + ['start'], + ['update', 'abc'], + ['update', ''], + ['end', ''], + ['wait', 2100], + ['consume', 'pasted'], + ]); + expect(result.onData).toEqual([{ data: 'pasted', consumed: false }]); + expect(result.state).toMatchObject({ awaitingCommit: false, committed: false }); + }); +}); diff --git a/test/mobile-ime-preview.test.ts b/test/mobile-ime-preview.test.ts index 8b52b35a..cfdec9a3 100644 --- a/test/mobile-ime-preview.test.ts +++ b/test/mobile-ime-preview.test.ts @@ -1,15 +1,21 @@ import { readFileSync } from 'node:fs'; import vm from 'node:vm'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; type Listener = (event: Record) => void; type ListenerOptions = boolean | { capture?: boolean }; type RegisteredListener = { listener: Listener; capture: boolean }; -class FakeTextarea { - value = 'unchanged'; - private listeners = new Map(); +/** + * A DOM node with just enough event dispatch to reproduce listener ORDER: an + * ancestor's capture listeners, then the target's capture listeners, then the + * target's bubble listeners (at-target capture-first, as in Chromium 89+ and + * WebKit), then the ancestor's bubble listeners. + */ +class FakeNode { + parent: FakeNode | null = null; + protected listeners = new Map(); addEventListener(type: string, listener: Listener, options?: ListenerOptions) { const listeners = this.listeners.get(type) ?? []; @@ -26,31 +32,46 @@ class FakeTextarea { if (index >= 0) listeners.splice(index, 1); } - dispatch(type: string, event: Record = {}) { - const listeners = [...(this.listeners.get(type) ?? [])]; - for (const phase of [true, false]) { - for (const registered of listeners) { - if (registered.capture === phase) registered.listener({ type, ...event }); - } + run(type: string, capture: boolean, event: Record) { + for (const registered of [...(this.listeners.get(type) ?? [])]) { + if (registered.capture === capture) registered.listener(event); } } + dispatch(type: string, event: Record = {}) { + const full = { type, target: this, ...event }; + const ancestors: FakeNode[] = []; + for (let node = this.parent; node; node = node.parent) ancestors.unshift(node); + for (const ancestor of ancestors) ancestor.run(type, true, full); + this.run(type, true, full); + this.run(type, false, full); + for (const ancestor of [...ancestors].reverse()) ancestor.run(type, false, full); + } + listenerCount() { return [...this.listeners.values()].reduce((total, listeners) => total + listeners.length, 0); } } +class FakeTextarea extends FakeNode { + value = 'unchanged'; +} + type Scheduled = { id: number; callback: () => void; delay?: number }; function harness( overrides: Record = {}, - beforeCreate?: (textarea: FakeTextarea, getController: () => Record | undefined) => void + beforeCreate?: (textarea: FakeTextarea, getController: () => Record | undefined) => void ) { const source = readFileSync(new URL('../src/web/public/mobile-ime-preview.js', import.meta.url), 'utf8'); const context = vm.createContext({ navigator: {} }); vm.runInContext(source, context, { filename: 'mobile-ime-preview.js' }); const api = vm.runInContext('MobileImePreview', context); + // The terminal element: an ancestor of the helper textarea, like xterm's + // `.xterm` root is of `.xterm-helper-textarea`. + const element = new FakeNode(); const textarea = new FakeTextarea(); + textarea.parent = element; const frames: Scheduled[] = []; const timers: Scheduled[] = []; let nextId = 1; @@ -79,6 +100,7 @@ function harness( beforeCreate?.(textarea, () => controller); controller = api.create({ textarea, + keydownTarget: element, render, clear, onCommit, @@ -93,6 +115,7 @@ function harness( return { api, + element, textarea, frames, timers, @@ -222,34 +245,177 @@ describe('MobileImePreview', () => { const h = harness(); h.textarea.dispatch('compositionstart'); h.textarea.dispatch('compositionupdate', { data: '確定' }); - h.textarea.dispatch('keydown', { key: 'Enter', isComposing: false }); + h.textarea.dispatch('keydown', { key: 'Enter', keyCode: 13, isComposing: false }); expect(h.controller.consumeTerminalData('確定')).toBe(true); h.textarea.dispatch('compositionend', { data: 'stale' }); expect(h.controller.consumeTerminalData('stale')).toBe(false); }); - test('capture keydown finalization precedes an earlier xterm bubble onData listener', () => { - const consumed: boolean[] = []; + /** + * Stand-in for xterm 6.0: `terminal.open()` registers a CAPTURE keydown + * listener on the helper textarea (CoreBrowserTerminal.ts:379), and + * CompositionHelper.keydown (CompositionHelper.ts:94-108) finalizes the + * composition there, emitting the commit through onData synchronously, for + * every keyCode except 20/229 and 16/17/18. It is registered BEFORE the + * controller is created, exactly as terminal.open() precedes + * _initMobileImePreview(). + */ + function withXtermStandIn() { + const emitted: Array<{ data: string; consumed: boolean }> = []; + let composing = false; + let composition = ''; const h = harness({}, (textarea, getController) => { - textarea.addEventListener('keydown', () => { - const controller = getController() as { consumeTerminalData(data: string): boolean }; - consumed.push(controller.consumeTerminalData('確定')); + const emit = (data: string) => emitted.push({ data, consumed: getController()?.consumeTerminalData(data) }); + textarea.addEventListener('compositionstart', () => { + composing = true; + composition = ''; }); + textarea.addEventListener('compositionupdate', (event) => { + composition = String(event.data ?? ''); + }); + textarea.addEventListener( + 'keydown', + (event) => { + if (composing && ![20, 229, 16, 17, 18].includes(event.keyCode as number)) { + composing = false; + emit(composition); + } + if (event.keyCode === 13) emit('\r'); + }, + true + ); }); + return { ...h, emitted, isXtermComposing: () => composing }; + } + + test('Enter mid-composition hands the commit xterm emits in its capture keydown to the preview', () => { + const h = withXtermStandIn(); h.textarea.dispatch('compositionstart'); h.textarea.dispatch('compositionupdate', { data: '確定' }); - h.textarea.dispatch('keydown', { key: 'Enter', isComposing: false }); + h.flushFrame(); + h.textarea.dispatch('keydown', { key: 'Enter', keyCode: 13, isComposing: false }); - expect(consumed).toEqual([true]); - expect(h.controller.consumeTerminalData('確定')).toBe(false); - expect(h.onCommit).toHaveBeenCalledOnce(); + expect(h.emitted).toEqual([ + { data: '確定', consumed: true }, + { data: '\r', consumed: false }, + ]); + expect(h.onCommit).toHaveBeenCalledWith('確定'); + expect(h.controller.state).toMatchObject({ composing: false, awaitingCommit: false, committed: true }); + h.flushFrame(); + expect(h.render).toHaveBeenLastCalledWith({ text: '確定', phase: 'committed' }); + // The next unrelated keystroke is ordinary input, not an IME commit. + expect(h.controller.consumeTerminalData('x')).toBe(false); + }); + + test.each([ + // keyCode 229 with isComposing:false and a real key identity: xterm keeps + // composing, so the controller must too. + ['the IME composition character', 'k', 229], + ['CapsLock', 'CapsLock', 20], + ['Shift', 'Shift', 16], + ['Control', 'Control', 17], + ['Alt', 'Alt', 18], + ])('a keydown for %s keeps tracking the composition xterm is still composing', (_label, key, keyCode) => { + const h = withXtermStandIn(); + h.textarea.dispatch('compositionstart'); + h.textarea.dispatch('compositionupdate', { data: 'か' }); + h.textarea.dispatch('keydown', { key, keyCode, isComposing: false }); + expect(h.isXtermComposing()).toBe(true); + expect(h.controller.state).toMatchObject({ composing: true, awaitingCommit: false }); + + // The preview follows the composition instead of freezing on the old value. + h.textarea.dispatch('compositionupdate', { data: 'かな' }); + h.flushFrame(); + expect(h.render).toHaveBeenLastCalledWith({ text: 'かな', phase: 'provisional' }); + expect(h.emitted).toEqual([]); + }); + + test('ignores keydowns that did not target the helper textarea', () => { + const h = harness(); + const sibling = new FakeNode(); + sibling.parent = h.element; + h.textarea.dispatch('compositionstart'); + h.textarea.dispatch('compositionupdate', { data: '漢字' }); + sibling.dispatch('keydown', { key: 'Enter', keyCode: 13, isComposing: false }); + expect(h.controller.state).toMatchObject({ composing: true, awaitingCommit: false }); + }); + + test('destroy stops the controller observing keydown on the terminal element', () => { + const h = withXtermStandIn(); h.controller.destroy(); h.textarea.dispatch('compositionstart'); h.textarea.dispatch('compositionupdate', { data: 'later' }); - h.textarea.dispatch('keydown', { key: 'Enter', isComposing: false }); - expect(consumed).toEqual([true, false]); - expect(h.onCommit).toHaveBeenCalledOnce(); + h.textarea.dispatch('keydown', { key: 'Enter', keyCode: 13, isComposing: false }); + expect(h.emitted).toEqual([ + { data: 'later', consumed: false }, + { data: '\r', consumed: false }, + ]); + expect(h.onCommit).not.toHaveBeenCalled(); + expect(h.element.listenerCount()).toBe(0); + }); + + describe('a commit that never reaches onData', () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + function realTimerHarness() { + return harness({ + setTimer: (callback: () => void, delay: number) => setTimeout(callback, delay), + clearTimer: (id: ReturnType) => clearTimeout(id), + }); + } + + test.each([ + ['compositionend', (h: ReturnType) => h.textarea.dispatch('compositionend', { data: '' })], + [ + 'a finalizing keydown', + (h: ReturnType) => h.textarea.dispatch('keydown', { key: 'Enter', keyCode: 13 }), + ], + ])('stops waiting after the same 2 s bound when finalized by %s', (_label, finalize) => { + const h = realTimerHarness(); + h.textarea.dispatch('compositionstart'); + h.textarea.dispatch('compositionupdate', { data: 'deleted' }); + finalize(h); + h.clear.mockClear(); + expect(h.controller.state).toMatchObject({ awaitingCommit: true, timerPending: true }); + + vi.advanceTimersByTime(1999); + expect(h.controller.state.awaitingCommit).toBe(true); + vi.advanceTimersByTime(1); + expect(h.controller.state).toMatchObject({ awaitingCommit: false, latest: '', timerPending: false }); + expect(h.clear).toHaveBeenCalledOnce(); + + // The next unrelated keystroke or paste is not adopted as the IME commit. + expect(h.controller.consumeTerminalData('x')).toBe(false); + expect(h.controller.consumeTerminalData('pasted line')).toBe(false); + expect(h.onCommit).not.toHaveBeenCalled(); + }); + + test('a commit that arrives in time replaces the wait bound with the committed one', () => { + const h = realTimerHarness(); + h.textarea.dispatch('compositionstart'); + h.textarea.dispatch('compositionend', { data: '日本' }); + vi.advanceTimersByTime(1500); + expect(h.controller.consumeTerminalData('日本')).toBe(true); + // The wait bound would have fired at 2000 ms; the committed bound runs + // a full 2 s from the commit instead. + vi.advanceTimersByTime(1000); + expect(h.controller.state.committed).toBe(true); + vi.advanceTimersByTime(1000); + expect(h.controller.state.committed).toBe(false); + }); + + test('a new composition cancels the previous wait bound', () => { + const h = realTimerHarness(); + h.textarea.dispatch('compositionstart'); + h.textarea.dispatch('compositionend', { data: '' }); + vi.advanceTimersByTime(1500); + h.textarea.dispatch('compositionstart'); + h.textarea.dispatch('compositionupdate', { data: 'next' }); + vi.advanceTimersByTime(1000); + expect(h.controller.state).toMatchObject({ composing: true, latest: 'next' }); + }); }); test('generation fences stale frames and timers', () => { @@ -332,6 +498,7 @@ describe('MobileImePreview', () => { destroy.textarea.dispatch('compositionend'); expect(destroy.controller.consumeTerminalData('final')).toBe(true); expect(destroy.textarea.listenerCount()).toBe(0); + expect(destroy.element.listenerCount()).toBe(0); expect(destroy.frames).toHaveLength(0); expect(destroy.timers).toHaveLength(0); expect(destroy.controller.state.latest).toBe(''); @@ -353,6 +520,7 @@ describe('MobileImePreview', () => { clearController = clear.controller; expect(() => clear.textarea.dispatch('compositionstart')).not.toThrow(); expect(clear.textarea.listenerCount()).toBe(0); + expect(clear.element.listenerCount()).toBe(0); expect(clear.frames).toHaveLength(0); expect(clear.timers).toHaveLength(0); }); @@ -453,10 +621,12 @@ describe('MobileImePreview', () => { const h = harness(); h.textarea.dispatch('compositionstart'); h.textarea.dispatch('compositionupdate', { data: 'pending' }); - expect(h.textarea.listenerCount()).toBe(6); + expect(h.textarea.listenerCount()).toBe(5); + expect(h.element.listenerCount()).toBe(1); h.controller.destroy(); h.controller.destroy(); expect(h.textarea.listenerCount()).toBe(0); + expect(h.element.listenerCount()).toBe(0); expect(h.frames).toHaveLength(0); h.textarea.dispatch('compositionupdate', { data: 'ignored' }); expect(h.scheduleFrame).toHaveBeenCalledOnce();