From d6ec7570268f501eaaad9160482f728bd08ab477 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 10:39:17 +0200 Subject: [PATCH] fix(app): keep malformed font settings from breaking the UI Unfinished CSS strings in font preferences consumed the shared theme stylesheet. Reject incomplete syntax at the settings boundary, recover persisted malformed values through the existing default path, and restore rejected edits in the font field. --- .../browser/appearance-font-family.spec.ts | 48 +++++++++++++++++++ .../src/hooks/use-settings/storage.test.ts | 32 +++++++++++++ .../app/src/hooks/use-settings/storage.ts | 19 +++++++- .../appearance/appearance-section.tsx | 14 ++++-- 4 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 packages/app/e2e/browser/appearance-font-family.spec.ts diff --git a/packages/app/e2e/browser/appearance-font-family.spec.ts b/packages/app/e2e/browser/appearance-font-family.spec.ts new file mode 100644 index 0000000000..6ec8600017 --- /dev/null +++ b/packages/app/e2e/browser/appearance-font-family.spec.ts @@ -0,0 +1,48 @@ +import { expect, test } from "../support/fixtures"; +import type { Page } from "@playwright/test"; +import { openSettingsSection } from "../support/helpers/settings"; + +async function openAppearance(page: Page) { + await page.goto("/settings"); + await openSettingsSection(page, "appearance"); +} + +async function setFontFamily(page: Page, name: string, family: string) { + const input = page.getByRole("textbox", { name }); + await input.fill(family); + await input.press("Tab"); +} + +for (const name of ["Code font family", "Interface font family"]) { + test(`keeps settings usable after an unfinished quote in ${name}`, async ({ page }, testInfo) => { + await openAppearance(page); + await setFontFamily(page, name, "'Fira Code', monospace"); + await expect(page.getByRole("textbox", { name })).toHaveValue("'Fira Code', monospace"); + + await setFontFamily(page, name, "'foo"); + await expect(page.getByRole("textbox", { name })).toHaveValue("'Fira Code', monospace"); + await expect(page.getByRole("button", { name: "Appearance", exact: true })).toBeInViewport(); + await expect(page.getByText("Theme", { exact: true }).first()).toHaveCSS("font-size", "12px"); + + await page.screenshot({ path: testInfo.outputPath("rejected-font.png") }); + await page.reload(); + await expect(page.getByRole("textbox", { name })).toHaveValue("'Fira Code', monospace"); + await expect(page.getByRole("button", { name: "Appearance", exact: true })).toBeInViewport(); + await setFontFamily(page, name, ""); + await expect(page.getByRole("textbox", { name })).toHaveValue(""); + }); +} + +test("opens settings with defaults when a malformed font was already saved", async ({ page }) => { + await page.addInitScript(() => { + localStorage.setItem( + "@paseo:app-settings", + JSON.stringify({ monoFontFamily: "'foo", uiFontFamily: '"foo' }), + ); + }); + await openAppearance(page); + await expect(page.getByRole("textbox", { name: "Code font family" })).toHaveValue(""); + await expect(page.getByRole("textbox", { name: "Interface font family" })).toHaveValue(""); + await expect(page.getByRole("button", { name: "Appearance", exact: true })).toBeInViewport(); + await expect(page.getByText("Theme", { exact: true }).first()).toHaveCSS("font-size", "12px"); +}); diff --git a/packages/app/src/hooks/use-settings/storage.test.ts b/packages/app/src/hooks/use-settings/storage.test.ts index d396f26d9a..6d9c8d4c7c 100644 --- a/packages/app/src/hooks/use-settings/storage.test.ts +++ b/packages/app/src/hooks/use-settings/storage.test.ts @@ -15,6 +15,7 @@ import { parseClampedFontSize, parseTerminalScrollbackLines, resolveContentMaxWidth, + sanitizeFontFamily, saveAppSettings, type SettingsDeps, } from "./storage"; @@ -1094,3 +1095,34 @@ describe("content max width", () => { expect((await load("wide")).contentMaxWidth).toBeNull(); }); }); + +describe("font family CSS safety", () => { + it.each(["'foo", '"foo', "Menlo, 'foo", "foo\\", "foo/*", "foo(", "foo["])( + "rejects unfinished CSS syntax: %s", + (value) => expect(sanitizeFontFamily(value)).toBeNull(), + ); + + it.each([ + "Menlo", + "'Fira Code', monospace", + '"Fira Code", monospace', + "'Font\\'s name'", + "Font\\ Name", + "'Font (Mono)'", + "'Font [Mono]'", + ])("preserves complete font families: %s", (value) => + expect(sanitizeFontFamily(value)).toBe(value), + ); + + it.each(["uiFontFamily", "monoFontFamily"])( + "recovers a persisted unmatched quote in %s", + async (field) => { + const deps = makeDeps({ + storage: createInMemoryKeyValueStorage({ + [APP_SETTINGS_KEY]: JSON.stringify({ [field]: "'foo" }), + }), + }); + expect(await loadAppSettingsFromStorage(deps)).toMatchObject({ [field]: "" }); + }, + ); +}); diff --git a/packages/app/src/hooks/use-settings/storage.ts b/packages/app/src/hooks/use-settings/storage.ts index 6c44546668..54ee6b10bc 100644 --- a/packages/app/src/hooks/use-settings/storage.ts +++ b/packages/app/src/hooks/use-settings/storage.ts @@ -522,7 +522,24 @@ export function sanitizeFontFamily(value: unknown): string | null { if ([...trimmed].some((char) => char.charCodeAt(0) <= 0x1f)) { return null; // control chars would corrupt the font-family string } - return trimmed; // quotes/commas are legit in stacks + // Unistyles serializes theme values into one shared stylesheet. An unfinished + // string, escape, comment, or block consumes the declarations that follow. + let quote: string | null = null; + let escaped = false; + for (const char of trimmed) { + if (escaped) { + escaped = false; + } else if (char === "\\") { + escaped = true; + } else if (quote !== null) { + if (char === quote) quote = null; + } else if (char === "'" || char === '"') { + quote = char; + } else if (/[()[\]/*]/.test(char)) { + return null; + } + } + return quote === null && !escaped ? trimmed : null; } async function loadLegacyDesktopSettingsFromStorage(storage: KeyValueStorage): Promise<{ diff --git a/packages/app/src/screens/settings/appearance/appearance-section.tsx b/packages/app/src/screens/settings/appearance/appearance-section.tsx index 6e98819323..2dfea09fd0 100644 --- a/packages/app/src/screens/settings/appearance/appearance-section.tsx +++ b/packages/app/src/screens/settings/appearance/appearance-section.tsx @@ -241,7 +241,7 @@ interface FontFamilyRowProps { draft: string; withBorder: boolean; onChangeDraft: (value: string) => void; - onCommit: (value: string) => void; + onCommit: (value: string) => string; } function FontFamilyRow({ @@ -255,13 +255,16 @@ function FontFamilyRow({ onChangeDraft, onCommit, }: FontFamilyRowProps) { + const inputRef = useRef(null); const handleCommit = useCallback(() => { - onCommit(draft); + const committed = onCommit(draft); + inputRef.current?.replaceText(committed); }, [draft, onCommit]); // Resync from the committed value when it changes elsewhere. useEffect(() => { onChangeDraft(value); + inputRef.current?.replaceText(value); // Only resync on external value changes, not on local keystrokes. // eslint-disable-next-line react-hooks/exhaustive-deps }, [value]); @@ -273,6 +276,7 @@ function FontFamilyRow({ {hint}