Opened 19 hours ago
Last modified 16 hours ago
#66103 new defect (bug)
WP_Font_Face compiles an array font-variation-settings into invalid CSS
| Reported by: | kimjiwoon | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | General | Version: | 6.4 |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description
WP_Font_Face::generate_font_face_css() accepts font-variation-settings as an array and converts it with compile_variations(). That method concatenates each axis as "$key $value" with no separator and no quotes around the axis tag, so the output is not valid CSS.
Steps to reproduce
<?php wp_print_font_faces( array( 'Example Variable' => array( array( 'font-family' => 'Example Variable', 'src' => array( 'https://example.org/example-variable.woff2' ), 'font-variation-settings' => array( 'wght' => 400, 'GRAD' => 0 ), ), ), ) );
Actual
font-variation-settings:wght 400GRAD 0;
Expected
font-variation-settings:"wght" 400, "GRAD" 0;
The font-variation-settings descriptor takes a comma-separated list of <string> <number> pairs, where the axis tag is a quoted four-character string. Without the quotes and commas a browser drops the declaration.
The array branch exists since 6.4.0 ([56500] / #59165) and has no unit test; the string form, which theme.json documents, is passed through unchanged and works.
Suggested fix
private function compile_variations( array $font_variation_settings ) { - $variations = ''; - - foreach ( $font_variation_settings as $key => $value ) { - $variations .= "$key $value"; - } - - return $variations; + $variations = array(); + + foreach ( $font_variation_settings as $tag => $value ) { + $variations[] = sprintf( '"%s" %s', $tag, $value ); + } + + return implode( ', ', $variations ); }
plus a data provider case in tests/phpunit/tests/fonts/font-face/wpFontFace/generateAndPrint.php (or the shared dataset) asserting the expected output above.
Change History (2)
This ticket was mentioned in PR #13514 on WordPress/wordpress-develop by @kimjiwoon.
19 hours ago
#1
- Keywords has-patch has-unit-tests added
@ugyensupport commented on PR #13514:
16 hours ago
#2
## Test report
Tested against a fresh wordpress-develop clone at trunk (ba9d19e41e), PHP 8.5.7, PHPUnit 9.6.36, MySQL.
Method: applied only the new dataset-case hunk to unpatched trunk first (source untouched) to confirm the array-form case reproduces the bug, then applied the source fix on top and re-ran, then ran --group fonts in full and phpcs on both changed files.
Results:
| Run | Result |
|---|---|
--group fonts, before fix (test-only patch on unpatched source) | 2 failed / 232 — the new "variation settings array" case fails with font-variation-settings:slnt 0wght 400;, exactly the invalid CSS described in the ticket
|
--group fonts, after fix (full patch) | 232 / 232 passed (930 assertions) — matches the numbers in this PR's own description |
phpcs --standard=phpcs.xml.dist on both changed files | 0 errors |
Direct repro of the ticket's own example (compile_variations( array( 'wght' => 400, 'GRAD' => 0 ) ) via reflection) | font-variation-settings:"wght" 400, "GRAD" 0; — matches the ticket's "Expected" line exactly
|
The diff also applied cleanly (git apply --check) against current trunk with no conflicts.
Visual check: also rendered the real compiled CSS through a live variable font (Inter, wght+opsz axes) in a browser to confirm the practical effect, not just the string diff. Before the fix, the malformed declaration (font-variation-settings:wght 900opsz 32;) is invalid CSS and the browser drops it entirely, so the text renders at the font's default weight. After the fix, the same declaration (font-variation-settings:"wght" 900, "opsz" 32;) is valid and the browser applies it, rendering at weight 900. Happy to attach the before/after screenshots if useful — didn't want to route them through a public image host for this comment.
No regressions found in --group fonts; the string-form path (already correct) is unaffected. LGTM.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
WP_Font_Face::compile_variations()turned an arrayfont-variation-settingsintowght 400GRAD 0: no quotes around the axis tags and no commas between the axes, so browsers drop the declaration. It now prints"wght" 400, "GRAD" 0.The string form, which theme.json documents, is passed through unchanged and is not affected.
Tests: two cases in the
WP_Font_Facedataset, one for the string form and one for the array form. Before the fix the array case fails withfont-variation-settings:slnt 0wght 400;; after it,phpunit --group fontspasses (232 tests, 930 assertions), andphpcsreports nothing on the two changed files.## Use of AI Tools
This pull request was prepared with Claude Code (Anthropic). It located the defect, wrote the fix and test cases, and ran the tests and coding standards checks locally in the
wordpress-developDocker environment.