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