fix(producer): normalize system-primary font stacks (#1857)

2026-09-10 22:20:14 +00:00 · 2026-07-02 00:06:30 -07:00
parent bf8b3d9796
commit e10e61ceb2
19 changed files with 6431 additions and 1731 deletions
@@ -192,11 +192,9 @@ describe("injectDeterministicFontFaces — failClosedFontFetch: true", () => {
    expect(result).toContain("data-hyperframes-deterministic-fonts");
  });

-  it("does NOT throw when font-family uses CSS var() references", async () => {
+  it("does NOT throw when font-family uses unresolved CSS var() references", async () => {
    const html = `<!doctype html><html><head><style>
-      :root { --ui-font: "Inter"; --vowel-font: "Montserrat"; }
-      body { font-family: var(--ui-font), sans-serif; }
-      h1 { font-family: var(--vowel-font), serif; }
+      body { font-family: var(--missing-font), sans-serif; }
    </style></head><body><h1>hello</h1></body></html>`;
    const result = await injectDeterministicFontFaces(html, {
      failClosedFontFetch: true,
@@ -205,6 +203,21 @@ describe("injectDeterministicFontFaces — failClosedFontFetch: true", () => {
    expect(result).toBe(html);
  });

+  it("resolves simple CSS var() font aliases when injecting deterministic fonts", async () => {
+    const html = `<!doctype html><html><head><style>
+      :root { --ui-font: "Inter"; --vowel-font: "Montserrat"; }
+      body { font-family: var(--ui-font), sans-serif; }
+      h1 { font-family: var(--vowel-font), serif; }
+    </style></head><body><h1>hello</h1></body></html>`;
+    const fetchImpl = (async () =>
+      new Response("/* no extra faces */", { status: 200 })) as unknown as typeof fetch;
+    const result = await injectDeterministicFontFaces(html, {
+      failClosedFontFetch: true,
+      fetchImpl,
+    });
+    expect(result).toContain("data-hyperframes-deterministic-fonts");
+  });
+
  it("still resolves concrete fonts alongside var() in mixed declarations", async () => {
    const html = `<!doctype html><html><head><style>
      body { font-family: var(--ui-font), "Inter", sans-serif; }
@@ -10,6 +10,7 @@ import {
  SYSTEM_FONT_SIZE_LIMIT,
 } from "@hyperframes/core/fonts/system-locator";
 import { parseHTML } from "linkedom";
+import postcss, { type AtRule, type Declaration, type Rule } from "postcss";
 import { EMBEDDED_FONT_DATA } from "./fontData.generated.js";
 import { fontToDataUri } from "./fontCompression.js";

@@ -61,9 +62,217 @@ export function parseFontFamilyValue(value: string): string[] {
    .filter((piece) => piece.length > 0);
 }

+function systemPrimaryReplacement(value: string, deterministicPrimary: string): string | null {
+  const families = parseFontFamilyValue(value);
+  if (families.length === 0) return null;
+  if (!GENERIC_FAMILIES.has(normalizeFamilyName(families[0]!))) return null;
+  return `${deterministicPrimary}, ${value.trim()}`;
+}
+
+function parseCssRoot(css: string): postcss.Root | null {
+  try {
+    return postcss.parse(css);
+  } catch {
+    return null;
+  }
+}
+
+function isFontFaceDeclaration(decl: Declaration): boolean {
+  const parent = decl.parent;
+  return parent?.type === "atrule" && (parent as AtRule).name.toLowerCase() === "font-face";
+}
+
+function normalizeCssDeclarations(root: postcss.Root, deterministicPrimary: string): boolean {
+  let changed = false;
+  root.walkDecls((decl) => {
+    if (decl.prop.startsWith("--")) {
+      const replacement = systemPrimaryReplacement(decl.value, deterministicPrimary);
+      if (!replacement) return;
+      decl.value = replacement;
+      changed = true;
+      return;
+    }
+
+    if (decl.prop.toLowerCase() !== "font-family") return;
+    if (isFontFaceDeclaration(decl)) {
+      return;
+    }
+    const replacement = systemPrimaryReplacement(decl.value, deterministicPrimary);
+    if (!replacement) return;
+    decl.value = replacement;
+    changed = true;
+  });
+
+  return changed;
+}
+
+function normalizeCssFontFamilyDeclarations(css: string, deterministicPrimary: string): string {
+  const root = parseCssRoot(css);
+  if (!root) return css;
+  const changed = normalizeCssDeclarations(root, deterministicPrimary);
+  return changed ? root.toString() : css;
+}
+
+function normalizeInlineStyleAttribute(style: string, deterministicPrimary: string): string {
+  const root = parseCssRoot(`*{${style}}`);
+  if (!root) return style;
+  const rule = root.first;
+  if (rule?.type !== "rule") return style;
+  const before = rule.toString();
+  normalizeCssDeclarations(root, deterministicPrimary);
+  if (rule.toString() === before) return style;
+  const serialized = ((rule as Rule).nodes ?? []).map((node) => node.toString()).join("; ");
+  return serialized.endsWith(";") ? serialized : `${serialized};`;
+}
+
+/**
+ * Import/generated HTML often uses host UI stacks such as
+ * `-apple-system, BlinkMacSystemFont, sans-serif` as a primary family. That is
+ * fine on the author's machine but not in distributed render workers, where
+ * host fonts differ by OS. Promote a bundled deterministic family to the
+ * primary slot while preserving the original stack as fallbacks.
+ */
+export function normalizeSystemFontPrimaryFamilies(
+  html: string,
+  deterministicPrimary = "Inter",
+): string {
+  const { document } = parseHTML(html);
+  let changed = false;
+
+  for (const styleEl of Array.from(document.querySelectorAll("style"))) {
+    const current = styleEl.textContent ?? "";
+    const next = normalizeCssFontFamilyDeclarations(current, deterministicPrimary);
+    if (next === current) continue;
+    styleEl.textContent = next;
+    changed = true;
+  }
+
+  for (const el of Array.from(document.querySelectorAll("[style]"))) {
+    const current = el.getAttribute("style") ?? "";
+    const next = normalizeInlineStyleAttribute(current, deterministicPrimary);
+    if (next === current) continue;
+    el.setAttribute("style", next);
+    changed = true;
+  }
+
+  for (const el of Array.from(document.querySelectorAll("[data-font-family]"))) {
+    const current = el.getAttribute("data-font-family") ?? "";
+    const next = systemPrimaryReplacement(current, deterministicPrimary);
+    if (!next) continue;
+    el.setAttribute("data-font-family", next);
+    changed = true;
+  }
+
+  return changed ? document.toString() : html;
+}
+
 /** Surfaces font-family is declared on in served HTML. */
 export type FontFamilySurface = "font-family" | "data-font-family";

+export type FontFamilyDeclaration = {
+  surface: FontFamilySurface;
+  declaration: string;
+  families: string[];
+};
+
+function collectCssCustomProperties(css: string, customProperties: Map<string, string>): void {
+  const root = parseCssRoot(css);
+  if (!root) return;
+  root.walkDecls((decl) => {
+    if (!decl.prop.startsWith("--")) return;
+    customProperties.set(decl.prop, decl.value);
+  });
+}
+
+function* iterateCssRootFontFamilyDeclarations(
+  root: postcss.Root,
+): Generator<FontFamilyDeclaration> {
+  const declarations: FontFamilyDeclaration[] = [];
+  root.walkDecls((decl) => {
+    if (decl.prop.toLowerCase() !== "font-family") return;
+    if (isFontFaceDeclaration(decl)) return;
+    const declaration = decl.value;
+    declarations.push({
+      surface: "font-family",
+      declaration,
+      families: parseFontFamilyValue(declaration),
+    });
+  });
+  yield* declarations;
+}
+
+function* iterateCssFontFamilyDeclarations(css: string): Generator<FontFamilyDeclaration> {
+  const root = parseCssRoot(css);
+  if (!root) return;
+  yield* iterateCssRootFontFamilyDeclarations(root);
+}
+
+function* iterateInlineStyleFontFamilyDeclarations(
+  style: string,
+): Generator<FontFamilyDeclaration> {
+  const root = parseCssRoot(`*{${style}}`);
+  if (!root) return;
+  yield* iterateCssRootFontFamilyDeclarations(root);
+}
+
+/**
+ * Collect simple CSS custom-property font aliases from style blocks and inline
+ * styles. CSS cascade is richer than this map, but for compiler-generated
+ * imports the common shape is `--font: Inter, sans-serif` paired with
+ * `font-family: var(--font)`.
+ */
+export function collectFontFamilyCustomProperties(html: string): Map<string, string> {
+  const { document } = parseHTML(html);
+  const customProperties = new Map<string, string>();
+
+  for (const styleEl of Array.from(document.querySelectorAll("style"))) {
+    collectCssCustomProperties(styleEl.textContent ?? "", customProperties);
+  }
+  for (const el of Array.from(document.querySelectorAll("[style]"))) {
+    collectCssCustomProperties(`*{${el.getAttribute("style") ?? ""}}`, customProperties);
+  }
+
+  return customProperties;
+}
+
+function primaryCssVariableName(value: string): string | null {
+  const trimmed = value.trim();
+  if (!trimmed.toLowerCase().startsWith("var(")) return null;
+
+  let depth = 0;
+  for (let index = 0; index < trimmed.length; index += 1) {
+    const char = trimmed[index];
+    if (char === "(") {
+      depth += 1;
+      continue;
+    }
+    if (char !== ")") continue;
+    depth -= 1;
+    if (depth !== 0) continue;
+
+    const varExpression = trimmed.slice(0, index + 1);
+    const inner = varExpression.slice(4, -1).trim();
+    const commaIndex = inner.indexOf(",");
+    const variableName = (commaIndex === -1 ? inner : inner.slice(0, commaIndex)).trim();
+    return /^--[A-Za-z0-9_-]+$/.test(variableName) ? variableName : null;
+  }
+
+  return null;
+}
+
+export function resolveFontFamilyDeclarationFamilies(
+  declaration: string,
+  customProperties: ReadonlyMap<string, string>,
+): string[] {
+  const families = parseFontFamilyValue(declaration);
+  const variableName = primaryCssVariableName(declaration);
+  if (!variableName) return families;
+
+  const resolved = customProperties.get(variableName);
+  if (!resolved) return families;
+  return [...parseFontFamilyValue(resolved), ...families.slice(1)];
+}
+
 /**
 * Iterate every font-family declaration in a compiled HTML document. Yields
 * each declaration's surface (CSS property vs HTML attribute), raw value,
@@ -72,16 +281,20 @@ export type FontFamilySurface = "font-family" | "data-font-family";
 */
 export function* iterateFontFamilyDeclarations(
  html: string,
-): Generator<{ surface: FontFamilySurface; declaration: string; families: string[] }, void, void> {
-  const sources: ReadonlyArray<readonly [RegExp, FontFamilySurface]> = [
-    [/font-family\s*:\s*([^;}{]+)[;}]?/gi, "font-family"],
-    [/data-font-family=["']([^"']+)["']/gi, "data-font-family"],
-  ];
-  for (const [regex, surface] of sources) {
-    for (const match of html.matchAll(regex)) {
-      const declaration = match[1] ?? "";
-      yield { surface, declaration, families: parseFontFamilyValue(declaration) };
-    }
+): Generator<FontFamilyDeclaration, void, void> {
+  const { document } = parseHTML(html);
+
+  for (const styleEl of Array.from(document.querySelectorAll("style"))) {
+    yield* iterateCssFontFamilyDeclarations(styleEl.textContent ?? "");
+  }
+
+  for (const el of Array.from(document.querySelectorAll("[style]"))) {
+    yield* iterateInlineStyleFontFamilyDeclarations(el.getAttribute("style") ?? "");
+  }
+
+  for (const el of Array.from(document.querySelectorAll("[data-font-family]"))) {
+    const declaration = el.getAttribute("data-font-family") ?? "";
+    yield { surface: "data-font-family", declaration, families: parseFontFamilyValue(declaration) };
  }
 }

@@ -204,8 +417,12 @@ function extractExistingFontFaces(html: string): Set<string> {

 function extractRequestedFontFamilies(html: string): Map<string, string> {
  const requested = new Map<string, string>();
-  for (const { families } of iterateFontFamilyDeclarations(html)) {
-    for (const originalCase of families) {
+  const customProperties = collectFontFamilyCustomProperties(html);
+  for (const { declaration } of iterateFontFamilyDeclarations(html)) {
+    for (const originalCase of resolveFontFamilyDeclarationFamilies(
+      declaration,
+      customProperties,
+    )) {
      const normalized = originalCase.toLowerCase();
      if (!normalized || GENERIC_FAMILIES.has(normalized)) continue;
      if (normalized.startsWith("var(")) continue;
@@ -15,6 +15,7 @@ import {
  localizeRemoteFontFaces,
  recompileWithResolutions,
 } from "./htmlCompiler.js";
+import { validateNoSystemFonts } from "./render/planValidation.js";

 // ── collectExternalAssets ──────────────────────────────────────────────────

@@ -678,6 +679,56 @@ describe("detectShaderTransitionUsage", () => {
  });
 });

+describe("system-primary font normalization", () => {
+  it("promotes Inter before system/generic primary stacks before distributed plan validation", async () => {
+    const projectDir = mkdtempSync(join(tmpdir(), "hf-system-primary-font-"));
+    writeFileSync(
+      join(projectDir, "index.html"),
+      `<!doctype html>
+<html>
+  <head>
+    <style>
+      :root { --system-font: -apple-system, BlinkMacSystemFont, sans-serif; }
+      body { font-family: -apple-system, BlinkMacSystemFont, sans-serif; }
+      .system-ui { font-family: system-ui, sans-serif; }
+      .var-font { font-family: var(--system-font), sans-serif; }
+      .deterministic { font-family: "Montserrat", system-ui, sans-serif; }
+    </style>
+  </head>
+  <body>
+    <div
+      data-composition-id="root"
+      data-width="640"
+      data-height="360"
+      data-duration="1"
+      data-font-family="ui-monospace, monospace"
+      style="--inline-system-font: system-ui, sans-serif; font-family: sans-serif"
+    >
+      <span class="var-font">Hello</span>
+    </div>
+  </body>
+</html>`,
+    );
+
+    const compiled = await compileForRender(projectDir, join(projectDir, "index.html"), projectDir);
+
+    expect(() => validateNoSystemFonts(compiled.html)).not.toThrow();
+    const compact = compiled.html.replace(/\s+/g, "");
+    expect(compact).toContain("--system-font:Inter,-apple-system,BlinkMacSystemFont,sans-serif");
+    expect(compact).toContain("font-family:Inter,-apple-system,BlinkMacSystemFont,sans-serif");
+    expect(compact).toContain("font-family:Inter,system-ui,sans-serif");
+    expect(compact).toContain("font-family:var(--system-font),sans-serif");
+    expect(compact).toContain('font-family:"Montserrat",system-ui,sans-serif');
+    expect(compact).toContain('data-font-family="Inter,ui-monospace,monospace"');
+    expect(compact).toContain('data-hyperframes-deterministic-fonts="true"');
+
+    const { document } = parseHTML(compiled.html);
+    const rootStyle = document.querySelector('[data-composition-id="root"]')?.getAttribute("style");
+    expect(rootStyle).toContain("--inline-system-font: Inter, system-ui, sans-serif");
+    expect(rootStyle).toContain("font-family: Inter, sans-serif");
+  });
+});
+
 describe("template-wrapped sub-composition media offsets", () => {
  function writeTemplateWrappedProject(
    hostAttrs: string,
@@ -42,7 +42,10 @@ import {
 } from "@hyperframes/engine";
 import { assertPublicHttpsUrl, downloadToTemp, isHttpUrl } from "../utils/urlDownloader.js";
 import type { Page } from "puppeteer-core";
-import { injectDeterministicFontFaces } from "./deterministicFonts.js";
+import {
+  injectDeterministicFontFaces,
+  normalizeSystemFontPrimaryFamilies,
+} from "./deterministicFonts.js";
 import { prepareAnimatedGifInputs } from "./animatedGifPrep.js";
 import { createStudioPositionSeekReapplyScript } from "@hyperframes/studio-server/manual-edits-render-script";
 import { defaultLogger, type ProducerLogger } from "../logger.js";
@@ -1548,16 +1551,17 @@ export async function compileForRender(
  const renderModeHints = detectRenderModeHints(sanitizedHtml);
  const hasShaderTransitions = detectShaderTransitionUsage(sanitizedHtml);

-  const coalescedHtml = await injectDeterministicFontFaces(
+  const normalizedFontHtml = normalizeSystemFontPrimaryFamilies(
    injectTextRenderingRule(
      coalesceHeadStylesAndBodyScripts(promoteCssImportsToLinkTags(sanitizedHtml)),
    ),
-    {
-      failClosedFontFetch: options.failClosedFontFetch === true,
-      allowSystemFontCapture: options.allowSystemFontCapture,
-    },
  );

+  const coalescedHtml = await injectDeterministicFontFaces(normalizedFontHtml, {
+    failClosedFontFetch: options.failClosedFontFetch === true,
+    allowSystemFontCapture: options.allowSystemFontCapture,
+  });
+
  // Download CDN scripts and inline them AFTER coalescing. This order matters:
  // coalesceHeadStylesAndBodyScripts merges inline scripts and appends them at
  // the end of <body>. If we inlined CDN scripts first, the GSAP library would
@@ -258,4 +258,28 @@ describe("validateNoSystemFonts", () => {
    const ok = `<style>body { font-family: "Inter", -apple-system, BlinkMacSystemFont, sans-serif; }</style>`;
    expect(() => validateNoSystemFonts(ok)).not.toThrow();
  });
+
+  it("resolves simple CSS var() primary aliases before rejecting system fonts", () => {
+    const offending = `<style>
+      :root { --ui-font: -apple-system, BlinkMacSystemFont, sans-serif; }
+      body { font-family: var(--ui-font), sans-serif; }
+    </style>`;
+    let caught: unknown;
+    try {
+      validateNoSystemFonts(offending);
+    } catch (err) {
+      caught = err;
+    }
+    expect(caught).toBeInstanceOf(PlanValidationError);
+    expect((caught as PlanValidationError).code).toBe(SYSTEM_FONT_USED);
+    expect((caught as Error).message).toContain(`"-apple-system"`);
+  });
+
+  it("accepts CSS var() primary aliases that resolve to deterministic fonts", () => {
+    const ok = `<style>
+      :root { --ui-font: "Inter", -apple-system, sans-serif; }
+      body { font-family: var(--ui-font), sans-serif; }
+    </style>`;
+    expect(() => validateNoSystemFonts(ok)).not.toThrow();
+  });
 });
@@ -6,7 +6,12 @@
 */

 import { BROWSER_GPU_NOT_SOFTWARE } from "@hyperframes/engine";
-import { GENERIC_FAMILIES, iterateFontFamilyDeclarations } from "../deterministicFonts.js";
+import {
+  collectFontFamilyCustomProperties,
+  GENERIC_FAMILIES,
+  iterateFontFamilyDeclarations,
+  resolveFontFamilyDeclarationFamilies,
+} from "../deterministicFonts.js";

 /**
 * Re-export the BROWSER_GPU_NOT_SOFTWARE code so distributed adapters and
@@ -118,12 +123,13 @@ export function validateNoGpuEncode(config: ValidateNoGpuEncodeInput): void {
 * @font-face injector and this validator scan the same regions.
 */
 export function validateNoSystemFonts(compiledHtml: string): void {
-  for (const { surface, declaration, families } of iterateFontFamilyDeclarations(compiledHtml)) {
+  const customProperties = collectFontFamilyCustomProperties(compiledHtml);
+  for (const { surface, declaration } of iterateFontFamilyDeclarations(compiledHtml)) {
+    const families = resolveFontFamilyDeclarationFamilies(declaration, customProperties);
    if (families.length === 0) continue;
    const primaryRaw = families[0]!;
-    // TODO(#1654): var() as primary bypasses this check — the resolved value
-    // could be system-ui or undefined. Consider resolving :root definitions
-    // or emitting a warning when primary is var().
+    // Unresolved var() primaries are left to the browser; resolved custom
+    // properties are checked above so common `--font: system-ui` aliases fail.
    if (!GENERIC_FAMILIES.has(primaryRaw.toLowerCase())) continue;
    throw new PlanValidationError(
      SYSTEM_FONT_USED,