FN-7603: force xterm DOM-based char measurement to fix mobile terminal spacing
Fixes recurrence #5 of mobile terminal inter-character spacing by unifying xterm's cell-width measurement pipeline with WidthCache's DOM-based glyph measurement, validated against real xterm instead of the jsdom mock. - Add withDomBasedTerminalCharacterMeasurement() in terminalPreferences.ts: transiently hides window.OffscreenCanvas during terminal.open() so CharSizeService's constructor throws and self-selects its own DOM-based fallback strategy, unifying dimensions.css.cell.width with WidthCache.get('W') measurement - Wire withDomBasedTerminalCharacterMeasurement() around terminal.open() calls in SessionTerminal.tsx and TerminalModal.tsx - Add FNXC:Terminal comments documenting the Canvas-vs-DOM measurement divergence root cause, grounded in the installed @xterm/xterm@5.5.0 source - Add docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md recurrence #5 section - Expand TerminalModal.test.tsx coverage for the new measurement-forcing behavior - Add changeset fn-7603-mobile-terminal-spacing.md (patch, fix) Files changed: .changeset/fn-7603-mobile-terminal-spacing.md | 7 + docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md | 101 ++++++ packages/dashboard/app/components/SessionTerminal.tsx | 16 +- packages/dashboard/app/components/TerminalModal.tsx | 16 +- packages/dashboard/app/components/__tests__/TerminalModal.test.tsx | 363 ++++++++++++++++++++- packages/dashboard/app/utils/terminalPreferences.ts | 63 ++++ 6 files changed, 554 insertions(+), 12 deletions(-) Fusion-Task-Id: FN-7603 Fusion-Task-Lineage: 6c7d980f-953e-4fa9-908e-b24125904cbe Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7603-mobile-terminal-spacing.md
Normal file
7
.changeset/fn-7603-mobile-terminal-spacing.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Fix persistent mobile terminal inter-character spacing (5th recurrence root cause).
|
||||
category: fix
|
||||
dev: xterm's CharSizeService picks a Canvas-based (OffscreenCanvas) or DOM-based character-measurement strategy at terminal.open() time; DomRenderer's letter-spacing bake always measures via a separate DOM-based WidthCache, so a Canvas-vs-DOM measurement mismatch survived FN-7561/FN-7567's remeasure-ordering fixes. `withDomBasedTerminalCharacterMeasurement` in terminalPreferences.ts forces CharSizeService onto the same DOM strategy for both TerminalModal and SessionTerminal.
|
||||
@@ -207,3 +207,104 @@ jsdom cannot exercise real xterm.js internals, so the FN-7567 regression models
|
||||
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`.
|
||||
|
||||
## Recurrence #5 (FN-7603): mock/real divergence — Canvas vs DOM character measurement
|
||||
|
||||
FN-7561's `forceTerminalFontRemeasure()` and FN-7567's post-fit re-bake both ran correctly, and both
|
||||
were validated ONLY against jsdom test doubles that never exercise real `@xterm/xterm@5.5.0`. On the
|
||||
reported real mobile device, ordinary ASCII still rendered with visible gaps on the initial paint. This
|
||||
is the fifth recurrence of the same defect, so the FN-7603 executor was required to read the installed
|
||||
`@xterm/xterm@5.5.0`/`@xterm/addon-fit@0.10.0` source before touching production code (see task
|
||||
document key="xterm-source-audit" on FN-7603).
|
||||
|
||||
### The actual mechanism
|
||||
|
||||
xterm's `CharSizeService` selects ONE of two measurement strategies **the moment `terminal.open()`
|
||||
runs**:
|
||||
|
||||
```js
|
||||
// @xterm/xterm/lib/xterm.js (installed 5.5.0), CharSizeService constructor
|
||||
try { this._measureStrategy = new OffscreenCanvasStrategy(optionsService) } // canvas: ctx.measureText("W")
|
||||
catch { this._measureStrategy = new DomFallbackStrategy(document, container, optionsService) } // DOM: offsetWidth/32
|
||||
```
|
||||
|
||||
The Canvas strategy is chosen whenever `OffscreenCanvas` + `CanvasRenderingContext2D.measureText()`
|
||||
reporting `fontBoundingBoxAscent`/`fontBoundingBoxDescent` are available — true on essentially every
|
||||
real modern mobile Safari/Chrome. `dimensions.css.cell.width` (which feeds both `FitAddon.fit()`'s
|
||||
column count, per the installed `addon-fit@0.10.0` `proposeDimensions()`, and
|
||||
`DomRenderer._setDefaultSpacing()`'s baked letter-spacing) derives from whichever strategy
|
||||
`CharSizeService` picked.
|
||||
|
||||
Separately, `DomRenderer._setDefaultSpacing()` and `DomRendererRowFactory.createRow()`'s per-glyph
|
||||
override BOTH measure via `WidthCache`, which is **always** DOM-based (`offsetWidth` of a hidden
|
||||
32×-repeated-character span) — entirely independent of `CharSizeService`'s strategy choice. Real glyphs
|
||||
are painted 100% through the DOM (`DomRenderer` never draws through canvas). Canvas 2D text measurement
|
||||
and DOM/CSS text layout are two different browser rendering pipelines that can disagree — even by a
|
||||
fraction of a device pixel — for the same font on the same device; this is a documented,
|
||||
long-standing cross-API text-metrics inconsistency. `_setDefaultSpacing()`'s formula
|
||||
(`dimensions.css.cell.width - widthCache.get('W')`) only correctly converges to zero (tight, contiguous
|
||||
cells) when both operands are measured through the SAME pipeline. None of FN-7456/FN-7460/FN-7561/
|
||||
FN-7567 (or their test doubles) ever modeled this — all four assumed CharSizeService's measurement and
|
||||
WidthCache's measurement were the same value.
|
||||
|
||||
### Why FN-7456/FN-7460/FN-7561/FN-7567 missed this
|
||||
|
||||
Every prior fix's test double (`mockHandleCharSizeChanged`) treated the measured character width as a
|
||||
single shared value used for both "the cell width that drives fit" and "the width WidthCache subtracts
|
||||
in `_setDefaultSpacing()`" — a faithful-looking model of xterm's DOM-only fallback strategy, but NOT of
|
||||
the Canvas strategy that real xterm actually selects by default on real mobile browsers. Because jsdom
|
||||
cannot run real `@xterm/xterm`, and no fix before FN-7603 cross-checked the double against the installed
|
||||
source, the divergence between "what CharSizeService measures" (Canvas, in the real common case) and
|
||||
"what WidthCache measures" (always DOM) went completely uncovered for four recurrences.
|
||||
|
||||
### Solution
|
||||
|
||||
Force `CharSizeService` to construct with its own DOM fallback strategy — unifying the cell-width
|
||||
measurement with `WidthCache`'s measurement — by making `OffscreenCanvas` transiently unavailable for
|
||||
the synchronous duration of `terminal.open()` (where `CharSizeService` is constructed):
|
||||
|
||||
```ts
|
||||
// packages/dashboard/app/utils/terminalPreferences.ts
|
||||
export function withDomBasedTerminalCharacterMeasurement<T>(fn: () => T): T {
|
||||
const descriptor = Object.getOwnPropertyDescriptor(window, "OffscreenCanvas");
|
||||
delete (window as any).OffscreenCanvas;
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
if (descriptor) Object.defineProperty(window, "OffscreenCanvas", descriptor);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Both `TerminalModal.tsx` and `SessionTerminal.tsx` now wrap their `terminal.open(container)` call in
|
||||
`withDomBasedTerminalCharacterMeasurement(() => terminal.open(container))`. `CharSizeService`'s
|
||||
constructor try-block throws (no `OffscreenCanvas` global), so it self-selects the SAME DOM-based
|
||||
strategy `WidthCache` already always uses — no hardcoded letter-spacing/cell-width compensation is
|
||||
added; the fix unifies the measurement pipeline instead.
|
||||
|
||||
Do not:
|
||||
|
||||
- Patch `window.OffscreenCanvas` outside the narrow synchronous `open()` window — other page code
|
||||
(charts, canvas-based rendering elsewhere in the dashboard) may legitimately need it.
|
||||
- Assume this is scoped to mobile only — desktop with the DOM renderer (WebGL addon failed to load, or
|
||||
`renderer: "canvas"` preference) has the identical divergence and benefits from the same fix.
|
||||
- Treat this as a replacement for FN-7561/FN-7567 — both remain necessary; this fix addresses a
|
||||
different, independent measurement-pipeline mismatch.
|
||||
|
||||
### Regression coverage (Canvas-vs-DOM divergence, not CSS/call-count)
|
||||
|
||||
- Extended the FN-7567 double: `mockCanvasCharWidthPx` (drives `FitAddon.fit()`'s column count,
|
||||
mirroring `dimensions.css.cell.width`) can diverge from `mockDomCharWidthPx` (mirrors
|
||||
`WidthCache.get('W')`) by a fixed offset, gated on `window.OffscreenCanvas` being defined at the
|
||||
moment the mock's `open()` runs — exactly mirroring the real `CharSizeService` constructor's
|
||||
try/catch strategy selection.
|
||||
- The assertion is the same rendered-geometry invariant as FN-7567 (baked letter-spacing `== 0`), but
|
||||
now fails on HEAD even with the full FN-7561/FN-7567 settle+pre/post-fit-remeasure sequence present,
|
||||
because the divergence is NOT an ordering bug — it's a measurement-pipeline bug those fixes cannot
|
||||
see or fix.
|
||||
- See `TerminalModal.test.tsx` describe block "FN-7603 mobile inter-character spacing (Canvas vs DOM
|
||||
CharSizeService measurement divergence)".
|
||||
- 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 — this gap is recorded explicitly (task document
|
||||
key="repro" on FN-7603) rather than treating jsdom/desktop WebKit as proof.
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
resolveTerminalFontFamily,
|
||||
resolveTerminalGlyphFontFamily,
|
||||
waitForTerminalFontMetrics,
|
||||
withDomBasedTerminalCharacterMeasurement,
|
||||
} from "../utils/terminalPreferences";
|
||||
|
||||
/**
|
||||
@@ -459,7 +460,20 @@ export function SessionTerminal({
|
||||
term.loadAddon(unicode11);
|
||||
term.unicode.activeVersion = "11";
|
||||
|
||||
term.open(containerRef.current);
|
||||
/*
|
||||
FNXC:Terminal 2026-07-05-12:40:
|
||||
FN-7603 recurrence #5: mirror TerminalModal's fix — force xterm's
|
||||
CharSizeService to self-select its DOM-based measurement strategy for
|
||||
the synchronous duration of open() so cell-width measurement (feeding
|
||||
FitAddon.fit() and DomRenderer._setDefaultSpacing()'s baked
|
||||
letter-spacing) uses the SAME pipeline as WidthCache's DOM-based
|
||||
per-glyph measurement, instead of the default Canvas/OffscreenCanvas
|
||||
strategy that measures through a different pipeline. See
|
||||
docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md.
|
||||
*/
|
||||
withDomBasedTerminalCharacterMeasurement(() => {
|
||||
term.open(containerRef.current!);
|
||||
});
|
||||
xtermRef.current = term;
|
||||
fitAddonRef.current = fitAddon as unknown as ITerminalAddon;
|
||||
|
||||
|
||||
@@ -46,6 +46,7 @@ import {
|
||||
resolveTerminalFontFamily,
|
||||
resolveTerminalGlyphFontFamily,
|
||||
waitForTerminalFontMetrics,
|
||||
withDomBasedTerminalCharacterMeasurement,
|
||||
writeTerminalPreferences,
|
||||
type TerminalPreferences,
|
||||
type TerminalRenderer,
|
||||
@@ -1406,7 +1407,20 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG
|
||||
}
|
||||
|
||||
// Open terminal in container
|
||||
terminal.open(terminalRef.current);
|
||||
/*
|
||||
FNXC:Terminal 2026-07-05-12:40:
|
||||
FN-7603 recurrence #5: force xterm's CharSizeService to self-select its
|
||||
DOM-based measurement strategy (instead of its default Canvas/
|
||||
OffscreenCanvas strategy) for the synchronous duration of open(), so the
|
||||
cell-width measurement that feeds FitAddon.fit() and
|
||||
DomRenderer._setDefaultSpacing()'s baked letter-spacing uses the SAME
|
||||
pipeline as WidthCache's DOM-based per-glyph measurement. See
|
||||
`withDomBasedTerminalCharacterMeasurement` and
|
||||
docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md.
|
||||
*/
|
||||
withDomBasedTerminalCharacterMeasurement(() => {
|
||||
terminal.open(terminalRef.current!);
|
||||
});
|
||||
|
||||
// Clear watchdog — imports and open() succeeded within deadline
|
||||
if (watchdogTimer) {
|
||||
|
||||
@@ -83,13 +83,40 @@ 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;
|
||||
/*
|
||||
FNXC:Terminal 2026-07-05-12:45:
|
||||
FN-7603 recurrence #5: real xterm's `CharSizeService` picks ONE of two
|
||||
measurement strategies at `terminal.open()` time — Canvas/OffscreenCanvas
|
||||
(`ctx.measureText("W")`, chosen whenever `OffscreenCanvas` + the required
|
||||
`TextMetrics` fields are available, i.e. virtually every real mobile browser)
|
||||
or a DOM fallback (`offsetWidth` of a hidden repeated-"W" span, chosen only if
|
||||
the canvas strategy's constructor throws). Separately, `DomRenderer.
|
||||
_setDefaultSpacing()`/`DomRendererRowFactory` ALWAYS measure via `WidthCache`,
|
||||
which is ALWAYS DOM-based (`offsetWidth`), regardless of which strategy
|
||||
CharSizeService picked. The FN-7567 mock above (and prior recurrences) modeled
|
||||
both as the SAME shared width, hiding this divergence. Model them
|
||||
independently: `mockCanvasCharWidthPx` (drives `FitAddon.fit()`'s column
|
||||
count, mirroring `dimensions.css.cell.width`) can diverge from
|
||||
`mockDomCharWidthPx` (mirrors `WidthCache.get('W')`) whenever
|
||||
`window.OffscreenCanvas` is defined at the moment the mock's `open()` runs —
|
||||
exactly mirroring the real `CharSizeService` constructor's
|
||||
`try { new OffscreenCanvasStrategy } catch { new DomFallbackStrategy }`
|
||||
selection. The production fix (`withDomBasedTerminalCharacterMeasurement`)
|
||||
hides `window.OffscreenCanvas` for the synchronous duration of `open()`, which
|
||||
this mock's `open()` observes to decide which strategy was "selected".
|
||||
*/
|
||||
const MOCK_CANVAS_DOM_DIVERGENCE_PX = 0.7;
|
||||
let mockFontsSettledForCharSize = false;
|
||||
let mockMeasuredCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX;
|
||||
let mockDomCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX;
|
||||
let mockCanvasCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX;
|
||||
let mockCharSizeServiceUsesCanvasStrategy = true;
|
||||
let mockBakedLetterSpacingPx = 0;
|
||||
|
||||
function resetMockTerminalGeometry(): void {
|
||||
mockFontsSettledForCharSize = false;
|
||||
mockMeasuredCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX;
|
||||
mockDomCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX;
|
||||
mockCanvasCharWidthPx = MOCK_FALLBACK_CHAR_WIDTH_PX;
|
||||
mockCharSizeServiceUsesCanvasStrategy = true;
|
||||
mockBakedLetterSpacingPx = 0;
|
||||
mockTerminalInstance.cols = 80;
|
||||
}
|
||||
@@ -104,27 +131,48 @@ function getMockBakedLetterSpacingPx(): number {
|
||||
return mockBakedLetterSpacingPx;
|
||||
}
|
||||
|
||||
/**
|
||||
* Mirrors real xterm's `CharSizeService` constructor picking its measurement
|
||||
* strategy the moment `terminal.open()` runs: Canvas/OffscreenCanvas when
|
||||
* `window.OffscreenCanvas` is present, DOM fallback otherwise.
|
||||
*/
|
||||
function mockSelectCharSizeServiceStrategyAtOpen(): void {
|
||||
mockCharSizeServiceUsesCanvasStrategy =
|
||||
typeof (window as unknown as { OffscreenCanvas?: unknown }).OffscreenCanvas !== "undefined";
|
||||
}
|
||||
|
||||
// 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.
|
||||
//
|
||||
// `mockDomCharWidthPx` mirrors `WidthCache.get('W')` (always DOM-based).
|
||||
// `mockCanvasCharWidthPx` mirrors `CharSizeService.width`: identical to the
|
||||
// DOM value when the DOM strategy was selected at open(), but offset by a
|
||||
// fixed divergence when the Canvas strategy was selected — modeling the real
|
||||
// cross-pipeline (Canvas 2D vs DOM layout) measurement discrepancy that
|
||||
// `_setDefaultSpacing()`'s `dimensions.css.cell.width - widthCache.get('W')`
|
||||
// formula depends on both operands agreeing to correctly converge to zero.
|
||||
function mockHandleCharSizeChanged(): void {
|
||||
mockMeasuredCharWidthPx = mockFontsSettledForCharSize
|
||||
mockDomCharWidthPx = mockFontsSettledForCharSize
|
||||
? MOCK_SETTLED_CHAR_WIDTH_PX
|
||||
: MOCK_FALLBACK_CHAR_WIDTH_PX;
|
||||
mockCanvasCharWidthPx = mockCharSizeServiceUsesCanvasStrategy
|
||||
? mockDomCharWidthPx + MOCK_CANVAS_DOM_DIVERGENCE_PX
|
||||
: mockDomCharWidthPx;
|
||||
const cols = (mockTerminalInstance.cols as number) || 1;
|
||||
const cellWidthPx = MOCK_CONTAINER_WIDTH_PX / cols;
|
||||
mockBakedLetterSpacingPx = cellWidthPx - mockMeasuredCharWidthPx;
|
||||
mockBakedLetterSpacingPx = cellWidthPx - mockDomCharWidthPx;
|
||||
}
|
||||
|
||||
// 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()).
|
||||
// Mirrors FitAddon.proposeDimensions(): cols = floor(availableWidth /
|
||||
// renderService.dimensions.css.cell.width) — the CANVAS-strategy-derived
|
||||
// value when that strategy is active, matching the installed
|
||||
// @xterm/addon-fit@0.10.0 source (`t.css.cell.width`).
|
||||
const mockFitAddonFit = vi.fn(() => {
|
||||
mockTerminalInstance.cols = Math.max(
|
||||
1,
|
||||
Math.floor(MOCK_CONTAINER_WIDTH_PX / mockMeasuredCharWidthPx),
|
||||
Math.floor(MOCK_CONTAINER_WIDTH_PX / mockCanvasCharWidthPx),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -183,7 +231,11 @@ function createMockTerminalOptions(): Record<string, unknown> {
|
||||
|
||||
const mockTerminalInstance = {
|
||||
loadAddon: vi.fn(),
|
||||
open: vi.fn(),
|
||||
// FN-7603: mirror the real CharSizeService constructor's strategy
|
||||
// selection, which happens synchronously inside terminal.open().
|
||||
open: vi.fn(() => {
|
||||
mockSelectCharSizeServiceStrategyAtOpen();
|
||||
}),
|
||||
onData: vi.fn((cb: (data: string) => void) => {
|
||||
terminalDataHandler = cb;
|
||||
return { dispose: vi.fn() };
|
||||
@@ -7645,3 +7697,294 @@ describe("TerminalModal — FN-7567 mobile inter-character spacing (stale post-f
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:Terminal 2026-07-05-12:50:
|
||||
FN-7603 (recurrence #5 of mobile terminal inter-character spacing, after
|
||||
FN-7456's DOM glyph-fallback fix, FN-7460's `text-size-adjust: none`,
|
||||
FN-7561's `forceTerminalFontRemeasure`, and FN-7567's post-fit re-bake) root
|
||||
cause, grounded against the installed `@xterm/xterm@5.5.0` source (see task
|
||||
document key="xterm-source-audit" on FN-7603): xterm's `CharSizeService`
|
||||
selects ONE of two independent measurement strategies the moment
|
||||
`terminal.open()` runs — a Canvas/`OffscreenCanvas` strategy (chosen whenever
|
||||
`OffscreenCanvas` + the required `TextMetrics` fields are available, i.e.
|
||||
virtually every real modern mobile browser) or a DOM fallback strategy (only
|
||||
selected if the canvas strategy's constructor throws). `dimensions.css.cell.width`
|
||||
(which feeds `FitAddon.fit()`'s column count AND `DomRenderer.
|
||||
_setDefaultSpacing()`'s baked letter-spacing) derives from whichever strategy
|
||||
CharSizeService picked. Separately, `WidthCache` (used by both
|
||||
`_setDefaultSpacing()` and `DomRendererRowFactory`'s per-glyph override) is
|
||||
ALWAYS DOM-based. Real glyphs are painted 100% through the DOM, so
|
||||
`_setDefaultSpacing()`'s `cell.width - widthCache.get('W')` formula only
|
||||
converges to zero — i.e. tight, contiguous monospace cells — when BOTH
|
||||
operands are measured through the SAME pipeline. Canvas 2D text measurement
|
||||
and DOM/CSS text layout are two different browser rendering pipelines that can
|
||||
disagree by a small but visible amount for the same font on the same device —
|
||||
a divergence none of FN-7456/FN-7460/FN-7561/FN-7567 (or their test doubles)
|
||||
ever modeled, because all four assumed a single unified character-width
|
||||
measurement. This is why the reported symptom survived every prior remedy:
|
||||
none of them touched WHICH measurement pipeline xterm's cell geometry is
|
||||
computed from, only WHEN it recomputes.
|
||||
|
||||
The fix (`withDomBasedTerminalCharacterMeasurement` in terminalPreferences.ts)
|
||||
makes `window.OffscreenCanvas` transiently unavailable for the synchronous
|
||||
duration of `terminal.open()`, forcing `CharSizeService`'s constructor
|
||||
try-block to throw and self-select its own DOM fallback strategy — unifying
|
||||
`dimensions.css.cell.width` and `WidthCache.get('W')` onto the SAME
|
||||
measurement pipeline instead of adding any hardcoded letter-spacing
|
||||
compensation.
|
||||
|
||||
This suite extends the FN-7567 geometry-accurate mock
|
||||
(`mockHandleCharSizeChanged`/`mockFitAddonFit`) to model the Canvas-vs-DOM
|
||||
divergence explicitly (`mockCanvasCharWidthPx` vs `mockDomCharWidthPx`, gated
|
||||
on `window.OffscreenCanvas` availability observed at the mock's `open()` call
|
||||
— exactly mirroring the real CharSizeService constructor's strategy
|
||||
selection), which the FN-7567 double could not represent (it modeled both
|
||||
measurements as a single shared value). It fails on pre-fix code — where
|
||||
`window.OffscreenCanvas` stays available throughout `open()`, so the mock
|
||||
selects its "Canvas strategy" and the baked letter-spacing settles to a
|
||||
persistent NONZERO value even after the full settle + pre/post-fit remeasure
|
||||
sequence FN-7561/FN-7567 added — and passes once the fix hides
|
||||
`OffscreenCanvas` around `open()`, converging the bake to exactly zero.
|
||||
See `docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md`
|
||||
recurrence-#5 section.
|
||||
*/
|
||||
describe("TerminalModal — FN-7603 mobile inter-character spacing (Canvas vs DOM CharSizeService measurement divergence)", () => {
|
||||
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;
|
||||
let previousOffscreenCanvas: unknown;
|
||||
let hadOwnOffscreenCanvas: boolean;
|
||||
|
||||
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;
|
||||
hadOwnOffscreenCanvas = Object.prototype.hasOwnProperty.call(window, "OffscreenCanvas");
|
||||
previousOffscreenCanvas = (window as unknown as { OffscreenCanvas?: unknown }).OffscreenCanvas;
|
||||
// Real reported device: a narrow touch-primary mobile viewport with a
|
||||
// modern engine that supports OffscreenCanvas (true on essentially every
|
||||
// real mobile Safari/Chrome), so xterm's CharSizeService would pick its
|
||||
// Canvas measurement strategy absent the fix.
|
||||
Object.defineProperty(window, "innerWidth", { value: 390, configurable: true });
|
||||
Object.defineProperty(window, "ontouchstart", { value: null, configurable: true });
|
||||
Object.defineProperty(window, "OffscreenCanvas", {
|
||||
value: class MockOffscreenCanvas {},
|
||||
writable: true,
|
||||
configurable: true,
|
||||
});
|
||||
window.localStorage.removeItem(TERMINAL_FONT_SIZE_KEY);
|
||||
window.localStorage.removeItem(TERMINAL_PREFERENCES_KEY);
|
||||
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 });
|
||||
}
|
||||
if (hadOwnOffscreenCanvas) {
|
||||
Object.defineProperty(window, "OffscreenCanvas", {
|
||||
value: previousOffscreenCanvas,
|
||||
writable: true,
|
||||
configurable: true,
|
||||
});
|
||||
} else {
|
||||
delete (window as unknown as { OffscreenCanvas?: unknown }).OffscreenCanvas;
|
||||
}
|
||||
vi.useRealTimers();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("converges baked letter-spacing to exactly zero by forcing xterm off its Canvas character-measurement strategy during open()", 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();
|
||||
|
||||
settleMockTerminalFontForCharSize();
|
||||
|
||||
await act(async () => {
|
||||
resolveLoad?.();
|
||||
resolveReady?.();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getFontRemeasureCount()).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
/*
|
||||
* The decisive geometry assertion for this recurrence: on pre-fix code,
|
||||
* `window.OffscreenCanvas` stays defined throughout `terminal.open()`, so
|
||||
* the mock's CharSizeService strategy selects "Canvas" and
|
||||
* `mockCanvasCharWidthPx` diverges from `mockDomCharWidthPx` by
|
||||
* `MOCK_CANVAS_DOM_DIVERGENCE_PX`. Even after the full FN-7561/FN-7567
|
||||
* settle + pre/post-fit remeasure sequence runs to completion, the baked
|
||||
* letter-spacing does NOT converge to zero — it settles at a persistent,
|
||||
* nonzero residual driven purely by the Canvas-vs-DOM measurement
|
||||
* mismatch, exactly matching "still spaced apart even after every prior
|
||||
* fix ran correctly". This assertion fails on HEAD before this task's fix
|
||||
* and passes once `withDomBasedTerminalCharacterMeasurement` hides
|
||||
* `OffscreenCanvas` around `open()`.
|
||||
*/
|
||||
await waitFor(() => {
|
||||
expect(getMockBakedLetterSpacingPx()).toBeCloseTo(0, 5);
|
||||
});
|
||||
});
|
||||
|
||||
it("also converges to zero with the mobile keyboard already open and a persisted 10px font", 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);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getMockBakedLetterSpacingPx()).toBeCloseTo(0, 5);
|
||||
});
|
||||
|
||||
Object.defineProperty(window, "visualViewport", { value: undefined, writable: true, configurable: true });
|
||||
});
|
||||
|
||||
it("does not regress when the real browser has no OffscreenCanvas support (xterm already self-selects the DOM strategy)", async () => {
|
||||
window.localStorage.removeItem(TERMINAL_FONT_SIZE_KEY);
|
||||
delete (window as unknown as { OffscreenCanvas?: unknown }).OffscreenCanvas;
|
||||
settleMockTerminalFontForCharSize();
|
||||
|
||||
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());
|
||||
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getMockBakedLetterSpacingPx()).toBeCloseTo(0, 5);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -231,6 +231,69 @@ export function forceTerminalFontRemeasure(
|
||||
terminal.options.fontFamily = fontFamily;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:Terminal 2026-07-05-12:40:
|
||||
FN-7603 recurrence #5 root cause (grounded in the installed `@xterm/xterm@5.5.0`
|
||||
source, see task doc key="xterm-source-audit" on FN-7603): xterm's
|
||||
`CharSizeService` picks its character-measurement strategy at construction time
|
||||
(inside `terminal.open()`) via `try { new OffscreenCanvasStrategy(optionsService) }
|
||||
catch { new DomFallbackStrategy(document, helperContainer, optionsService) }`.
|
||||
Whenever `OffscreenCanvas` + `CanvasRenderingContext2D.measureText()` reporting
|
||||
`fontBoundingBoxAscent`/`fontBoundingBoxDescent` are available — true on
|
||||
essentially every real modern mobile Safari/Chrome — the CANVAS strategy is
|
||||
chosen, and `dimensions.css.cell.width` (which feeds `FitAddon.fit()`'s column
|
||||
count AND `DomRenderer._setDefaultSpacing()`'s baked letter-spacing) is measured
|
||||
via Canvas 2D `ctx.measureText("W")`. Separately, `DomRenderer._setDefaultSpacing()`
|
||||
subtracts `WidthCache.get('W')` — an ENTIRELY SEPARATE, DOM-based measurement
|
||||
(`offsetWidth` of a hidden 32x-repeated-"W" span) — from that canvas-measured
|
||||
cell width to compute the compensating letter-spacing baked onto `.xterm-rows`.
|
||||
Those are two DIFFERENT browser text-rendering pipelines (Canvas 2D vs CSS/DOM
|
||||
layout) queried against the same font; real glyphs are painted 100% via DOM
|
||||
(`DomRenderer` never draws through canvas), so the only way for the baked
|
||||
spacing to reliably converge to the DOM's own natural (tight) glyph advance is
|
||||
for BOTH measurements to go through the SAME (DOM) pipeline. FN-7456/FN-7460/
|
||||
FN-7561/FN-7567 never touched which measurement strategy CharSizeService uses,
|
||||
only WHEN/how often it remeasures — so this Canvas-vs-DOM divergence survived
|
||||
all four prior fixes and can still bake a small-but-visible, non-zero,
|
||||
systematic inter-character gap on the very first mobile layout even after every
|
||||
prior remedy runs correctly. Force xterm onto the SAME (DOM) measurement
|
||||
pipeline CharSizeService already ships as its own fallback strategy by making
|
||||
`OffscreenCanvas` transiently unavailable for the synchronous duration of
|
||||
`terminal.open()`, so `CharSizeService`'s constructor try-block throws and it
|
||||
self-selects its own DOM-based `l` strategy — unifying `dimensions.css.cell.width`
|
||||
and `WidthCache.get('W')` onto one measurement pipeline instead of adding any
|
||||
hardcoded compensation. See `docs/solutions/ui-bugs/xterm-options-noop-remeasure-after-font-settle.md`
|
||||
recurrence #5 section.
|
||||
*/
|
||||
export function withDomBasedTerminalCharacterMeasurement<T>(fn: () => T): T {
|
||||
if (typeof window === "undefined" || !("OffscreenCanvas" in window)) {
|
||||
// Nothing to hide — CharSizeService will already fall back to its DOM
|
||||
// strategy on its own (e.g. older WebKit without OffscreenCanvas support).
|
||||
return fn();
|
||||
}
|
||||
|
||||
const descriptor = Object.getOwnPropertyDescriptor(window, "OffscreenCanvas");
|
||||
const originalValue = (window as unknown as Record<string, unknown>).OffscreenCanvas;
|
||||
|
||||
try {
|
||||
delete (window as unknown as Record<string, unknown>).OffscreenCanvas;
|
||||
} catch {
|
||||
// Some environments define OffscreenCanvas as non-configurable; nothing
|
||||
// we can safely do, so proceed without forcing the DOM strategy.
|
||||
return fn();
|
||||
}
|
||||
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
if (descriptor) {
|
||||
Object.defineProperty(window, "OffscreenCanvas", descriptor);
|
||||
} else if (originalValue !== undefined) {
|
||||
(window as unknown as Record<string, unknown>).OffscreenCanvas = originalValue;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function isObject(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user