From 513f714328e37e4b3eb8a00eae0453642d93c769 Mon Sep 17 00:00:00 2001 From: PathGao Date: Tue, 22 Sep 2026 12:37:57 +0800 Subject: [PATCH 1/2] fix(settings): quote the chosen font family so odd names still apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A font family the user picked was interpolated into CSS bare — `font-family: {settings.previewFont}, sans-serif` in the viewer, and the raw string handed to Monaco — which is valid only while the name happens to be a sequence of CSS identifiers. A family called `M+ 1c`, `04b03` or `Gill Sans (Body)` made the whole declaration invalid, so the browser dropped it and the element kept the font it inherited: the family is listed in the settings dropdown, selecting it changes nothing, and the failure is indistinguishable from the font not being installed. `fontFamilyValue` quotes the name and escapes `"` and `\` in it, leaves the CSS generics unquoted (quoted, the Linux defaults `Monospace` and `system-ui` would become families nobody has), and returns the fallback alone for a blank preference, which an empty `preview.font` key in localStorage can produce. Monaco is not a second implementation of this: it quotes on its own only when the name carries a space or a `+`, and skips its own quoting once the value contains a quote character. Measured in a WKWebView with the parent element at `40px Courier`: bare `M+ 1c`, `04b03`, `Gill Sans (Body)` and `Helvetica!` each computed to `Courier` at an identical 264.04px, while `Helvetica` rendered at 257.66px. Quoted, all four keep their declaration and fall through to `sans-serif` at 257.66px. --- scripts/editorOptionWiring.test.ts | 5 ++- scripts/fontFamily.test.ts | 60 ++++++++++++++++++++++++++++++ src/lib/MarkdownViewer.svelte | 3 +- src/lib/utils/editorOptions.ts | 3 +- src/lib/utils/fontFamily.ts | 55 +++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 scripts/fontFamily.test.ts create mode 100644 src/lib/utils/fontFamily.ts diff --git a/scripts/editorOptionWiring.test.ts b/scripts/editorOptionWiring.test.ts index f80d096e..cc1f65a4 100644 --- a/scripts/editorOptionWiring.test.ts +++ b/scripts/editorOptionWiring.test.ts @@ -574,7 +574,10 @@ function occurrenceHighlighting( callee: string, occurrencesHighlight: boolean, ): { occurrencesHighlight: string; selectionHighlight: boolean } { - const options = optionsPassedTo(callee, { occurrencesHighlight }); + // The full fixture, not the one setting under test: `editorOptionsFromSettings` + // reads the chosen font family now, and a stub missing it is a `string` field + // that is `undefined` — a shape the type forbids and production cannot hand it. + const options = optionsPassedTo(callee, { ...SETTINGS, occurrencesHighlight }); return { occurrencesHighlight: EditorOptions.occurrencesHighlight.validate(options.occurrencesHighlight), selectionHighlight: EditorOptions.selectionHighlight.validate(options.selectionHighlight), diff --git a/scripts/fontFamily.test.ts b/scripts/fontFamily.test.ts new file mode 100644 index 00000000..cd5d3467 --- /dev/null +++ b/scripts/fontFamily.test.ts @@ -0,0 +1,60 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { fontFamilyValue } from '../src/lib/utils/fontFamily.js'; +import { readSource } from './sourceTree.js'; + +const viewerSource = readSource(new URL('../src/lib/MarkdownViewer.svelte', import.meta.url)); +const editorOptionsSource = readSource(new URL('../src/lib/utils/editorOptions.ts', import.meta.url)); + +// The names in this test are the failure, not a sample of exotic input: each +// one is a real family that the settings dropdown offered and that selecting +// did nothing at all for, because bare interpolation made the declaration +// invalid and the browser dropped it whole. Measured in a WKWebView with the +// parent element at `40px Courier`: `M+ 1c`, `04b03`, `Gill Sans (Body)` and +// `Helvetica!` each computed to `Courier` at an identical width, while +// `Helvetica` in the same shape rendered. See #810. +test('a family name is quoted, whatever characters it carries', () => { + assert.equal(fontFamilyValue('Lato', 'sans-serif'), '"Lato", sans-serif'); + assert.equal(fontFamilyValue('M+ 1c', 'sans-serif'), '"M+ 1c", sans-serif'); + assert.equal(fontFamilyValue('04b03', 'sans-serif'), '"04b03", sans-serif'); + assert.equal(fontFamilyValue('Gill Sans (Body)', 'sans-serif'), '"Gill Sans (Body)", sans-serif'); + assert.equal(fontFamilyValue('Helvetica!', 'sans-serif'), '"Helvetica!", sans-serif'); +}); + +test('quotes and backslashes in a name cannot end the string early', () => { + assert.equal(fontFamilyValue('Say "Hi"', 'sans-serif'), '"Say \\"Hi\\"", sans-serif'); + assert.equal(fontFamilyValue('back\\slash', 'sans-serif'), '"back\\\\slash", sans-serif'); + // A family name reaches this from localStorage, which any window can write, + // so the value has to be unable to close the string and open a declaration + // of its own. + assert.equal(fontFamilyValue('x"; color: red; font-family: "y', 'sans-serif'), '"x\\"; color: red; font-family: \\"y", sans-serif'); +}); + +test('generic families stay unquoted, which is what keeps the Linux defaults working', () => { + // `defaultFontsFor('linux')` picks `Monospace` and `system-ui`. Quoted, both + // become a request for a family nobody has; CSS keywords are + // case-insensitive, so `Monospace` is the generic. + assert.equal(fontFamilyValue('Monospace', 'monospace'), 'Monospace, monospace'); + assert.equal(fontFamilyValue('system-ui', 'sans-serif'), 'system-ui, sans-serif'); + assert.equal(fontFamilyValue('ui-rounded', 'sans-serif'), 'ui-rounded, sans-serif'); +}); + +test('a blank preference is the fallback alone, not a leading comma', () => { + // `stringSetting` applies any non-null raw value, so an empty `preview.font` + // key lands as an empty family. Interpolated bare it produced + // `font-family: , sans-serif` — invalid, and dropped like the rest. + assert.equal(fontFamilyValue('', 'sans-serif'), 'sans-serif'); + assert.equal(fontFamilyValue(' ', 'monospace'), 'monospace'); + assert.equal(fontFamilyValue(' Lato ', 'sans-serif'), '"Lato", sans-serif'); +}); + +test('both places a chosen family reaches CSS go through the one helper', () => { + assert.match(viewerSource, /font-family: \{fontFamilyValue\(settings\.previewFont, 'sans-serif'\)\}/); + assert.match(editorOptionsSource, /fontFamily: fontFamilyValue\(settings\.editorFont, "monospace"\),/); + // The absence claim is the point of this one: a second bare interpolation of + // a font preference is the defect returning, and it cannot be observed by + // running the helper that does exist. + assert.doesNotMatch(viewerSource, /font-family: \{settings\./); + assert.doesNotMatch(editorOptionsSource, /fontFamily: settings\./); +}); diff --git a/src/lib/MarkdownViewer.svelte b/src/lib/MarkdownViewer.svelte index c408f713..16e5b369 100644 --- a/src/lib/MarkdownViewer.svelte +++ b/src/lib/MarkdownViewer.svelte @@ -45,6 +45,7 @@ import { type FoldHost, } from './utils/foldState.js'; import { routeDroppedFile, type DropPane } from './utils/fileDrop.js'; +import { fontFamilyValue } from './utils/fontFamily.js'; import { headingReference, preferredReferenceStyle } from './utils/headingReference.js'; import { findSourceLineRange, @@ -4049,7 +4050,7 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu if(e.key === 'Enter' || e.key === ' ') handleLinkClick(e as unknown as MouseEvent); }} tabindex="-1" - style="outline: none; font-family: {settings.previewFont}, sans-serif; font-size: {settings.previewFontSize}px; flex: 1; --preview-max-width: {previewContentWidth === null ? '100%' : `${previewContentWidth}px`};"> + style="outline: none; font-family: {fontFamilyValue(settings.previewFont, 'sans-serif')}; font-size: {settings.previewFontSize}px; flex: 1; --preview-max-width: {previewContentWidth === null ? '100%' : `${previewContentWidth}px`};"> {#if frontMatterInfo.exists}
`, so quoting is the whole + * fix; `"` and `\` inside the name are escaped, which is what makes the value + * unable to terminate the string early and inject further declarations. + * + * Monaco is not a second implementation of this. It quotes a family of its own + * accord (`BareFontInfo._wrapInQuotes`) but only when the name carries a space + * or a `+`, so `04b03` reaches the stylesheet bare — and it skips its own + * quoting entirely once the value contains a quote character, which is what + * lets this one stand in front of it. + */ +export function fontFamilyValue(name: string, fallback: string): string { + const family = name.trim(); + if (family === '') return fallback; + if (GENERIC_FAMILIES.has(family.toLowerCase())) return `${family}, ${fallback}`; + return `"${family.replace(/["\\]/g, '\\$&')}", ${fallback}`; +} From aaed3ae7ce978b56c3dcc3c20ae93adb344aa8d6 Mon Sep 17 00:00:00 2001 From: PathGao Date: Tue, 22 Sep 2026 13:08:50 +0800 Subject: [PATCH 2/2] fix(settings): the generics exemption is the grammar's list, not a lookalike list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fangsong` and `emoji` were in it. Neither is a bare generic in CSS Fonts 4: the script-specific generics are spelled `generic(fangsong)`, and `emoji` is not in the grammar at all. `fangsong` is the one that mattered — macOS ships `FangSong` and `STFangsong`, so exempting the name handed a user who picked their 仿宋 face an unquoted family name, which an engine that does treat the bare word as a keyword resolves to a generic instead of to their font. What is left is `` plus `` from §2.1.2. The comment now also says which names must never be added: a family named after a CSS-wide keyword has to be quoted per §2.1.1, and bare `inherit` or `default` is rejected outright — measured in a WKWebView, both drop the declaration while the quoted form is accepted. --- scripts/fontFamily.test.ts | 18 ++++++++++++++++++ src/lib/utils/fontFamily.ts | 20 ++++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/scripts/fontFamily.test.ts b/scripts/fontFamily.test.ts index cd5d3467..b191a78b 100644 --- a/scripts/fontFamily.test.ts +++ b/scripts/fontFamily.test.ts @@ -40,6 +40,24 @@ test('generic families stay unquoted, which is what keeps the Linux defaults wor assert.equal(fontFamilyValue('ui-rounded', 'sans-serif'), 'ui-rounded, sans-serif'); }); +test('the generics table holds the grammar and nothing that only looks like it', () => { + // `generic(fangsong)` is how CSS Fonts 4 §2.1.2 spells the script-specific + // generics, so the bare word is an ordinary family name — and macOS ships two + // real ones. Exempting it would emit a user's 仿宋 face unquoted, which an + // engine that does treat the bare word as a keyword resolves to a generic. + assert.equal(fontFamilyValue('FangSong', 'serif'), '"FangSong", serif'); + assert.equal(fontFamilyValue('fangsong', 'serif'), '"fangsong", serif'); + assert.equal(fontFamilyValue('emoji', 'sans-serif'), '"emoji", sans-serif'); + // The other direction, and the one that must never be "fixed" by adding an + // entry: §2.1.1 requires a family named after a CSS-wide keyword to be + // quoted. Measured in a WKWebView, `font-family: inherit, serif` and + // `font-family: default, serif` are rejected outright, while the quoted form + // is accepted. + for (const reserved of ['inherit', 'initial', 'unset', 'revert', 'revert-layer', 'default']) { + assert.equal(fontFamilyValue(reserved, 'serif'), `"${reserved}", serif`); + } +}); + test('a blank preference is the fallback alone, not a leading comma', () => { // `stringSetting` applies any non-null raw value, so an empty `preview.font` // key lands as an empty family. Interpolated bare it produced diff --git a/src/lib/utils/fontFamily.ts b/src/lib/utils/fontFamily.ts index a83a7786..c510c9b8 100644 --- a/src/lib/utils/fontFamily.ts +++ b/src/lib/utils/fontFamily.ts @@ -6,6 +6,24 @@ * case-insensitive because CSS keywords are, which is also what keeps the * Linux defaults working: `defaultFontsFor('linux')` picks `Monospace` and * `system-ui`, and `Monospace` is the generic, spelled with a capital M. + * + * The list is `` plus `` from + * CSS Fonts 4 §2.1.2, and nothing else. Two names that look like they belong + * are deliberately absent: + * + * - `fangsong` and the other script-specific generics are spelled + * `generic(fangsong)` now, so the bare word is an ordinary family name — + * and a real one: macOS ships `FangSong` and `STFangsong`. Exempting it + * would hand a user who picked their 仿宋 face an unquoted name, which an + * engine that still treats the bare word as a keyword resolves to a generic + * instead of to their font. + * - `emoji` is not in the grammar at all. + * + * The CSS-wide keywords — `inherit`, `initial`, `unset`, `revert`, + * `revert-layer` — and `default` must never be added here. §2.1.1 requires a + * family with one of those names to be quoted, and measurement agrees: bare, + * the whole declaration is rejected, which is the defect this module exists to + * fix rather than a case it should reintroduce. */ const GENERIC_FAMILIES = new Set([ 'serif', @@ -19,8 +37,6 @@ const GENERIC_FAMILIES = new Set([ 'ui-monospace', 'ui-rounded', 'math', - 'emoji', - 'fangsong', ]); /**