diff --git a/desktop/src/app/fontCandidates.test.ts b/desktop/src/app/fontCandidates.test.ts index 97fa9e8cf..870035007 100644 --- a/desktop/src/app/fontCandidates.test.ts +++ b/desktop/src/app/fontCandidates.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi, type Mock } from "vitest"; import { appendFamily, @@ -7,60 +7,145 @@ import { isFontAvailable, } from "./fontCandidates"; -afterEach(() => { - vi.unstubAllGlobals(); - Reflect.deleteProperty(document, "fonts"); -}); +/** + * A stand-in for the engine's font matching, faithful to what Edge/WebView2 + * 153.0.4234 was measured doing: a `local()` lookup resolves for a family the + * machine has, and rejects with NetworkError for one it does not. + * + * The previous suite replaced `document.fonts.check()` with a fake that encoded + * the *assumption* it reports absence. That is how a picker which listed every + * family on the machine, installed or not, stayed green. + */ +function stubFontMatching( + installed: readonly string[], + options: { refuse?: boolean } = {}, +): string[] { + const sources: string[] = []; + class FakeFontFace { + family = ""; + + constructor(_name: string, source: string) { + // An engine can refuse the lookup itself, e.g. a malformed source. + if (options.refuse) throw new DOMException("refused", "SyntaxError"); + sources.push(source); + this.family = /^local\("(.*)"\)$/.exec(source)?.[1] ?? ""; + } -function stubFonts(installed: string[]): void { + load(): Promise { + return installed.includes(this.family) + ? Promise.resolve(this) + : Promise.reject( + new DOMException(`${this.family} is not available`, "NetworkError"), + ); + } + } + vi.stubGlobal("FontFace", FakeFontFace); + return sources; +} + +/** The engine's own answer about availability, for the record: true for anything. */ +function stubCheckAlwaysTrue(): Mock<() => boolean> { + const check = vi.fn(() => true); Object.defineProperty(document, "fonts", { configurable: true, - value: { - check: (font: string) => - installed.some((family) => font.includes(`"${family}"`)), - }, + value: { check }, }); + return check; } +afterEach(() => { + vi.unstubAllGlobals(); + Reflect.deleteProperty(document, "fonts"); +}); + describe("isFontAvailable", () => { - it("reports nothing when the Font Loading API is absent", () => { - // jsdom and older WebViews have no document.fonts. Claiming every family - // exists there would offer the user settings that do nothing. - Reflect.deleteProperty(document, "fonts"); - expect(isFontAvailable("Inter")).toBe(false); + it("does not consult document.fonts.check, which reports true for absent families", async () => { + // Measured on Edge/WebView2 153.0.4234 — the engine Tauri uses on Windows — + // where check() answered true for all 39 families in a sweep that included + // `__Absent Font 12345__`: an unmatched family still renders through the + // fallback. Asking it offers every candidate on every machine. + const check = stubCheckAlwaysTrue(); + stubFontMatching(["Inter"]); + + expect(check()).toBe(true); // the engine's answer, for the record + check.mockClear(); + + expect(await isFontAvailable("__Absent Font 12345__")).toBe(false); + expect(check).not.toHaveBeenCalled(); }); - it("asks the document rather than guessing", () => { - stubFonts(["Inter"]); - expect(isFontAvailable("Inter")).toBe(true); - expect(isFontAvailable("Definitely Not Installed")).toBe(false); + it("resolves a family the machine has", async () => { + stubFontMatching(["Inter"]); + + expect(await isFontAvailable("Inter")).toBe(true); }); - it("survives a family name that would break the shorthand", () => { + it("asks the engine about the family it was given", async () => { + const sources = stubFontMatching(["Inter"]); + + await isFontAvailable("Inter"); + + expect(sources).toEqual(['local("Inter")']); + }); + + it("reports a family the machine lacks as unavailable", async () => { + stubFontMatching(["Inter"]); + + expect(await isFontAvailable("Definitely Not Installed")).toBe(false); + }); + + it("survives a family name that would end the lookup string", async () => { + const sources = stubFontMatching(["broken"]); + + expect(await isFontAvailable('bro"ken')).toBe(true); + expect(sources).toEqual(['local("broken")']); + }); + + it("reports nothing when the engine has no FontFace", async () => { + // jsdom, and a WebView without the Font Loading API: claim nothing rather + // than offer the user settings that do nothing. + vi.stubGlobal("FontFace", undefined); + + expect(await isFontAvailable("Inter")).toBe(false); + }); + + it("reports nothing when the engine refuses the lookup", async () => { + stubFontMatching(["Inter"], { refuse: true }); + + expect(await isFontAvailable("Inter")).toBe(false); + }); + + it("leaves document.fonts untouched", async () => { + // Probing must not register anything: the face is loaded to ask a question, + // not to be used for rendering. + const add = vi.fn(); Object.defineProperty(document, "fonts", { configurable: true, - value: { - check: () => { - throw new SyntaxError("bad font shorthand"); - }, - }, + value: { add }, }); - expect(isFontAvailable('bro"ken')).toBe(false); + stubFontMatching(["Inter"]); + + await isFontAvailable("Inter"); + + expect(add).not.toHaveBeenCalled(); }); }); describe("availableFontCandidates", () => { - it("offers only what is installed", () => { - stubFonts(["Inter", "PingFang SC"]); - expect(availableFontCandidates().map((c) => c.family)).toEqual([ + it("offers only what the machine has, in declaration order", async () => { + stubFontMatching(["PingFang SC", "Consolas", "Inter"]); + + expect((await availableFontCandidates()).map((c) => c.family)).toEqual([ "Inter", + "Consolas", "PingFang SC", ]); }); - it("returns nothing rather than the whole list when probing is impossible", () => { - Reflect.deleteProperty(document, "fonts"); - expect(availableFontCandidates()).toEqual([]); + it("returns nothing rather than the whole list when probing is impossible", async () => { + vi.stubGlobal("FontFace", undefined); + + expect(await availableFontCandidates()).toEqual([]); }); it("covers each group so the picker is useful on any platform", () => { diff --git a/desktop/src/app/fontCandidates.ts b/desktop/src/app/fontCandidates.ts index ab760f6a0..e7f6b19de 100644 --- a/desktop/src/app/fontCandidates.ts +++ b/desktop/src/app/fontCandidates.ts @@ -4,9 +4,25 @@ * A plain dropdown of font names would be a hardcoded guess: the list that is * right on a Windows box with Microsoft YaHei is wrong on a Mac with PingFang, * and offering a family the system lacks produces a setting that silently does - * nothing. `document.fonts.check()` answers the question directly, so the - * candidates below are only ever *suggestions* — the UI shows the survivors - * and still accepts free text for anything not listed. + * nothing. The candidates below are therefore only ever *suggestions* — the UI + * shows the survivors and still accepts free text for anything not listed. + * + * Presence comes from a `local()` lookup, not from `document.fonts.check()`. + * That call is the obvious API and it does not answer this question: it reports + * whether the text *can* be rendered, and a family the machine lacks still + * renders through the fallback. Measured on Edge/WebView2 153.0.4234 — the + * engine Tauri uses on Windows — `check()` answered true for all 39 families in + * a sweep that included `__Absent Font 12345__`, so every candidate survived + * the filter and the picker offered fonts the machine did not have. `local()` + * goes through font matching instead: it rejected every family with no font on + * the machine (Roboto, Helvetica, PingFang SC, …) while resolving the rest, and + * agreed with the Windows font registry on every name it was asked about. + * `queryLocalFonts()` would enumerate the system list directly, but it needs a + * user gesture and a permission grant, so it cannot back a picker that is + * populated when the settings page opens. + * + * The probe is asynchronous, and an engine that cannot probe reports nothing + * rather than pretending every family exists. */ export interface FontCandidate { @@ -21,7 +37,11 @@ export interface FontCandidate { */ export const FONT_CANDIDATES: readonly FontCandidate[] = [ { family: "Inter", group: "Interface" }, - { family: "Segoe UI Variable", group: "Interface" }, + // Windows 11 ships its UI face as one variable font, and the system exposes + // its optical sizes as separate families; the bare "Segoe UI Variable" is not + // one of them. Measured on this engine: a local() lookup of the bare name + // fails, while "… Text" — the size used at body text — resolves. + { family: "Segoe UI Variable Text", group: "Interface" }, { family: "Segoe UI", group: "Interface" }, { family: "SF Pro Text", group: "Interface" }, { family: "Helvetica Neue", group: "Interface" }, @@ -39,26 +59,44 @@ export const FONT_CANDIDATES: readonly FontCandidate[] = [ { family: "Hiragino Sans GB", group: "CJK" }, ]; +/** Name for the throwaway face a probe loads. It is never registered. */ +const PROBE_FACE_NAME = "deepcode-font-probe"; + +/** Quote a family for a `local()` source, dropping characters that would end it. */ +function quote(family: string): string { + return `"${family.replaceAll('"', "").replaceAll("\\", "")}"`; +} + /** - * Whether `family` resolves on this machine. + * Whether the engine resolves `family` by name. * - * `document.fonts.check` needs a full font shorthand and throws on a malformed - * one, so the family is quoted and the call is guarded. An environment without - * the Font Loading API (jsdom, an old WebView) reports nothing rather than - * pretending every family exists. + * `local()` is the lookup that fails for a family the machine lacks, so a + * failed load is the negative answer. An engine without `FontFace` — or one + * that refuses local lookups — reports nothing rather than claiming every + * family exists. The face is never added to `document.fonts`, so probing + * leaves no trace. */ -export function isFontAvailable(family: string): boolean { - if (typeof document === "undefined" || !document.fonts?.check) return false; +export async function isFontAvailable(family: string): Promise { + if (typeof FontFace === "undefined") return false; try { - return document.fonts.check(`12px "${family.replaceAll('"', "")}"`); + await new FontFace(PROBE_FACE_NAME, `local(${quote(family)})`).load(); + return true; } catch { return false; } } /** The candidates present on this machine, in declaration order. */ -export function availableFontCandidates(): FontCandidate[] { - return FONT_CANDIDATES.filter((candidate) => isFontAvailable(candidate.family)); +export async function availableFontCandidates(): Promise { + const verdicts = await Promise.all( + FONT_CANDIDATES.map(async (candidate) => ({ + candidate, + present: await isFontAvailable(candidate.family), + })), + ); + return verdicts + .filter((verdict) => verdict.present) + .map((verdict) => verdict.candidate); } /** Append `family` to a comma-separated list, ignoring duplicates. */ diff --git a/desktop/src/features/settings/AppearanceSettings.tsx b/desktop/src/features/settings/AppearanceSettings.tsx index c5d9ff5ac..be004ba8f 100644 --- a/desktop/src/features/settings/AppearanceSettings.tsx +++ b/desktop/src/features/settings/AppearanceSettings.tsx @@ -1,5 +1,5 @@ import { Monitor, Moon, Sun } from "lucide-react"; -import { useMemo, useId, useState } from "react"; +import { useEffect, useId, useState } from "react"; import { APPEARANCE_DEFAULTS, @@ -11,6 +11,7 @@ import { import { appendFamily, availableFontCandidates, + type FontCandidate, } from "../../app/fontCandidates"; import { useAppearance } from "../../app/useAppearance"; import { parseVsCodeTheme, ThemeImportError } from "../../app/importedTheme"; @@ -82,9 +83,13 @@ export function AppearanceSettings() { const { t } = useTranslation(); const fieldId = useId(); const [importError, setImportError] = useState(null); - // Probed once per mount: the set of installed fonts does not change while - // the settings page is open. - const installed = useMemo(() => availableFontCandidates(), []); + // The installed set is probed once per mount — it does not change while the + // settings page is open — and the probe is asynchronous because the engine's + // only reliable answer, a `local()` font load, is. + const [installed, setInstalled] = useState([]); + useEffect(() => { + void availableFontCandidates().then(setInstalled); + }, []); const isDefault = APPEARANCE_SETTINGS.every( (setting) => appearance[setting.key] === APPEARANCE_DEFAULTS[setting.key], ) && appearance.importedTheme === null;