FN-7567: re-bake xterm letter-spacing after fit() to fix mobile terminal spacing recurrence
Fixes recurrence #4 of the mobile terminal excess character-spacing bug: SessionTerminal and TerminalModal now force a second genuine xterm font remeasure AFTER fitAddon.fit() settles the post-fit column count, since handleResize() never re-bakes DomRenderer's letter-spacing compensation itself. - SessionTerminal.tsx: call forceTerminalFontRemeasure() again after fit()/sendResizeMessage() in the resize handler, re-baking spacing against the settled (post-fit) column count instead of the stale pre-fit one. - TerminalModal.tsx: same second forceTerminalFontRemeasure() call after fitAddon.fit()/sendResize() in its resize handling path. - Expanded SessionTerminal.test.tsx and TerminalModal.test.tsx coverage to assert the post-fit remeasure occurs. - Added docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md documenting recurrence #4 root cause (DomRenderer._setDefaultSpacing() never recomputes from handleResize()). - Added changeset fn-7567-mobile-terminal-spacing.md (patch, category fix). Files changed: .changeset/fn-7567-mobile-terminal-spacing.md | 7 + docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md | 89 ++++++ packages/dashboard/app/components/SessionTerminal.tsx | 23 ++ packages/dashboard/app/components/TerminalModal.tsx | 43 ++- packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx | 127 +++++++- packages/dashboard/app/components/__tests__/TerminalModal.test.tsx | 334 ++++++++++++++++++++- 6 files changed, 619 insertions(+), 4 deletions(-) Fusion-Task-Id: FN-7567 Fusion-Task-Lineage: 5da20522-82d3-4c4d-9008-db71bc5b4d75 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7567-mobile-terminal-spacing.md
Normal file
7
.changeset/fn-7567-mobile-terminal-spacing.md
Normal file
@@ -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.
|
||||
@@ -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`.
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, unknown>): Record<strin
|
||||
store[key] = value;
|
||||
if (key === "fontFamily" || key === "fontSize") {
|
||||
fontRemeasureCount += 1;
|
||||
mockHandleCharSizeChanged();
|
||||
}
|
||||
}
|
||||
},
|
||||
@@ -151,6 +209,7 @@ beforeEach(() => {
|
||||
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<void>((resolve) => {
|
||||
resolveLoad = resolve;
|
||||
}),
|
||||
),
|
||||
ready: new Promise<void>((resolve) => {
|
||||
resolveReady = resolve;
|
||||
}),
|
||||
},
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
render(<SessionTerminal sessionId="s1" />);
|
||||
|
||||
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(
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
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<void>((resolve) => {
|
||||
resolveLoad = resolve;
|
||||
}),
|
||||
),
|
||||
ready: new Promise<void>((resolve) => {
|
||||
resolveReady = resolve;
|
||||
}),
|
||||
},
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
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<void>((resolve) => {
|
||||
resolveLoad = resolve;
|
||||
}),
|
||||
),
|
||||
ready: new Promise<void>((resolve) => {
|
||||
resolveReady = resolve;
|
||||
}),
|
||||
},
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
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(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user