mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(compiler): skip CSS var() in font resolver (#1655)
* fix(compiler): skip CSS var() in font resolver — fixes FONT_FETCH_FAILED on distributed renders The font scanner treated `var(--ui-font)` as a literal font family name, causing fail-closed distributed renders to throw FONT_FETCH_FAILED for any composition using CSS custom properties in font-family declarations. CSS var() expressions resolve at browser paint time, not at compile time. The regex-based font scanner cannot resolve them statically — skip them and let headless Chrome handle variable substitution during render. Closes #1654 — Miga * test(regression): add distributed css-var-fonts fixture Regression test for compositions that use CSS custom properties in font-family declarations. Exercises the var() skip guard in extractRequestedFontFamilies() under the distributed renderer's fail-closed font resolution path. Baseline needs to be generated on first CI run with --update. — Miga * fix(compiler): address review feedback — mixed declaration test + validator TODO Add unit test verifying concrete fonts alongside var() in mixed declarations still get resolved (non-aggression pin). Add TODO(#1654) in validateNoSystemFonts for the var()-as-primary gap flagged by both reviewers. — Miga * fix(test): correct stale 4xx fail-closed test expectations The 4xx tests expected no throw, but that was the contract before #1255 added the system font capture path (Path 3). Post-#1255, a font that gets 4xx from Google Fonts AND isn't a bundled alias AND has no system font IS genuinely unresolvable — fail-closed mode should throw. The 4xx distinction still matters at the fetch level (no retry, treated as deterministic "not served"), but at the final unresolved check, a completely unresolvable font must throw regardless of the HTTP status that caused the Google Fonts path to return empty. Updated tests to match the actual contract: 4xx + unresolvable = throw. Also set allowSystemFontCapture: false to match how distributed renders (plan.ts:799) actually call the function. — Miga --------- Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
This commit is contained in:
co-authored by
Miguel Ángel
parent
8c13a72696
commit
468f7ec35b
@@ -109,26 +109,40 @@ describe("injectDeterministicFontFaces — failClosedFontFetch: true", () => {
|
||||
expect((caught as Error).message).toContain("simulated network failure");
|
||||
});
|
||||
|
||||
it("does NOT throw on a 4xx response — 4xx means 'Google Fonts does not serve this family', a deterministic answer", async () => {
|
||||
// Google Fonts returns HTTP 400 for non-Google families like
|
||||
// "Segoe UI", "Arial", "Futura". Same response on every retry, so it
|
||||
// doesn't violate the byte-identical-retry contract — the render
|
||||
// falls back to embedded faces / the composition's font-family chain.
|
||||
// No FONT_FETCH_FAILED.
|
||||
const result = await injectDeterministicFontFaces(HTML_REQUESTING_UNRESOLVED_FONT, {
|
||||
failClosedFontFetch: true,
|
||||
fetchImpl: makeHttp400Fetch(),
|
||||
});
|
||||
// No @font-face was injected (no faces returned), but the call resolves.
|
||||
expect(result.includes("data-hyperframes-deterministic-fonts")).toBe(false);
|
||||
it("throws on a 4xx response when font is completely unresolvable", async () => {
|
||||
// 4xx from Google Fonts is deterministic ("this family isn't served"),
|
||||
// so it doesn't throw at the *fetch* level. But the font still ends up
|
||||
// unresolvable (no alias, no Google Fonts, no system capture) which IS
|
||||
// a fail-closed error — the render would use a fallback font, producing
|
||||
// non-deterministic output across machines.
|
||||
let caught: unknown;
|
||||
try {
|
||||
await injectDeterministicFontFaces(HTML_REQUESTING_UNRESOLVED_FONT, {
|
||||
failClosedFontFetch: true,
|
||||
allowSystemFontCapture: false,
|
||||
fetchImpl: makeHttp400Fetch(),
|
||||
});
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(FontFetchError);
|
||||
expect((caught as FontFetchError).code).toBe(FONT_FETCH_FAILED);
|
||||
expect((caught as FontFetchError).familyName).toBe("NotARealFontFamilyForTest");
|
||||
});
|
||||
|
||||
it("does NOT throw on a 404 response either — same reasoning as 4xx generally", async () => {
|
||||
const result = await injectDeterministicFontFaces(HTML_REQUESTING_UNRESOLVED_FONT, {
|
||||
failClosedFontFetch: true,
|
||||
fetchImpl: makeHttp404Fetch(),
|
||||
});
|
||||
expect(result.includes("data-hyperframes-deterministic-fonts")).toBe(false);
|
||||
it("throws on a 404 response when font is completely unresolvable", async () => {
|
||||
let caught: unknown;
|
||||
try {
|
||||
await injectDeterministicFontFaces(HTML_REQUESTING_UNRESOLVED_FONT, {
|
||||
failClosedFontFetch: true,
|
||||
allowSystemFontCapture: false,
|
||||
fetchImpl: makeHttp404Fetch(),
|
||||
});
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(FontFetchError);
|
||||
expect((caught as FontFetchError).code).toBe(FONT_FETCH_FAILED);
|
||||
});
|
||||
|
||||
it("throws FontFetchError on a 5xx response — non-deterministic, could differ on retry", async () => {
|
||||
@@ -178,6 +192,32 @@ describe("injectDeterministicFontFaces — failClosedFontFetch: true", () => {
|
||||
expect(result).toContain("data-hyperframes-deterministic-fonts");
|
||||
});
|
||||
|
||||
it("does NOT throw when font-family uses 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; }
|
||||
</style></head><body><h1>hello</h1></body></html>`;
|
||||
const result = await injectDeterministicFontFaces(html, {
|
||||
failClosedFontFetch: true,
|
||||
fetchImpl: makeFailingFetch(),
|
||||
});
|
||||
expect(result).toBe(html);
|
||||
});
|
||||
|
||||
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; }
|
||||
</style></head><body><p>mixed</p></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("does NOT throw when the HTML requests no fonts at all", async () => {
|
||||
const html = `<!doctype html><html><body><p>no fonts</p></body></html>`;
|
||||
const result = await injectDeterministicFontFaces(html, {
|
||||
|
||||
@@ -208,6 +208,7 @@ function extractRequestedFontFamilies(html: string): Map<string, string> {
|
||||
for (const originalCase of families) {
|
||||
const normalized = originalCase.toLowerCase();
|
||||
if (!normalized || GENERIC_FAMILIES.has(normalized)) continue;
|
||||
if (normalized.startsWith("var(")) continue;
|
||||
if (!requested.has(normalized)) requested.set(normalized, originalCase);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -109,6 +109,9 @@ export function validateNoSystemFonts(compiledHtml: string): void {
|
||||
for (const { surface, declaration, families } of iterateFontFamilyDeclarations(compiledHtml)) {
|
||||
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().
|
||||
if (!GENERIC_FAMILIES.has(primaryRaw.toLowerCase())) continue;
|
||||
throw new PlanValidationError(
|
||||
SYSTEM_FONT_USED,
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
{
|
||||
"name": "Distributed: CSS var() font-family",
|
||||
"description": "Composition that declares font-family via CSS custom properties (var(--ui-font), var(--display-font)). Exercises the fix for FONT_FETCH_FAILED when the font resolver encounters var() expressions in fail-closed distributed renders.",
|
||||
"tags": ["distributed", "mp4", "h264", "sdr", "fonts", "css-variables"],
|
||||
|
||||
"minPsnr": 30,
|
||||
"maxFrameFailures": 0,
|
||||
|
||||
"renderConfig": {
|
||||
"fps": 30,
|
||||
"chunkSize": 15
|
||||
}
|
||||
}
|
||||
Binary file not shown.
@@ -0,0 +1,74 @@
|
||||
<!doctype html>
|
||||
<html lang="en">
|
||||
<head>
|
||||
<meta charset="utf-8" />
|
||||
<meta content="width=device-width, initial-scale=1.0" name="viewport" />
|
||||
<title>CSS var() font-family regression fixture</title>
|
||||
<script src="https://cdnjs.cloudflare.com/ajax/libs/gsap/3.12.2/gsap.min.js"></script>
|
||||
<style>
|
||||
@import url("https://fonts.googleapis.com/css2?family=Inter:wght@400;700&family=Montserrat:wght@400;700;900&display=swap");
|
||||
|
||||
:root {
|
||||
--ui-font: "Inter";
|
||||
--display-font: "Montserrat";
|
||||
}
|
||||
|
||||
body,
|
||||
html {
|
||||
margin: 0;
|
||||
padding: 0;
|
||||
width: 640px;
|
||||
height: 360px;
|
||||
background: #1a1a2e;
|
||||
overflow: hidden;
|
||||
font-family: var(--ui-font), sans-serif;
|
||||
}
|
||||
|
||||
#main-comp {
|
||||
position: relative;
|
||||
width: 640px;
|
||||
height: 360px;
|
||||
}
|
||||
|
||||
.heading {
|
||||
position: absolute;
|
||||
top: 30%;
|
||||
left: 50%;
|
||||
transform: translateX(-50%);
|
||||
font-family: var(--display-font), sans-serif;
|
||||
font-size: 48px;
|
||||
font-weight: 900;
|
||||
color: #e94560;
|
||||
white-space: nowrap;
|
||||
opacity: 0;
|
||||
}
|
||||
|
||||
.body-text {
|
||||
position: absolute;
|
||||
top: 60%;
|
||||
left: 50%;
|
||||
transform: translateX(-50%);
|
||||
font-family: var(--ui-font), sans-serif;
|
||||
font-size: 18px;
|
||||
font-weight: 400;
|
||||
color: #c4c4c4;
|
||||
white-space: nowrap;
|
||||
opacity: 0;
|
||||
}
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<div id="main-comp" data-composition-id="css-var-fonts" data-width="640" data-height="360" data-start="0">
|
||||
<div class="heading">CSS Variable Fonts</div>
|
||||
<div class="body-text">var(--ui-font) and var(--display-font)</div>
|
||||
</div>
|
||||
<script>
|
||||
window.__timelines = window.__timelines || {};
|
||||
const tl = gsap.timeline();
|
||||
tl.to(".heading", { opacity: 1, duration: 0.5 });
|
||||
tl.to(".body-text", { opacity: 1, duration: 0.5 }, "+=0.2");
|
||||
tl.to({}, { duration: 1 });
|
||||
window.__timelines["css-var-fonts"] = tl;
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
||||
Reference in New Issue
Block a user