diff --git a/.changeset/fn-7567-mobile-terminal-spacing.md b/.changeset/fn-7567-mobile-terminal-spacing.md new file mode 100644 index 0000000000..1462cca8e4 --- /dev/null +++ b/.changeset/fn-7567-mobile-terminal-spacing.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix mobile terminal excess character spacing that survived earlier font-remeasure fixes. +category: fix +dev: `TerminalModal`/`SessionTerminal` re-bake xterm's `DomRenderer` letter-spacing compensation AFTER `fitAddon.fit()` settles the post-fit column count (not just before it), since `handleResize()` never re-bakes spacing itself. See `docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md` recurrence #4. 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 index dbf4337082..2ea9a036da 100644 --- 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 @@ -23,12 +23,15 @@ related_components: - FN-7456 - FN-7460 - FN-7561 + - FN-7567 tags: - xterm - font-loading - options-service - mobile-safari - remeasure + - letter-spacing + - domrenderer --- # xterm OptionsService no-op reassignment silently skips post-load remeasure @@ -118,3 +121,89 @@ jsdom cannot exercise real xterm.js internals, so the regression coverage models - 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`). + +## Recurrence #4 (FN-7567): forcing a genuine remeasure BEFORE `fit()` bakes stale spacing + +FN-7561's `forceTerminalFontRemeasure()` fix (above) is necessary but was not sufficient. On the real +mobile device, ordinary ASCII (`test`, `ls`, filenames) still rendered with visible gaps on the +initial paint even with the FN-7561 fix, FN-7460's `text-size-adjust: none`, and FN-7456's +symbols-free font stack all present. + +### New root cause + +Real xterm's `DomRenderer._setDefaultSpacing()` — the letter-spacing compensation baked onto +`.xterm-rows` as `spacing = dimensions.css.cell.width - widthCache.get('W')` — only recomputes from +two call sites: + +- `handleCharSizeChanged()`, wired to `CharSizeService.onCharSizeChange`, which fires on any + **genuine** (distinct-value) `fontFamily`/`fontSize` option transition — exactly what + `forceTerminalFontRemeasure()` forces. +- `handleDevicePixelRatioChange()`. + +It is **never** recomputed from `handleResize()` — the path `fitAddon.fit()` → +`terminal.resize(cols, rows)` takes. + +Both mobile settle sites in `TerminalModal.tsx` and `SessionTerminal.tsx` (the initial post-font-load +settle and the live-preferences-apply settle) called `forceTerminalFontRemeasure()` **before** +`fitAddon.fit()`. That correctly forces a genuine option transition and does bake letter-spacing — +but it bakes it against the column count that predates the fit. `fitAddon.fit()` then changes the +column count (and therefore the true cell width) but never re-bakes the spacing, so the terminal keeps +rendering with a spacing value computed against a column count that no longer matches reality. The +gap persists until an unrelated later event (device-pixel-ratio change, orientation, reconnect) +happens to force another genuine option/DPR-triggered remeasure — exactly matching the report that +the terminal "only repairs itself after an incidental refit." + +### Why FN-7456/FN-7460/FN-7561 missed this + +All three prior fixes and their regressions asserted CSS-property presence (`text-size-adjust: none`, +symbols-free `fontFamily`) or a remeasure **call count** (`fontRemeasureCount`), never the actual baked +letter-spacing value relative to the **post-fit** column count. A test that only checks "a remeasure +happened" cannot distinguish "remeasure happened but was baked against stale pre-fit geometry" from +"remeasure happened and reflects final geometry." + +### Solution + +Force a **second** genuine remeasure **after** `fitAddon.fit()` settles the column count, so the +letter-spacing bake is recomputed against the FINAL (post-fit) geometry, not the pre-fit one: + +```ts +// packages/dashboard/app/components/TerminalModal.tsx (mirrored in SessionTerminal.tsx) +forceTerminalFontRemeasure(terminal, resolvedFontFamilyRef.current); +terminal.options.fontSize = fontSizeRef.current; +fitAddon.fit(); +resizeRef.current?.(terminal.cols, terminal.rows); +forceTerminalFontRemeasure(terminal, resolvedFontFamilyRef.current); // re-bake against final cols +terminal.refresh(0, Math.max(0, terminal.rows - 1)); +``` + +The `scheduleRefit(rebakeSpacingAfterFit)` path in `TerminalModal.tsx` only re-bakes on the *settled* +(font-metrics-ready) call site, not on the immediate first frame — at that point the web font may not +have loaded yet, so re-baking there would only bake against fallback-font metrics again. + +Do not: + +- Re-bake unconditionally on every frame/resize — only after a settle that already forced a genuine + remeasure and then fit. +- Replace this with a hardcoded letter-spacing/cell-width compensation. + +### Regression coverage (geometry-based, not CSS/call-count) + +jsdom cannot exercise real xterm.js internals, so the FN-7567 regression models the real +`CharSizeService`/`DomRenderer` contracts directly on the test double instead of a plain mock: + +- `mockHandleCharSizeChanged()` mirrors `CharSizeService.measure()` → `DomRenderer.handleCharSizeChanged()` + → `_setDefaultSpacing()`: recomputes `bakedLetterSpacingPx = cellWidthPx - measuredCharWidthPx` using + the **current** (possibly stale, pre-fit) column count, firing only on a genuine option transition. +- `mockFitAddonFit()` mirrors `FitAddon.fit()` → `terminal.resize(cols, rows)` → + `DomRenderer.handleResize()`: recomputes `cols`/cell-width from the current measured char width but + deliberately does **not** touch the baked letter-spacing (matching real xterm). +- The assertion is the actual rendered geometry invariant: baked letter-spacing must equal `0` (cell + width matches the settled glyph advance width) after the full settle+fit sequence — not merely that + `forceTerminalFontRemeasure`/`fontRemeasureCount` was called. +- See `TerminalModal.test.tsx` describe block "FN-7567 mobile inter-character spacing (stale post-fit + letter-spacing bake)" and the mirrored `SessionTerminal.test.tsx` coverage. +- 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/utils/__tests__/terminalPreferences.test.ts app/__tests__/terminal-input.test.ts --silent=passed-only --reporter=dot`. +- Real mobile Safari/Chrome sanity check remains the strongest signal; a real-device screenshot was not + obtainable in this execution environment (headless coding agent, no physical device access) — this + gap is recorded explicitly rather than treating jsdom/desktop WebKit as proof. See task document + key="repro" on FN-7567 and `docs/ios-acceptance.md`. diff --git a/packages/dashboard/app/components/SessionTerminal.tsx b/packages/dashboard/app/components/SessionTerminal.tsx index 2acf5783c9..1b0d72cd2f 100644 --- a/packages/dashboard/app/components/SessionTerminal.tsx +++ b/packages/dashboard/app/components/SessionTerminal.tsx @@ -341,6 +341,15 @@ export function SessionTerminal({ try { (fitAddonRef.current as { fit?: () => void } | null)?.fit?.(); sendResizeMessage(terminal.cols, terminal.rows); + /* + FNXC:Terminal 2026-07-04-11:45: + FN-7567 recurrence #4: `_setDefaultSpacing()` only recomputes from a + genuine option-change remeasure or a devicePixelRatio change, never + from the `fit()`/resize above, so re-bake spacing once more here + against the settled (post-fit) column count instead of the stale + pre-fit one baked by the `forceTerminalFontRemeasure` call above. + */ + forceTerminalFontRemeasure(terminal, resolvedFontFamily); terminal.refresh(0, Math.max(0, terminal.rows - 1)); } catch { /* ignore teardown or transient measure failures */ @@ -507,11 +516,25 @@ export function SessionTerminal({ 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`. + + FNXC:Terminal 2026-07-04-11:45: + FN-7567 recurrence #4: the remeasure above is necessary but not + sufficient. Real xterm's `DomRenderer._setDefaultSpacing()` (the + letter-spacing compensation baked onto `.xterm-rows`) only recomputes + from a genuine option-change remeasure (what `forceTerminalFontRemeasure` + triggers) or a devicePixelRatio change — NEVER from `handleResize()`, + which is what `fitAddon.fit()` -> `terminal.resize(cols, rows)` + triggers. Baking spacing BEFORE `fit()` bakes it against the stale + pre-fit column count; force a second genuine remeasure AFTER `fit()` + settles the column count so spacing is re-baked against the FINAL + geometry, not the pre-fit one. See + `docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md`. */ forceTerminalFontRemeasure(term, resolvedFontFamily); term.options.fontSize = terminalPreferences.fontSize; (fitAddon as unknown as { fit: () => void }).fit(); sendResize(term.cols, term.rows); + forceTerminalFontRemeasure(term, resolvedFontFamily); term.refresh(0, Math.max(0, term.rows - 1)); } catch { /* ignore teardown or transient measure failures */ diff --git a/packages/dashboard/app/components/TerminalModal.tsx b/packages/dashboard/app/components/TerminalModal.tsx index e84ac9c5ad..39e4c78bf1 100644 --- a/packages/dashboard/app/components/TerminalModal.tsx +++ b/packages/dashboard/app/components/TerminalModal.tsx @@ -1254,11 +1254,32 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG 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. + + FNXC:Terminal 2026-07-04-11:35: + FN-7567 recurrence #4: forcing the remeasure above is necessary but not + sufficient. Real xterm's `DomRenderer._setDefaultSpacing()` (the + letter-spacing compensation baked onto `.xterm-rows`, computed as + `dimensions.css.cell.width - widthCache.get('W')`) only recomputes from + `handleCharSizeChanged()` (wired to `CharSizeService.onCharSizeChange`, + i.e. exactly what `forceTerminalFontRemeasure` above triggers) and from + `handleDevicePixelRatioChange()` — NEVER from `handleResize()`, which is + what `fitAddon.fit()` -> `terminal.resize(cols, rows)` triggers. Calling + `forceTerminalFontRemeasure` BEFORE `fitAddon.fit()` bakes spacing + against the column count that predates the fit, so once fit() changes + the column count (and therefore the true cell width) the baked spacing + goes stale and stays wrong until an unrelated later event (DPR change, + orientation) coincidentally forces another genuine option/DPR-change + remeasure — exactly the reported "only repairs itself after an + incidental refit" symptom. Force a SECOND genuine remeasure AFTER + `fitAddon.fit()` settles the column count so the letter-spacing bake is + recomputed against the FINAL geometry, not the pre-fit one. See + `docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md`. */ forceTerminalFontRemeasure(terminal, resolvedFontFamilyRef.current); terminal.options.fontSize = fontSizeRef.current; fitAddon.fit(); resizeRef.current?.(terminal.cols, terminal.rows); + forceTerminalFontRemeasure(terminal, resolvedFontFamilyRef.current); terminal.refresh(0, Math.max(0, terminal.rows - 1)); } catch { // Ignore fit/refresh errors during teardown or viewport transitions. @@ -1757,7 +1778,22 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG // Defer fit until the next frame so layout reflects the new font metrics // before FitAddon measures rows/cols. Reuse pendingFitRef so font changes and // visualViewport-triggered fits are coalesced into a single scheduled fit. - const scheduleRefit = () => { + /* + FNXC:Terminal 2026-07-04-11:40: + FN-7567 recurrence #4: `rebakeSpacingAfterFit` is set only by the settled + (font-metrics-ready) call site below. Real xterm's `DomRenderer._setDefaultSpacing()` + letter-spacing bake only recomputes from a genuine option-change remeasure, + never from `handleResize()` (what `fitAddon.fit()` triggers), so a settle + that calls `forceTerminalFontRemeasure()` and THEN fits must force one more + genuine remeasure AFTER the fit to re-bake spacing against the FINAL + (post-fit) column count — otherwise the bake stays computed against the + stale pre-fit column count until an unrelated later event happens to force + another remeasure. The unsettled immediate frame intentionally does not + rebake: at that point the web font has not necessarily loaded yet, so + forcing another remeasure there would just re-bake against the same + (possibly still-fallback) metrics. + */ + const scheduleRefit = (rebakeSpacingAfterFit = false) => { if (pendingFitRef.current !== null) { cancelAnimationFrame(pendingFitRef.current); pendingFitRef.current = null; @@ -1770,6 +1806,9 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG } refitTerminal(); xtermRef.current?.refresh?.(0, Math.max(0, xtermRef.current.rows - 1)); + if (rebakeSpacingAfterFit && xtermRef.current) { + forceTerminalFontRemeasure(xtermRef.current, resolvedFontFamily); + } }); pendingFitRef.current = frame; return frame; @@ -1797,7 +1836,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG } forceTerminalFontRemeasure(xtermRef.current, resolvedFontFamily); xtermRef.current.options.fontSize = terminalPreferences.fontSize; - scheduleRefit(); + scheduleRefit(true); }, () => { // FontFaceSet failures are non-fatal; the immediate frame above still diff --git a/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx b/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx index 616330e24e..e038937403 100644 --- a/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx +++ b/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx @@ -2,7 +2,64 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; 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() }; +/* +FNXC:Terminal 2026-07-04-11:50: +FN-7567 recurrence #4: forcing a genuine fontFamily/fontSize transition +(FN-7561's `forceTerminalFontRemeasure`) is necessary but not sufficient. Real +xterm's `DomRenderer._setDefaultSpacing()` (the letter-spacing compensation +baked onto `.xterm-rows`) is recomputed on `CharSizeService.onCharSizeChange` +(any genuine option change) and on `handleDevicePixelRatioChange`, but NOT on +`handleResize()` (the path `fitAddon.fit()` -> `terminal.resize(cols, rows)` +takes). `mockFitAddon.fit`/`mockTerm.cols` model this ordering-sensitive +geometry (measured char width, cell width derived from cols, and the baked +letter-spacing) directly, mirroring xterm's real internals, so the regression +asserts actual rendered geometry instead of a CSS-property or call-count check. +*/ +const MOCK_CONTAINER_WIDTH_PX = 728; +const MOCK_FALLBACK_CHAR_WIDTH_PX = 9; +const MOCK_SETTLED_CHAR_WIDTH_PX = 7; +let mockFontsSettledForCharSize = false; +let mockMeasuredCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX; +let mockBakedLetterSpacingPx = 0; + +function resetMockTerminalGeometry(): void { + mockFontsSettledForCharSize = false; + mockMeasuredCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX; + mockBakedLetterSpacingPx = 0; + mockTerm.cols = 80; +} + +/** Mirrors the real web font finishing its network load/paint settle. */ +function settleMockTerminalFontForCharSize(): void { + mockFontsSettledForCharSize = true; +} + +/** The rendered cell/advance-width geometry invariant under test: 0 == tight contiguous monospace. */ +function getMockBakedLetterSpacingPx(): number { + return mockBakedLetterSpacingPx; +} + +// Mirrors xterm's CharSizeService.measure() -> onCharSizeChange -> +// DomRenderer.handleCharSizeChanged(): runs on every GENUINE fontFamily/fontSize +// option transition, using the CURRENT (possibly stale, pre-fit) column count. +function mockHandleCharSizeChanged(): void { + mockMeasuredCharWidthPx = mockFontsSettledForCharSize + ? MOCK_SETTLED_CHAR_WIDTH_PX + : MOCK_FALLBACK_CHAR_WIDTH_PX; + const cols = (mockTerm.cols as number) || 1; + const cellWidthPx = MOCK_CONTAINER_WIDTH_PX / cols; + mockBakedLetterSpacingPx = cellWidthPx - mockMeasuredCharWidthPx; +} + +// Mirrors FitAddon.fit() -> terminal.resize(cols, rows) -> +// DomRenderer.handleResize(): recomputes cols from the CURRENT measured char +// width but deliberately does NOT touch letter-spacing (real xterm's +// handleResize() never calls _setDefaultSpacing()). +const mockFitAddon = { + fit: vi.fn(() => { + mockTerm.cols = Math.max(1, Math.floor(MOCK_CONTAINER_WIDTH_PX / mockMeasuredCharWidthPx)); + }), +}; let sessionKeyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; /* @@ -38,6 +95,7 @@ function wrapMockTerminalOptions(initial: Record): Record { mockTerm.dispose.mockClear(); mockTerm.options = {}; resetFontRemeasureCount(); + resetMockTerminalGeometry(); Object.defineProperty(document, "fonts", { value: undefined, configurable: true, @@ -404,6 +463,7 @@ describe("SessionTerminal", () => { // touched terminal preferences), so this must be a forced remeasure, not // an incidental preference-driven one. resetFontRemeasureCount(); + resetMockTerminalGeometry(); await act(async () => { resolveLoad?.(); @@ -420,6 +480,71 @@ describe("SessionTerminal", () => { expect(mockTerm.options.fontFamily).toBe(resolveTerminalFontFamily("nerd-font")); }); + /* + FNXC:Terminal 2026-07-04-11:55: + FN-7567 recurrence #4: forcing a genuine remeasure (the assertion above) is + necessary but not sufficient. Real xterm's `DomRenderer._setDefaultSpacing()` + letter-spacing bake only recomputes from a genuine option-change remeasure or + a devicePixelRatio change, never from the `fitAddon.fit()`/resize that + follows it, so a settle that bakes spacing BEFORE fit() leaves it stale + against the post-fit column count until an unrelated later event happens to + force another remeasure. This asserts the measured rendered geometry + invariant (baked letter-spacing == 0) using a xterm-internals-accurate model, + not a re-assertion of the FN-7561 call-count/CSS-property checks; it fails on + pre-fix code and passes once the settle re-bakes spacing AFTER fit(). + */ + it("renders contiguous monospace cells (zero baked letter-spacing) once the mobile web font settles after xterm's initial fit", async () => { + 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(FakeWS.instances.length).toBe(1); + }); + resetFontRemeasureCount(); + + // Simulate the real recurrence: the custom web font finishes downloading + // AFTER xterm's initial fallback-font measurement/fit already ran and + // baked a letter-spacing value that was internally consistent for the + // FALLBACK font at that (stale) column count. + settleMockTerminalFontForCharSize(); + + await act(async () => { + resolveLoad?.(); + resolveReady?.(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + await waitFor(() => { + expect(getFontRemeasureCount()).toBeGreaterThan(0); + }); + + // The measured geometry invariant: once font metrics settle and xterm + // refits to the correct column count for the SETTLED font, the baked + // letter-spacing compensation must be recomputed against that FINAL + // column count, not the stale pre-fit one. + await waitFor(() => { + expect(getMockBakedLetterSpacingPx()).toBeCloseTo(0, 5); + }); + }); + 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 5153399c89..40d96c86ce 100644 --- a/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/TerminalModal.test.tsx @@ -60,7 +60,73 @@ vi.mock("../../api", () => ({ })); // Mock xterm modules to prevent DOM errors in jsdom -const mockFitAddonFit = vi.fn(); +/* +FNXC:Terminal 2026-07-04-11:05: +FN-7567 recurrence #4: forcing a genuine fontFamily/fontSize transition +(FN-7561's `forceTerminalFontRemeasure`) is necessary but not sufficient. Real +xterm's `DomRenderer._setDefaultSpacing()` — the letter-spacing compensation +baked onto `.xterm-rows` (`spacing = dimensions.css.cell.width - +widthCache.get('W')`) — is recomputed on `CharSizeService.onCharSizeChange` +(any genuine option change) and on `handleDevicePixelRatioChange`, but NOT on +`handleResize()` (the path `fitAddon.fit()` -> `terminal.resize(cols, rows)` +takes; see `@xterm/xterm` `src/browser/renderer/dom/DomRenderer.ts`). Both +settle sites call `forceTerminalFontRemeasure()` (which bakes spacing against +the column count that predates `fit()`) and only THEN call `fitAddon.fit()` +(which changes cols/cell-width but never re-bakes spacing), leaving a stale, +oversized letter-spacing baked in until an unrelated later event happens to +force another genuine option/DPR change. `mockFitAddonFit`/`mockTerminalInstance.cols` +model this ordering-sensitive geometry (measured char width, cell width +derived from cols, and the baked letter-spacing) directly, mirroring xterm's +real internals, so the regression asserts actual geometry instead of a +CSS-property or call-count check. +*/ +const MOCK_CONTAINER_WIDTH_PX = 728; +const MOCK_FALLBACK_CHAR_WIDTH_PX = 9; +const MOCK_SETTLED_CHAR_WIDTH_PX = 7; +let mockFontsSettledForCharSize = false; +let mockMeasuredCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX; +let mockBakedLetterSpacingPx = 0; + +function resetMockTerminalGeometry(): void { + mockFontsSettledForCharSize = false; + mockMeasuredCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX; + mockBakedLetterSpacingPx = 0; + mockTerminalInstance.cols = 80; +} + +/** Mirrors the real web font finishing its network load/paint settle. */ +function settleMockTerminalFontForCharSize(): void { + mockFontsSettledForCharSize = true; +} + +/** The rendered cell/advance-width geometry invariant under test: 0 == tight contiguous monospace. */ +function getMockBakedLetterSpacingPx(): number { + return mockBakedLetterSpacingPx; +} + +// Mirrors xterm's CharSizeService.measure() -> onCharSizeChange -> +// DomRenderer.handleCharSizeChanged() -> _updateDimensions() + +// _setDefaultSpacing(): runs on every GENUINE fontFamily/fontSize option +// transition, using the CURRENT (possibly stale, pre-fit) column count. +function mockHandleCharSizeChanged(): void { + mockMeasuredCharWidthPx = mockFontsSettledForCharSize + ? MOCK_SETTLED_CHAR_WIDTH_PX + : MOCK_FALLBACK_CHAR_WIDTH_PX; + const cols = (mockTerminalInstance.cols as number) || 1; + const cellWidthPx = MOCK_CONTAINER_WIDTH_PX / cols; + mockBakedLetterSpacingPx = cellWidthPx - mockMeasuredCharWidthPx; +} + +// Mirrors FitAddon.fit() -> terminal.resize(cols, rows) -> +// DomRenderer.handleResize(): recomputes cols/cell-width from the CURRENT +// measured char width but deliberately does NOT touch letter-spacing (real +// xterm's handleResize() never calls _setDefaultSpacing()). +const mockFitAddonFit = vi.fn(() => { + mockTerminalInstance.cols = Math.max( + 1, + Math.floor(MOCK_CONTAINER_WIDTH_PX / mockMeasuredCharWidthPx), + ); +}); let terminalKeyEventHandler: ((event: KeyboardEvent) => boolean) | null = null; let terminalDataHandler: ((data: string) => void) | null = null; @@ -106,6 +172,7 @@ function createMockTerminalOptions(): Record { store[key] = value; if (key === "fontFamily" || key === "fontSize") { fontRemeasureCount += 1; + mockHandleCharSizeChanged(); } } }, @@ -286,6 +353,7 @@ describe("TerminalModal", () => { mockTerminalInstance.options.cursorStyle = "block"; mockTerminalInstance.options.cursorBlink = true; resetFontRemeasureCount(); + resetMockTerminalGeometry(); mockCreateTerminalSession.mockResolvedValue({ sessionId: "test-session-123", shell: "/bin/bash", @@ -7151,6 +7219,7 @@ describe("TerminalModal — FN-7561 mobile inter-character spacing (xterm no-op beforeEach(() => { vi.clearAllMocks(); resetFontRemeasureCount(); + resetMockTerminalGeometry(); vi.spyOn(window, "matchMedia").mockImplementation((query: string) => ({ matches: false, media: query, @@ -7171,6 +7240,7 @@ describe("TerminalModal — FN-7561 mobile inter-character spacing (xterm no-op mockTerminalInstance.options.cursorStyle = "block"; mockTerminalInstance.options.cursorBlink = true; resetFontRemeasureCount(); + resetMockTerminalGeometry(); mockUseTerminal.mockReturnValue(createMockTerminalState()); mockUseTerminalSessions.mockReturnValue(defaultSessionState); mockUseWorkspaces.mockReturnValue({ @@ -7231,6 +7301,7 @@ describe("TerminalModal — FN-7561 mobile inter-character spacing (xterm no-op // application done during xterm construction/effect setup — reset it and // isolate exactly what happens once the deferred font-load settles. resetFontRemeasureCount(); + resetMockTerminalGeometry(); await act(async () => { resolveLoad?.(); @@ -7297,6 +7368,7 @@ describe("TerminalModal — FN-7561 mobile inter-character spacing (xterm no-op await waitFor(() => expect(mockTerminalInstance.open).toHaveBeenCalled()); resetFontRemeasureCount(); + resetMockTerminalGeometry(); await act(async () => { resolveLoad?.(); @@ -7313,3 +7385,263 @@ describe("TerminalModal — FN-7561 mobile inter-character spacing (xterm no-op Object.defineProperty(window, "visualViewport", { value: undefined, writable: true, configurable: true }); }); }); + +/* +FNXC:Terminal 2026-07-04-11:20: +FN-7567 (recurrence #4 of mobile terminal inter-character spacing, after +FN-7456's DOM glyph-fallback fix, FN-7460's `text-size-adjust: none`, and +FN-7561's `forceTerminalFontRemeasure`) root cause: real xterm.js's +`DomRenderer._setDefaultSpacing()` bakes the letter-spacing compensation used +to correct rounding drift between the measured character width and the +computed cell width (`dimensions.css.cell.width`, which is itself derived from +the container width divided by the CURRENT column count). That bake only runs +from `handleCharSizeChanged()` (wired to `CharSizeService.onCharSizeChange`, +i.e. any genuine `fontFamily`/`fontSize` option transition) and from +`handleDevicePixelRatioChange()` \u2014 NEVER from `handleResize()`, which is what +`fitAddon.fit()` -> `terminal.resize(cols, rows)` triggers. Both mobile +settle sites (`remeasureAfterTerminalFontLoad` in TerminalModal.tsx and its +SessionTerminal.tsx sibling, plus the live-preferences-apply settle path in +both files) call `forceTerminalFontRemeasure()` \u2014 which correctly forces a +genuine option transition and DOES bake letter-spacing \u2014 but they bake it +using the STALE column count that predates `fitAddon.fit()`. `fitAddon.fit()` +then changes the column count (and therefore the true cell width) but never +re-bakes the letter-spacing, so the terminal keeps rendering with a spacing +value computed against a column count that no longer matches reality. This +produces genuinely excessive inter-character gaps that persist until an +unrelated later event (device-pixel-ratio change, orientation, reconnect) +happens to force another *genuine* option/DPR-triggered remeasure \u2014 exactly +matching the "only repairs itself after an incidental refit" report. See +`docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md` +recurrence-#4 addendum. + +This suite asserts the actual rendered geometry invariant (baked letter-spacing +== 0, i.e. cell width matches the settled glyph advance width) using a +xterm-internals-accurate model (`mockHandleCharSizeChanged`/`mockFitAddonFit`), +not a re-assertion of the FN-7456/FN-7460/FN-7561 CSS-property/call-count +checks. It fails on pre-fix code (stale pre-fit letter-spacing survives the +settle) and passes once the fix re-bakes spacing AFTER `fitAddon.fit()` +settles the column count. +*/ +describe("TerminalModal — FN-7567 mobile inter-character spacing (stale post-fit letter-spacing bake)", () => { + 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(); + resetMockTerminalGeometry(); + 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(); + resetMockTerminalGeometry(); + 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("renders contiguous monospace cells (zero baked letter-spacing) once the mobile web font settles after xterm's initial fit", async () => { + 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(); + + // Simulate the real recurrence: the custom web font finishes downloading + // AFTER xterm's initial fallback-font measurement/fit already ran and + // baked a letter-spacing value that was internally consistent for the + // FALLBACK font at that (stale) column count. + settleMockTerminalFontForCharSize(); + + await act(async () => { + resolveLoad?.(); + resolveReady?.(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + await waitFor(() => { + expect(getFontRemeasureCount()).toBeGreaterThan(0); + }); + + // The measured geometry invariant: once font metrics settle and xterm + // refits to the correct column count for the SETTLED font, the baked + // letter-spacing compensation must be recomputed against that FINAL + // column count, not the stale pre-fit one. A nonzero value here means + // rendered cells are wider (or narrower) than the glyph advance width \u2014 + // exactly the reported "characters spread across cells" symptom \u2014 and + // this assertion fails on pre-fix code, which bakes spacing only BEFORE + // `fitAddon.fit()` runs and never re-bakes it afterward. + await waitFor(() => { + expect(getMockBakedLetterSpacingPx()).toBeCloseTo(0, 5); + }); + }); + + it("also settles to zero baked letter-spacing at persisted 10px with the mobile keyboard already open at initial render", async () => { + window.localStorage.setItem(TERMINAL_FONT_SIZE_KEY, "10"); + + 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(); + settleMockTerminalFontForCharSize(); + + await act(async () => { + resolveLoad?.(); + resolveReady?.(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + await waitFor(() => { + expect(getFontRemeasureCount()).toBeGreaterThan(0); + }); + + // Same measured geometry invariant, but with the keyboard already + // constraining the viewport at initial render and a persisted 10px font — + // the exact FN-7460 screenshot conditions — to prove the fix is not + // accidentally scoped to only the default font-size/no-keyboard case. + await waitFor(() => { + expect(getMockBakedLetterSpacingPx()).toBeCloseTo(0, 5); + }); + + Object.defineProperty(window, "visualViewport", { value: undefined, writable: true, configurable: true }); + }); + + it("also settles to zero baked letter-spacing when a live font-size preference change resettles after fit", async () => { + Object.defineProperty(document, "fonts", { + value: { + load: vi.fn(() => Promise.resolve()), + ready: Promise.resolve(), + }, + configurable: true, + }); + + render(); + await waitFor(() => expect(mockTerminalInstance.open).toHaveBeenCalled()); + + // Let the initial-open settle path finish and reach a consistent baseline. + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + settleMockTerminalFontForCharSize(); + resetFontRemeasureCount(); + + fireEvent.click(await screen.findByTestId("terminal-font-size-increase")); + + await waitFor(() => { + expect(getFontRemeasureCount()).toBeGreaterThan(0); + }); + + await waitFor(() => { + expect(getMockBakedLetterSpacingPx()).toBeCloseTo(0, 5); + }); + }); +});