diff --git a/.changeset/fn-7561-mobile-terminal-spacing.md b/.changeset/fn-7561-mobile-terminal-spacing.md new file mode 100644 index 0000000000..020bb8c7c0 --- /dev/null +++ b/.changeset/fn-7561-mobile-terminal-spacing.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix mobile terminal text still rendering with excess inter-character gaps after font-load settle. +category: fix +dev: Root cause: xterm's OptionsService setter is a no-op when reassigning an already-current fontFamily/fontSize, so post-settle reapply never forced CharSizeService/DomRenderer to remeasure. Added `forceTerminalFontRemeasure()` in `terminalPreferences.ts`, used by both `TerminalModal.tsx` and `SessionTerminal.tsx` at every post-`waitForTerminalFontMetrics()` settle site. diff --git a/docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md b/docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md new file mode 100644 index 0000000000..dbf4337082 --- /dev/null +++ b/docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md @@ -0,0 +1,120 @@ +--- +title: "xterm OptionsService no-op reassignment silently skips post-load remeasure" +date: 2026-07-04 +category: ui-bugs +module: packages/dashboard/app/components/TerminalModal +problem_type: ui_bug +component: frontend_terminal +applies_when: "Code reapplies an xterm.js Terminal option (fontFamily, fontSize, etc.) to a value that may already equal the terminal's current option value, expecting that reassignment to force an internal recompute (character measurement, renderer dimensions, letter-spacing compensation)." +symptoms: + - "Mobile terminal text still renders with excessive inter-character spacing on the very first layout even after text-size-adjust is disabled and a document.fonts settle/remeasure step was already added" + - "The spacing only 'repairs itself' after an unrelated event: toggling the virtual keyboard, rotating the device, reconnecting the session, or manually changing the font size and changing it back" + - "Existing --keyboard-overlap/--vv-height/--vv-width/text-size-adjust: none assertions and a mocked resize(80, 24) all pass while the real-device symptom persists" +root_cause: xterm_optionsservice_setter_is_a_strict_noop_on_identical_values_so_reassigning_the_same_resolved_font_after_an_async_settle_never_fires_onoptionchange_and_never_forces_charsizeservice_domrenderer_remeasure +resolution_type: code_fix +severity: high +related_components: + - packages/dashboard/app/components/TerminalModal.tsx + - packages/dashboard/app/components/SessionTerminal.tsx + - packages/dashboard/app/utils/terminalPreferences.ts + - packages/dashboard/app/components/__tests__/TerminalModal.test.tsx + - packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx + - packages/dashboard/app/utils/__tests__/terminalPreferences.test.ts + - FN-7456 + - FN-7460 + - FN-7561 +tags: + - xterm + - font-loading + - options-service + - mobile-safari + - remeasure +--- + +# xterm OptionsService no-op reassignment silently skips post-load remeasure + +## Problem + +FN-7561 is the third recurrence of "mobile terminal renders with excessive inter-character spacing" after this exact subsystem was touched twice before: + +- FN-7456 added the iOS keyboard/viewport baseline and a symbols-free measured font stack (see `xterm-symbols-nerd-font-unicode-range.md`). +- FN-7460 added `-webkit-text-size-adjust: none` / `text-size-adjust: none` on `.terminal-xterm, .terminal-xterm *` after a real iPhone Safari report showed spacing surviving FN-7456, plus 10px/12px coverage. + +Despite both fixes, the real-device symptom persisted. Both prior fixes treated the browser's DOM text-size-adjust/font-boosting behavior as the entire mechanism. It was not. + +### The actual mechanism + +xterm.js measures character/cell metrics via `CharSizeService`, then `DomRenderer._setDefaultSpacing()` bakes a compensating `letter-spacing` onto `.xterm-rows`: + +```ts +// @xterm/xterm src/browser/renderer/dom/DomRenderer.ts +private _setDefaultSpacing(): void { + // measure same char as in CharSizeService to get the base deviation + const spacing = this.dimensions.css.cell.width - this._widthCache.get('W', false, false); + this._rowContainer.style.letterSpacing = `${spacing}px`; + this._rowFactory.defaultSpacing = spacing; +} +``` + +This recompute only runs from `_handleOptionsChanged()` (wired to `optionsService.onOptionChange`) or from `handleCharSizeChanged()`. Both app terminal surfaces (`TerminalModal.tsx`, `SessionTerminal.tsx`) reapply xterm font options after `waitForTerminalFontMetrics()` (added by FN-7456) settles, expecting that reassignment to force this recompute against the font that only just finished loading. But real xterm's `OptionsService` setter is a strict no-op on an unchanged value: + +```ts +// @xterm/xterm src/common/services/OptionsService.ts +const setter = (propName: string, value: any): void => { + value = this._sanitizeAndValidateOption(propName, value); + // Don't fire an option change event if they didn't change + if (this.rawOptions[propName] !== value) { + this.rawOptions[propName] = value; + this._onOptionChange.fire(propName); + } +}; +``` + +In the common case (the user never touched terminal preferences), the resolved `fontFamily`/`fontSize` after settle are *identical* to what was already applied a few lines earlier at xterm construction/effect setup. Reassigning the same value is therefore a total no-op: no `onOptionChange` fires, `CharSizeService` never remeasures, and `DomRenderer._setDefaultSpacing()` never recomputes the letter-spacing compensation against the now-loaded web font. The stale pre-load cell metrics (measured against a fallback system font before the custom font finished loading) persist as visible excess gaps on the very first mobile layout — exactly matching the report that the terminal "only repairs itself after keyboard toggle/orientation/reconnect": those events happen to force a genuine value change elsewhere in the pipeline (e.g. `handleResize`/`handleDevicePixelRatioChange`), incidentally triggering the missing remeasure. + +Both `TerminalModal.tsx` and `SessionTerminal.tsx` had this bug in **two** places each: the initial xterm-init settle path and the live-preferences-apply settle path. + +## Why FN-7456/FN-7460 missed this + +Both fixes (and their regression tests) only ever asserted the *final* font/size value and CSS text-size-adjust state, never whether a genuine value *transition* occurred inside xterm's internal option pipeline. A plain mock `options: { fontSize: 14 }` object cannot model xterm's no-op-on-unchanged-value contract, so no test could distinguish "the code reassigned the resolved value" (looks correct) from "xterm's internal measurement pipeline actually recomputed" (the real requirement). + +## Solution + +Force a genuine (distinct-value) transition through xterm's option setter every time font metrics settle, regardless of whether the resolved value already equals the terminal's current option value: + +```ts +// packages/dashboard/app/utils/terminalPreferences.ts +const TERMINAL_FONT_REMEASURE_SENTINEL_FONT_FAMILY = "monospace"; + +export function forceTerminalFontRemeasure( + terminal: { options: { fontFamily?: string } }, + fontFamily: string, +): void { + const sentinel = + fontFamily === TERMINAL_FONT_REMEASURE_SENTINEL_FONT_FAMILY + ? `${TERMINAL_FONT_REMEASURE_SENTINEL_FONT_FAMILY}, monospace` + : TERMINAL_FONT_REMEASURE_SENTINEL_FONT_FAMILY; + terminal.options.fontFamily = sentinel; + terminal.options.fontFamily = fontFamily; +} +``` + +Both assignments run synchronously with no yield in between, so no intermediate frame paints — the terminal never visibly flashes the sentinel font. Both `TerminalModal.tsx` and `SessionTerminal.tsx` now call `forceTerminalFontRemeasure(terminal, resolvedFontFamily)` (instead of a plain `terminal.options.fontFamily = resolvedFontFamily`) at every post-settle site, immediately before reapplying `fontSize` and refitting/resizing/refreshing. + +Do not: + +- Add a hardcoded `letterSpacing`, fixed cell width, or fixed column count to mask the symptom. +- Skip the reassignment when the resolved value already matches the current option value — that equality is exactly what causes the bug. +- Remove or weaken the FN-7456/FN-7460 `text-size-adjust`/font-stack/keyboard-overlap coverage; this fix is additive to those invariants, not a replacement. + +## Regression coverage + +jsdom cannot exercise real xterm.js internals, so the regression coverage models xterm's documented no-op-on-unchanged-value contract directly on the test double, and asserts the *transition*, not just the final value: + +- Wrap the mocked `Terminal.options` object in a real getter/setter pair with the same equality check as `@xterm/xterm`'s `OptionsService` setter, and track a counter that only increments on a genuine (distinct-value) `fontFamily`/`fontSize` transition. +- Simulate the real recurrence: xterm opens before `document.fonts.load()`/`document.fonts.ready` resolve (deferred promises), the resolved font/size are already applied and unchanged once they settle. +- Assert the transition counter goes from 0 to a positive count once `waitForTerminalFontMetrics()` settles — this fails pre-fix (a plain reassignment to the same value is a no-op) and passes post-fix (`forceTerminalFontRemeasure` always forces at least one genuine transition). +- Add a focused unit test for `forceTerminalFontRemeasure()` itself in `terminalPreferences.test.ts`, covering both "resolved value unchanged" and "resolved value genuinely different" cases. +- Cover both `TerminalModal` (mobile viewport, keyboard-open and keyboard-closed initial render) and `SessionTerminal` (embedded attach surface) — both surfaces independently reapply font options after settle and both had the bug. +- Run: `pnpm --filter @fusion/dashboard exec vitest run app/components/__tests__/TerminalModal.test.tsx app/components/__tests__/SessionTerminal.test.tsx app/components/__tests__/SessionTerminal.mobile.test.tsx app/__tests__/terminal-input.test.ts app/utils/__tests__/terminalPreferences.test.ts --silent=passed-only --reporter=dot`. +- Real mobile Safari/Chrome verification remains the strongest signal for this class of bug; if unavailable, record that as an explicit gap rather than treating desktop WebKit/jsdom as proof (see `docs/ios-acceptance.md`). diff --git a/packages/dashboard/app/components/SessionTerminal.tsx b/packages/dashboard/app/components/SessionTerminal.tsx index 49d4068085..2acf5783c9 100644 --- a/packages/dashboard/app/components/SessionTerminal.tsx +++ b/packages/dashboard/app/components/SessionTerminal.tsx @@ -10,6 +10,7 @@ import { useMobileKeyboard } from "../hooks/useMobileKeyboard"; import { isMobileViewport, MOBILE_MEDIA_QUERY } from "../hooks/useViewportMode"; import { TERMINAL_PREFERENCES_KEY, + forceTerminalFontRemeasure, readTerminalPreferences, resolveTerminalFontFamily, resolveTerminalGlyphFontFamily, @@ -321,6 +322,9 @@ export function SessionTerminal({ FNXC:Terminal 2026-06-30-22:47: Async font-metric waits can resolve out of order when a second terminal preference change lands first. Reapply only if the currently mounted xterm options still match the preference snapshot that scheduled this wait, preserving the latest small-font cell metrics instead of resurrecting stale spacing. + + FNXC:Terminal 2026-07-04-09:40: + FN-7561 recurrence #3: when the snapshot already matches (preferences unchanged, the common case), reassigning `fontFamily` to the SAME value is a no-op against real xterm's OptionsService — CharSizeService/DomRenderer never remeasure the web font that only just finished loading. Use `forceTerminalFontRemeasure` so a genuine value transition always occurs on settle. */ void waitForTerminalFontMetrics(terminalPreferences.fontSize, resolvedFontFamily).then( (fontMetricsSettled) => { @@ -332,7 +336,7 @@ export function SessionTerminal({ ) { return; } - terminal.options.fontFamily = resolvedFontFamily; + forceTerminalFontRemeasure(terminal, resolvedFontFamily); terminal.options.fontSize = terminalPreferences.fontSize; try { (fitAddonRef.current as { fit?: () => void } | null)?.fit?.(); @@ -500,8 +504,11 @@ export function SessionTerminal({ /* FNXC:Terminal 2026-06-18-07:15: SessionTerminal shares TerminalModal's real-iOS DOM/canvas measurement path and the same user-selectable font presets. FN-6638 ruled out stack ordering with the 66.76px-identical diagnostic, so this attach surface must also reapply font options and refit after best-effort FontFaceSet settlement even when iOS rejects the multi-family shorthand; WebGL desktop remains safe because the same invalidation path refreshes renderer metrics without changing renderer selection. + + FNXC:Terminal 2026-07-04-09:40: + FN-7561 recurrence #3: reassigning fontFamily to the SAME already-resolved value is a no-op against real xterm's OptionsService (no onOptionChange fires), so CharSizeService/DomRenderer never remeasure the web font that only just finished loading after xterm's initial pre-load measurement. Force a genuine value transition via `forceTerminalFontRemeasure`. */ - term.options.fontFamily = resolvedFontFamily; + forceTerminalFontRemeasure(term, resolvedFontFamily); term.options.fontSize = terminalPreferences.fontSize; (fitAddon as unknown as { fit: () => void }).fit(); sendResize(term.cols, term.rows); diff --git a/packages/dashboard/app/components/TerminalModal.tsx b/packages/dashboard/app/components/TerminalModal.tsx index a9643efb1d..e6d3fb98b4 100644 --- a/packages/dashboard/app/components/TerminalModal.tsx +++ b/packages/dashboard/app/components/TerminalModal.tsx @@ -41,6 +41,7 @@ import { MIN_TERMINAL_FONT_SIZE, TERMINAL_FONT_FAMILY_PRESETS, clampTerminalFontSize, + forceTerminalFontRemeasure, readTerminalPreferences, resolveTerminalFontFamily, resolveTerminalGlyphFontFamily, @@ -1250,8 +1251,11 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG /* FNXC:Terminal 2026-06-18-07:23: FN-6638 recurrence #4 showed the previous symbols-last stack-order fix was inert: the supplied diagnostic measured AGENTS.md at the same 66.76px for symbols-first, symbols-last, and system-mono stacks while real iOS Safari still widened ASCII cells. xterm measures cell geometry at open() time, so after best-effort FontFaceSet settlement we must always reapply the active preset's font options, fit, resize, and refresh; that invalidates stale DOM/canvas metrics on real iOS when the full shorthand is rejected and keeps desktop WebGL using the same renderer-neutral metric refresh. + + FNXC:Terminal 2026-07-04-09:35: + FN-7561 recurrence #3: reassigning `fontFamily` to the SAME already-resolved value (the common case, since preferences are unchanged) is a no-op against real xterm's OptionsService — no `onOptionChange` fires, so CharSizeService/DomRenderer never remeasure the web font that only just finished loading. Force a genuine value transition via `forceTerminalFontRemeasure` so the character/cell metrics and `_setDefaultSpacing()` letter-spacing compensation are recomputed against the settled font on every settle, not just when the preference itself changed. */ - terminal.options.fontFamily = resolvedFontFamilyRef.current; + forceTerminalFontRemeasure(terminal, resolvedFontFamilyRef.current); terminal.options.fontSize = fontSizeRef.current; fitAddon.fit(); resizeRef.current?.(terminal.cols, terminal.rows); @@ -1776,6 +1780,9 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG /* FNXC:Terminal 2026-06-30-13:18: The mobile screenshot recurrence happens at the visible 10px setting with the soft keyboard already open. A live font-size preference change must wait for the symbols-free measured stack to settle, then reapply xterm font options, refit, resize, and refresh; otherwise canvas/DOM metrics can keep the old wider cells until an unfold/orientation event forces a later measurement. + + FNXC:Terminal 2026-07-04-09:35: + FN-7561 recurrence #3: the two equality checks below only guard against a STALE out-of-order settle (a newer preference change landed first); when the values already match the current snapshot (the common initial-load case: preferences did not actually change) a plain reassignment is a no-op against real xterm's OptionsService, so CharSizeService/DomRenderer never remeasure the font that just finished loading. Use `forceTerminalFontRemeasure` so a genuine value transition always occurs on settle, regardless of whether the resolved value already equals the terminal's current option value. */ void waitForTerminalFontMetrics(terminalPreferences.fontSize, resolvedFontFamily).then( (fontMetricsSettled) => { @@ -1788,7 +1795,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG ) { return; } - xtermRef.current.options.fontFamily = resolvedFontFamily; + forceTerminalFontRemeasure(xtermRef.current, resolvedFontFamily); xtermRef.current.options.fontSize = terminalPreferences.fontSize; scheduleRefit(); }, diff --git a/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx b/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx index 4aaa157941..616330e24e 100644 --- a/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx +++ b/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx @@ -4,6 +4,48 @@ import { act, render, screen, fireEvent, waitFor } from "@testing-library/react" // ── Mock xterm + addon dynamic imports (jsdom has no canvas/WebGL) ────────── const mockFitAddon = { fit: vi.fn() }; let sessionKeyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; + +/* +FNXC:Terminal 2026-07-04-09:45: +Real xterm.js's OptionsService setter is a strict no-op (no onOptionChange +fires, so CharSizeService/DomRenderer never remeasure) whenever a caller +reassigns an option to a value that already equals the option's current value. +The previous plain `{ ...options }` spread on construction could not model this +no-op-on-unchanged-value behavior, letting FN-7561's recurrence (reassigning +the SAME resolved font after an async web-font settle never forces a genuine +remeasure) go uncaught. Track a `fontRemeasureCount` that only increments on a +genuine (distinct-value) fontFamily/fontSize transition. +*/ +let fontRemeasureCount = 0; +function resetFontRemeasureCount(): void { + fontRemeasureCount = 0; +} +function getFontRemeasureCount(): number { + return fontRemeasureCount; +} +function wrapMockTerminalOptions(initial: Record): Record { + const store: Record = { ...initial }; + const options: Record = {}; + for (const key of Object.keys(store)) { + Object.defineProperty(options, key, { + enumerable: true, + configurable: true, + get(): unknown { + return store[key]; + }, + set(value: unknown): void { + if (store[key] !== value) { + store[key] = value; + if (key === "fontFamily" || key === "fontSize") { + fontRemeasureCount += 1; + } + } + }, + }); + } + return options; +} + const mockTerm = { loadAddon: vi.fn(), open: vi.fn(), @@ -21,7 +63,12 @@ const mockTerm = { cols: 80, rows: 24, }; -vi.mock("@xterm/xterm", () => ({ Terminal: vi.fn(function Terminal(options) { mockTerm.options = { ...options }; return mockTerm; }) })); +vi.mock("@xterm/xterm", () => ({ + Terminal: vi.fn(function Terminal(options) { + mockTerm.options = wrapMockTerminalOptions(options as Record); + return mockTerm; + }), +})); vi.mock("@xterm/addon-fit", () => ({ FitAddon: vi.fn(function FitAddon() { return mockFitAddon; }) })); vi.mock("@xterm/addon-unicode11", () => ({ Unicode11Addon: vi.fn(function Unicode11Addon() { return {}; }) })); vi.mock("@xterm/addon-webgl", () => ({ @@ -103,6 +150,7 @@ beforeEach(() => { mockTerm.refresh.mockClear(); mockTerm.dispose.mockClear(); mockTerm.options = {}; + resetFontRemeasureCount(); Object.defineProperty(document, "fonts", { value: undefined, configurable: true, @@ -314,6 +362,64 @@ describe("SessionTerminal", () => { }); }); + /* + FNXC:Terminal 2026-07-04-09:50: + FN-7561 recurrence #3 root cause: reassigning `terminal.options.fontFamily`/`fontSize` + to the SAME already-resolved value (the common case, since preferences are + unchanged) is a no-op against real xterm's OptionsService — no `onOptionChange` + fires, so CharSizeService/DomRenderer never remeasure the web font that only + just finished loading after xterm's initial pre-load measurement. This proves + SessionTerminal forces a genuine value transition on settle too, not just + TerminalModal. + */ + it("forces a genuine xterm character-metric remeasure after the mobile web font settles later than xterm's initial measurement", async () => { + let resolveLoad: (() => void) | undefined; + let resolveReady: (() => void) | undefined; + const load = vi.fn( + () => + new Promise((resolve) => { + resolveLoad = resolve; + }), + ); + Object.defineProperty(document, "fonts", { + value: { + load, + ready: new Promise((resolve) => { + resolveReady = resolve; + }), + }, + configurable: true, + }); + + render(); + + await waitFor(() => { + expect(FakeWS.instances.length).toBe(1); + expect(load).toHaveBeenCalled(); + }); + expectMeasurementSafeFontStack(mockTerm.options.fontFamily as string); + + // Isolate exactly what happens once the deferred font-load settles; the + // resolved fontFamily/fontSize never actually changed (the user never + // touched terminal preferences), so this must be a forced remeasure, not + // an incidental preference-driven one. + resetFontRemeasureCount(); + + await act(async () => { + resolveLoad?.(); + resolveReady?.(); + await Promise.resolve(); + await Promise.resolve(); + }); + + await waitFor(() => { + expect(getFontRemeasureCount()).toBeGreaterThan(0); + }); + + expectMeasurementSafeFontStack(mockTerm.options.fontFamily as string); + expect(mockTerm.options.fontFamily).toBe(resolveTerminalFontFamily("nerd-font")); + }); + it("applies validated terminal preferences at xterm init", async () => { const { Terminal } = await import("@xterm/xterm"); window.localStorage.setItem( diff --git a/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx b/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx index d913d1428c..321091101e 100644 --- a/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx @@ -65,6 +65,55 @@ const mockFitAddonFit = vi.fn(); let terminalKeyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; let terminalDataHandler: ((data: string) => void) | null = null; +/* +FNXC:Terminal 2026-07-04-09:15: +Real xterm.js's OptionsService setter is a strict no-op (no onOptionChange fires, +so CharSizeService/DomRenderer never remeasure) whenever a caller reassigns an +option to a value that already equals the option's current value — see +`@xterm/xterm` `common/services/OptionsService.ts` `setter()`. The plain object +literal previously used for `mockTerminalInstance.options` could not model this +no-op-on-unchanged-value behavior, which is why prior FN-7456/FN-7460 coverage +could pass while the real recurrence (reassigning the SAME resolved font after +an async web-font settle never forces a genuine remeasure) stayed uncaught. +Track a `fontRemeasureCount` that only increments on a genuine (distinct-value) +fontFamily/fontSize transition so tests can assert xterm's measurement pipeline +was actually forced to recompute, not merely reassigned to an identical value. +*/ +let fontRemeasureCount = 0; +function resetFontRemeasureCount(): void { + fontRemeasureCount = 0; +} +function getFontRemeasureCount(): number { + return fontRemeasureCount; +} +function createMockTerminalOptions(): Record { + const store: Record = { + fontSize: 14, + fontFamily: undefined, + cursorStyle: undefined, + cursorBlink: undefined, + }; + const options: Record = {}; + for (const key of Object.keys(store)) { + Object.defineProperty(options, key, { + enumerable: true, + configurable: true, + get(): unknown { + return store[key]; + }, + set(value: unknown): void { + if (store[key] !== value) { + store[key] = value; + if (key === "fontFamily" || key === "fontSize") { + fontRemeasureCount += 1; + } + } + }, + }); + } + return options; +} + const mockTerminalInstance = { loadAddon: vi.fn(), open: vi.fn(), @@ -83,7 +132,7 @@ const mockTerminalInstance = { clear: vi.fn(), focus: vi.fn(), refresh: vi.fn(), - options: { fontSize: 14 }, + options: createMockTerminalOptions(), cols: 80, rows: 24, }; @@ -236,6 +285,7 @@ describe("TerminalModal", () => { mockTerminalInstance.options.fontSize = 14; mockTerminalInstance.options.cursorStyle = "block"; mockTerminalInstance.options.cursorBlink = true; + resetFontRemeasureCount(); mockCreateTerminalSession.mockResolvedValue({ sessionId: "test-session-123", shell: "/bin/bash", @@ -6796,3 +6846,208 @@ describe("TerminalModal — project-context propagation (FN-1765)", () => { }); }); }); + +/* +FNXC:Terminal 2026-07-04-09:20: +FN-7561 root cause: after FN-7456/FN-7460 disabled `text-size-adjust` and waited +for `document.fonts.ready`, both the initial xterm-init settle path and the live +preferences-apply settle path reapply `terminal.options.fontFamily`/`fontSize` +by assigning the ALREADY-RESOLVED value back onto the option. Real xterm's +OptionsService setter is a no-op when the new value strictly equals the current +value (no `onOptionChange` fires), so CharSizeService's canvas/DOM character +measurement and DomRenderer's `_setDefaultSpacing()` letter-spacing compensation +are never actually recomputed against the web font that only finished loading +AFTER xterm's initial (pre-load, fallback-font) measurement. The stale +pre-load cell metrics + compensation persist as visible excess inter-character +gaps until an unrelated event (resize/orientation/DPR change) happens to force +a genuine value change. This suite proves the app now forces a genuine +value-changing remeasure every time font metrics settle, not just a same-value +reassignment. +*/ +describe("TerminalModal — FN-7561 mobile inter-character spacing (xterm no-op remeasure)", () => { + const mockOnClose = vi.fn(); + const mockSendInput = vi.fn(); + const mockResize = vi.fn(); + const mockReconnect = vi.fn(); + + const createMockTerminalState = (overrides = {}) => ({ + connectionStatus: "connected" as const, + sendInput: mockSendInput, + resize: mockResize, + onData: vi.fn(() => vi.fn()), + onExit: vi.fn(() => vi.fn()), + onConnect: vi.fn(() => vi.fn()), + onScrollback: vi.fn(() => vi.fn()), + reconnect: mockReconnect, + onSessionInvalid: vi.fn(() => vi.fn()), + ...overrides, + }); + + let previousInnerWidth: number; + let previousOntouchstart: unknown; + + beforeEach(() => { + vi.clearAllMocks(); + resetFontRemeasureCount(); + vi.spyOn(window, "matchMedia").mockImplementation((query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn(), + })); + previousInnerWidth = window.innerWidth; + previousOntouchstart = window.ontouchstart; + // Real reported device: a narrow touch-primary mobile viewport. + Object.defineProperty(window, "innerWidth", { value: 390, configurable: true }); + Object.defineProperty(window, "ontouchstart", { value: null, configurable: true }); + mockTerminalInstance.options.fontFamily = XTERM_FONT_FAMILY; + mockTerminalInstance.options.fontSize = 12; + mockTerminalInstance.options.cursorStyle = "block"; + mockTerminalInstance.options.cursorBlink = true; + resetFontRemeasureCount(); + mockUseTerminal.mockReturnValue(createMockTerminalState()); + mockUseTerminalSessions.mockReturnValue(defaultSessionState); + mockUseWorkspaces.mockReturnValue({ + projectName: "kb", + workspaces: [], + loading: false, + error: null, + }); + mockCreateTerminalSession.mockResolvedValue({ + sessionId: "test-session-123", + shell: "/bin/bash", + cwd: "/project", + }); + }); + + afterEach(() => { + Object.defineProperty(window, "innerWidth", { value: previousInnerWidth, configurable: true }); + if (previousOntouchstart === undefined) { + delete (window as unknown as { ontouchstart?: unknown }).ontouchstart; + } else { + Object.defineProperty(window, "ontouchstart", { value: previousOntouchstart, configurable: true }); + } + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("forces a genuine xterm character-metric remeasure after the mobile web font settles later than xterm's initial measurement", async () => { + // Model the font not being ready yet at xterm construction (the real + // recurrence: the custom web font loads asynchronously, AFTER xterm's + // initial fallback-font character measurement), then settling shortly + // after. `waitForTerminalFontMetrics` awaits exactly this `load`/`ready` + // pair before reapplying font options. + let resolveLoad: (() => void) | undefined; + let resolveReady: (() => void) | undefined; + const load = vi.fn( + () => + new Promise((resolve) => { + resolveLoad = resolve; + }), + ); + Object.defineProperty(document, "fonts", { + value: { + load, + ready: new Promise((resolve) => { + resolveReady = resolve; + }), + }, + configurable: true, + }); + + render(); + + await waitFor(() => expect(mockTerminalInstance.open).toHaveBeenCalled()); + expectMeasurementSafeFontStack(mockTerminalInstance.options.fontFamily as string); + + // Nothing else has changed the resolved font/size at this point, so any + // remeasure count observed so far merely reflects the initial synchronous + // application done during xterm construction/effect setup — reset it and + // isolate exactly what happens once the deferred font-load settles. + resetFontRemeasureCount(); + + await act(async () => { + resolveLoad?.(); + resolveReady?.(); + await Promise.resolve(); + await Promise.resolve(); + }); + + // The resolved fontFamily/fontSize the app wants after settle is identical + // to what was already applied before the font finished loading (the user + // never touched terminal preferences). A naive "reassign the resolved + // value" is therefore a no-op against real xterm's OptionsService + // (identical-value assignments never fire onOptionChange), so + // CharSizeService/DomRenderer would silently keep stale pre-load cell + // metrics forever. The fix must force at least one genuine (distinct + // value) fontFamily/fontSize transition here so xterm actually + // remeasures against the now-loaded font — this is the invariant + // FN-7456/FN-7460's `text-size-adjust`/`--keyboard-overlap`/`--vv-height` + // assertions never covered. + await waitFor(() => { + expect(getFontRemeasureCount()).toBeGreaterThan(0); + }); + + // The terminal must still land on the correct, symbols-free, resolved + // font after the forced remeasure settles. + expectMeasurementSafeFontStack(mockTerminalInstance.options.fontFamily as string); + expect(mockTerminalInstance.options.fontFamily).toBe(XTERM_FONT_FAMILY); + }); + + it("also forces the remeasure when the mobile keyboard is already open at initial render", async () => { + const mockVV = { + width: 375, + height: 300, + offsetTop: 0, + offsetLeft: 0, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + }; + Object.defineProperty(window, "visualViewport", { + value: mockVV, + writable: true, + configurable: true, + }); + Object.defineProperty(window, "innerHeight", { value: 300, writable: true, configurable: true }); + + let resolveLoad: (() => void) | undefined; + let resolveReady: (() => void) | undefined; + Object.defineProperty(document, "fonts", { + value: { + load: vi.fn( + () => + new Promise((resolve) => { + resolveLoad = resolve; + }), + ), + ready: new Promise((resolve) => { + resolveReady = resolve; + }), + }, + configurable: true, + }); + + render(); + + await waitFor(() => expect(mockTerminalInstance.open).toHaveBeenCalled()); + resetFontRemeasureCount(); + + await act(async () => { + resolveLoad?.(); + resolveReady?.(); + await Promise.resolve(); + await Promise.resolve(); + }); + + await waitFor(() => { + expect(getFontRemeasureCount()).toBeGreaterThan(0); + }); + expectMeasurementSafeFontStack(mockTerminalInstance.options.fontFamily as string); + + Object.defineProperty(window, "visualViewport", { value: undefined, writable: true, configurable: true }); + }); +}); diff --git a/packages/dashboard/app/utils/__tests__/terminalPreferences.test.ts b/packages/dashboard/app/utils/__tests__/terminalPreferences.test.ts index 31a0f83a05..70431c3b95 100644 --- a/packages/dashboard/app/utils/__tests__/terminalPreferences.test.ts +++ b/packages/dashboard/app/utils/__tests__/terminalPreferences.test.ts @@ -4,6 +4,7 @@ import { LEGACY_TERMINAL_FONT_SIZE_KEY, TERMINAL_PREFERENCES_KEY, XTERM_FONT_FAMILY, + forceTerminalFontRemeasure, readTerminalPreferences, waitForTerminalFontMetrics, writeTerminalPreferences, @@ -114,4 +115,58 @@ describe("terminalPreferences", () => { expect(load).not.toHaveBeenCalledWith("12px \"Fusion Terminal Nerd Font Symbols\""); expect(readyAwaited).toBe(true); }); + + /* + FNXC:Terminal 2026-07-04-09:55: + FN-7561 root cause: xterm's real OptionsService setter is a no-op when a + caller reassigns an option to its already-current value, so simply + reassigning the resolved fontFamily after a web font settles never forces + xterm's internal CharSizeService/DomRenderer remeasure. Model that exact + no-op-on-unchanged-value contract and assert `forceTerminalFontRemeasure` + always produces at least one genuine (distinct-value) transition, even when + the resolved value already equals the terminal's current option value. + */ + describe("forceTerminalFontRemeasure", () => { + function createXtermLikeOptions(initialFontFamily: string) { + let current = initialFontFamily; + let changeCount = 0; + const terminal = { + options: { + get fontFamily(): string { + return current; + }, + set fontFamily(value: string) { + if (value !== current) { + current = value; + changeCount += 1; + } + }, + }, + }; + return { terminal, getChangeCount: () => changeCount }; + } + + it("forces a genuine value transition even when the resolved value already matches the current option", () => { + const { terminal, getChangeCount } = createXtermLikeOptions(XTERM_FONT_FAMILY); + + // A naive reassignment to the identical value would be a no-op against + // real xterm and is what let FN-7561 recur; assert the baseline first. + terminal.options.fontFamily = XTERM_FONT_FAMILY; + expect(getChangeCount()).toBe(0); + + forceTerminalFontRemeasure(terminal, XTERM_FONT_FAMILY); + + expect(getChangeCount()).toBeGreaterThan(0); + expect(terminal.options.fontFamily).toBe(XTERM_FONT_FAMILY); + }); + + it("lands on a genuinely different resolved value too", () => { + const { terminal, getChangeCount } = createXtermLikeOptions(XTERM_FONT_FAMILY); + + forceTerminalFontRemeasure(terminal, "system-mono, monospace"); + + expect(getChangeCount()).toBeGreaterThan(0); + expect(terminal.options.fontFamily).toBe("system-mono, monospace"); + }); + }); }); diff --git a/packages/dashboard/app/utils/terminalPreferences.ts b/packages/dashboard/app/utils/terminalPreferences.ts index dc6c62b3f7..a97d5a2ba8 100644 --- a/packages/dashboard/app/utils/terminalPreferences.ts +++ b/packages/dashboard/app/utils/terminalPreferences.ts @@ -193,6 +193,44 @@ export async function waitForTerminalFontMetrics( return true; } +/* +FNXC:Terminal 2026-07-04-09:30: +FN-7561 (recurrence #3 of mobile inter-character spacing, after FN-7456's DOM +glyph-fallback fix and FN-7460's `text-size-adjust: none`) root cause: real +xterm.js's `OptionsService` setter is a strict no-op when a caller reassigns an +option to a value that already strictly-equals its current value (no +`onOptionChange` fires — see `@xterm/xterm` `common/services/OptionsService.ts`). +Both terminal surfaces reapply `terminal.options.fontFamily`/`fontSize` with the +SAME already-resolved value once `waitForTerminalFontMetrics()` settles, which is +the common case (the user never touched terminal preferences). That reassignment +never fires `onOptionChange`, so xterm's `CharSizeService` (canvas/DOM character +measurement) and `DomRenderer._setDefaultSpacing()` (the letter-spacing +compensation baked onto `.xterm-rows`) never recompute against the web font that +finished loading AFTER xterm's initial pre-load (fallback-font) measurement. The +stale pre-load cell metrics + compensation persist as visible excess +inter-character gaps on the very first mobile layout until an unrelated event +(resize/orientation/DPR change) happens to produce a genuine value change and +incidentally force a real remeasure — exactly the "only repairs itself after +keyboard toggle/orientation/reconnect" symptom reported for this recurrence. +Force a genuine (distinct-value) transition through a sentinel font family +before landing back on the resolved value so xterm's internal remeasure +pipeline always runs at least once against the now-settled font, regardless of +whether the resolved value already matches the terminal's current option value. +*/ +const TERMINAL_FONT_REMEASURE_SENTINEL_FONT_FAMILY = "monospace"; + +export function forceTerminalFontRemeasure( + terminal: { options: { fontFamily?: string } }, + fontFamily: string, +): void { + const sentinel = + fontFamily === TERMINAL_FONT_REMEASURE_SENTINEL_FONT_FAMILY + ? `${TERMINAL_FONT_REMEASURE_SENTINEL_FONT_FAMILY}, monospace` + : TERMINAL_FONT_REMEASURE_SENTINEL_FONT_FAMILY; + terminal.options.fontFamily = sentinel; + terminal.options.fontFamily = fontFamily; +} + function isObject(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); }