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..b191a78b --- /dev/null +++ b/scripts/fontFamily.test.ts @@ -0,0 +1,78 @@ +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('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 + // `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}
` 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', + 'sans-serif', + 'monospace', + 'cursive', + 'fantasy', + 'system-ui', + 'ui-serif', + 'ui-sans-serif', + 'ui-monospace', + 'ui-rounded', + 'math', +]); + +/** + * A `font-family` declaration value carrying the family the user picked in + * settings, followed by `fallback`. + * + * The name used to be interpolated bare — `font-family: {settings.previewFont}, + * sans-serif` in the viewer, and the raw string handed to Monaco — which is + * valid CSS only while the name happens to be a sequence of 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 failure is silent and looks like the font not existing: the + * family is in the settings dropdown, selecting it changes nothing, and the + * user has no way to tell those two apart. Reported in #810 against fonts + * activated by a third-party font manager. + * + * A quoted string is always a valid ``, 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}`; +}