Make WordPress Core

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

WP_Font_Face::compile_variations() turned an array font-variation-settings into wght 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_Face dataset, one for the string form and one for the array form. Before the fix the array case fails with font-variation-settings:slnt 0wght 400;; after it, phpunit --group fonts passes (232 tests, 930 assertions), and phpcs reports 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-develop Docker environment.

@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.

Note: See TracTickets for help on using tickets.