Skip to content

fix(fonts): preserve source stylesheet on Google Fonts cap overflow - #839

Merged
chubes4 merged 4 commits into
Automattic:mainfrom
faisalahammad:fix/732-font-cap-preserve-stylesheet
Aug 5, 2026
Merged

fix(fonts): preserve source stylesheet on Google Fonts cap overflow#839
chubes4 merged 4 commits into
Automattic:mainfrom
faisalahammad:fix/732-font-cap-preserve-stylesheet

Conversation

@faisalahammad

Copy link
Copy Markdown
Contributor

Summary

A valid compiler font plan that exceeded SSI's fixed Google Fonts byte caps (CSS > 256 KiB or aggregate woff2 > 4 MiB) used to hard-abort the entire import with static_site_importer_font_materialization_failed. The cap overflow now falls back to a preserved @import of the original Google stylesheet so the import continues, with a font_materialization_partial_preserved diagnostic that reports the exact reason, observed bytes, and offending URL.

Fixes #732

Changes

includes/class-static-site-importer-font-materializer.php

Why: Multilingual fixtures (e.g. 29-multilingual-i18n) blow past the CSS or aggregate woff2 cap. The cap should be bounded, but exceeding it must not abort an otherwise valid import. The producer path (materialize_producer_faces) is unchanged.

The two private helpers that materialize the Google Fonts path now return a tagged union instead of a bare CSS string. prepare_overlay() inspects the union, and on a preserved state re-scans the plan stylesheets for the original Google @import URL, writes an @import-only assets/css/embedded-fonts.css, and emits the diagnostic.

// before — hard-fail path
$css = self::embed_font_sources( ... );
if ( '' === $css ) {
    return new WP_Error( 'static_site_importer_font_materialization_failed', ... );
}

// after — preserve when cap overflows
$result = self::embed_font_sources( ... );
if ( 'preserved' === $result['state'] ) {
    // re-emit @import of the original Google URL, attach diagnostic
    $diagnostics[] = self::diagnostic_with_detail(
        'font_materialization_partial_preserved',
        $result['source'],
        $result['reason'],
        array(
            'url'             => $result['url'],
            'observed_bytes'  => $result['observed_bytes'],
            'limit_bytes'     => $result['limit_bytes'],
            'aggregate_bytes' => $result['aggregate_bytes'] ?? null,
        )
    );
    // continue with the rest of prepare_overlay
}

Why: Strict allowlist is_google_stylesheet_url() already gates the URL before it reaches CSS, but a defense-in-depth addcslashes( $x, "\"\\\\\\n" ) is applied to the URL interpolated into the @import string so future allowlist loosening cannot leak a quote/backslash/newline into a CSS at-rule.

tests/smoke-google-fonts-cap-fallback.php (new)

Standalone PHP smoke, 5 cases:

  1. Google CSS body > 256 KiB → preserved @import of original URL, google_fonts_stylesheet_preserved_due_to_size diagnostic with observed_bytes and limit_bytes === 262144.
  2. Aggregate woff2 > 4 MiB → preserved @import, google_fonts_payloads_partial_preserved diagnostic with limit_bytes === 4194304 and aggregate_bytes.
  3. Producer path regression sentinel — FontMaterializationPlanBuilder plan still materializes; required producer diagnostics still gate materialization.
  4. No @import in plan + no fallback URL → WP_Error closed-fail with stylesheet_import_missing reason.
  5. Legacy /css?family=… v1 URL — same preservation contract.

test-manifest.json, homeboy-test-manifest.json

Registered the new smoke as standalone-php.

Testing

Test 1: standalone smokes

php tests/smoke-google-fonts-cap-fallback.php
php tests/smoke-webfont-producer-consumer.php

Result: both pass.

Test 2: fast lane + inventory

npm test
npm run test:inventory

Result: 46/47 pass. The single failure is smoke-url-batch-import.php (pre-existing on clean main, unrelated to fonts).

Test 3: end-to-end repro against fixture 29-multilingual-i18n
Run the fixture matrix; confirm the output no longer contains static_site_importer_font_materialization_failed and that a font_materialization_partial_preserved diagnostic appears with details.url, details.observed_bytes, and details.limit_bytes populated. Browser surfaces (Chrome/SVG parity) should render text using the Noto Sans JP / Noto Naskh Arabic families.

Test 4: happy path regression
Run a small Google Fonts fixture (1 family, 1 weight). assets/css/embedded-fonts.css should still contain data:font/woff2;base64,… URLs and no font_materialization_partial_preserved diagnostic should appear.

Replace hard-fail with tagged-union return for resolve_google_font_faces()
and embed_font_sources(). On cap violation, emit assets/css/embedded-fonts.css
with original Google @import, surface font_materialization_partial_preserved
diagnostic with observed_bytes, limit_bytes, and url. Producer path unchanged.

Fixes Automattic#732
The file was accidentally included in the fix commit. Restoring
main's original version per request.
- widen return union with optional aggregate_bytes?:int on resolve_google_font_faces + embed_font_sources
- drop always-true is_array/isset guards and impossible is_string branch in prepare_overlay
- remove : array on smoke closures that return WP_Error

Refs Automattic#839
@faisalahammad

Copy link
Copy Markdown
Contributor Author

CI Fix Summary — Homeboy Lint resolved

# File Error Fix
1 includes/class-static-site-importer-font-materializer.php:L585-634 PHPStan nullCoalesce.offset (aggregate_bytes) + function.alreadyNarrowedType (3 sites) widened return union with aggregate_bytes?:int, dropped always-true is_array/isset guards
2 includes/class-static-site-importer-font-materializer.php:L126-128 PHPStan function.impossibleType (is_string on return value) deleted impossible else branch (function never returns string now)
3 includes/class-static-site-importer-font-materializer.php (all changed lines) PHPCS 16E+7W AssociativeArrayFound + MultipleStatementAlignment phpcbf --standard=WordPress, 23 auto-fixables cleared
4 tests/smoke-google-fonts-cap-fallback.php:L59,100,141,169,182 closure return type mismatch (: array vs WP_Error) removed : array on stub closures that return WP_Error

Smoke tests: both passing · phpcbf clean · CI re-run triggered.

Inline embedded_fonts_css assignment now that state union guarantees
'embedded' after the preserved branch returns.

Errors fixed:
- phpstan.identical.alwaysTrue: 'embedded' === 'embedded' always evaluates
  to true at level 7

PHP 8.2 compatible. All CI checks passing.

Refs Automattic#839

@faisalahammad faisalahammad left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI Fix Summary — 1 failure resolved

# File Error Fix
1 includes/class-static-site-importer-font-materializer.php:125 phpstan.identical.alwaysTrue dropped dead if ( 'embedded' === $font_faces['state'] ); state union guarantees embedded after the preserved branch returns

Tests ✅ · Verification ✅ · CodeRabbit ✅ (not part of CI gate; not run) · PHP 8.2 ✅

Refs #839

@faisalahammad

Copy link
Copy Markdown
Contributor Author

CI Fix Summary - 1 failure resolved

# File Error Fix
1 includes/class-static-site-importer-font-materializer.php:125 phpstan.identical.alwaysTrue dropped dead if, state union guarantees embedded after preserved branch returns

Tests passing. Verification clean. Homeboy Lint re-run 31038257340 passed 3m19s. PHP 8.1-8.4 all pass.

Refs #839

@chubes4

chubes4 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Thanks @faisalahammad !

@chubes4
chubes4 merged commit e923036 into Automattic:main Aug 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Font materialization hard-fails multilingual Google Fonts plans over fixed byte caps

2 participants