Skip to content

WP_Theme_JSON: Prevent implicit coercion in to_ruleset #11236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ramonjd wants to merge 3 commits into
base: trunk
Choose a base branch
from
+56 −3
Open

WP_Theme_JSON: Prevent implicit coercion in to_ruleset #11236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7940c80
Enhancement: Update WP_Theme_JSON::to_ruleset to skip non-scalar valu…
ramonjd Mar 12, 2026
c4acf90
annotation for ticket
ramonjd Mar 12, 2026
18ac1b5
Adjust accessibility of ReflectionMethod in test_to_ruleset for PHP v…
ramonjd Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/wp-includes/class-wp-theme-json.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1982,6 +1982,7 @@ protected function get_css_variables( $nodes, $origins ) {
* creates the corresponding ruleset.
*
* @since 5.8.0
* @since 7.1.0 Skip declarations whose value is not a plain string (booleans, arrays, objects, etc.).
*
* @param string $selector CSS selector.
* @param array $declarations List of declarations.
Expand All @@ -1995,7 +1996,18 @@ protected static function to_ruleset( $selector, $declarations ) {
$declaration_block = array_reduce(
$declarations,
static function ( $carry, $element ) {
return $carry .= $element['name'] . ': ' . $element['value'] . ';'; },
$value = $element['value'];

if ( is_numeric( $value ) ) {
$value = (string) $value;
}

if ( ! is_string( $value ) ) {
return $carry;
}

return $carry .= $element['name'] . ': ' . $value . ';';
},
''
);

Expand Down
45 changes: 43 additions & 2 deletions tests/phpunit/tests/theme/wpThemeJson.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4138,6 +4138,7 @@ public function test_get_styles_with_content_width() {
* @ticket 60936
* @ticket 61165
* @ticket 61829
* @ticket 64848
*/
public function test_get_styles_with_appearance_tools() {
$theme_json = new WP_Theme_JSON(
Expand All @@ -4150,11 +4151,11 @@ public function test_get_styles_with_appearance_tools() {
);

$metadata = array(
'path' => array( 'settings' ),
'path' => array( 'styles' ),
'selector' => 'body',
);

$expected = ':where(body) { margin: 0; }.wp-site-blocks > .alignleft { float: left; margin-right: 2em; }.wp-site-blocks > .alignright { float: right; margin-left: 2em; }.wp-site-blocks > .aligncenter { justify-content: center; margin-left: auto; margin-right: auto; }:where(.wp-site-blocks) > * { margin-block-start: ; margin-block-end: 0; }:where(.wp-site-blocks) > :first-child { margin-block-start: 0; }:where(.wp-site-blocks) > :last-child { margin-block-end: 0; }:root { --wp--style--block-gap: ; }:root :where(.is-layout-flow) > :first-child{margin-block-start: 0;}:root :where(.is-layout-flow) > :last-child{margin-block-end: 0;}:root :where(.is-layout-flow) > *{margin-block-start: 1;margin-block-end: 0;}:root :where(.is-layout-constrained) > :first-child{margin-block-start: 0;}:root :where(.is-layout-constrained) > :last-child{margin-block-end: 0;}:root :where(.is-layout-constrained) > *{margin-block-start: 1;margin-block-end: 0;}:root :where(.is-layout-flex){gap: 1;}:root :where(.is-layout-grid){gap: 1;}.is-layout-flow > .alignleft{float: left;margin-inline-start: 0;margin-inline-end: 2em;}.is-layout-flow > .alignright{float: right;margin-inline-start: 2em;margin-inline-end: 0;}.is-layout-flow > .aligncenter{margin-left: auto !important;margin-right: auto !important;}.is-layout-constrained > .alignleft{float: left;margin-inline-start: 0;margin-inline-end: 2em;}.is-layout-constrained > .alignright{float: right;margin-inline-start: 2em;margin-inline-end: 0;}.is-layout-constrained > .aligncenter{margin-left: auto !important;margin-right: auto !important;}.is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)){margin-left: auto !important;margin-right: auto !important;}body .is-layout-flex{display: flex;}.is-layout-flex{flex-wrap: wrap;align-items: center;}.is-layout-flex > :is(*, div){margin: 0;}body .is-layout-grid{display: grid;}.is-layout-grid > :is(*, div){margin: 0;}';
$expected = ':where(body) { margin: 0; }.wp-site-blocks > .alignleft { float: left; margin-right: 2em; }.wp-site-blocks > .alignright { float: right; margin-left: 2em; }.wp-site-blocks > .aligncenter { justify-content: center; margin-left: auto; margin-right: auto; }:where(.wp-site-blocks) > * { margin-block-start: ; margin-block-end: 0; }:where(.wp-site-blocks) > :first-child { margin-block-start: 0; }:where(.wp-site-blocks) > :last-child { margin-block-end: 0; }:root { --wp--style--block-gap: ; }.is-layout-flow > .alignleft{float: left;margin-inline-start: 0;margin-inline-end: 2em;}.is-layout-flow > .alignright{float: right;margin-inline-start: 2em;margin-inline-end: 0;}.is-layout-flow > .aligncenter{margin-left: auto !important;margin-right: auto !important;}.is-layout-constrained > .alignleft{float: left;margin-inline-start: 0;margin-inline-end: 2em;}.is-layout-constrained > .alignright{float: right;margin-inline-start: 2em;margin-inline-end: 0;}.is-layout-constrained > .aligncenter{margin-left: auto !important;margin-right: auto !important;}.is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)){margin-left: auto !important;margin-right: auto !important;}body .is-layout-flex{display: flex;}.is-layout-flex{flex-wrap: wrap;align-items: center;}.is-layout-flex > :is(*, div){margin: 0;}body .is-layout-grid{display: grid;}.is-layout-grid > :is(*, div){margin: 0;}';
Copy link
Member Author

@ramonjd ramonjd Mar 12, 2026

Choose a reason for hiding this comment

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

The test result is different because we're no longer passing settings as metadata. Settings contains no style data, but supports data (mostly bools as values), but the source code was trying to fetch style data nevertheless. Since the paths to the settings and styles are same, the result was bool instead of real gap values.

It was this that led me to remove the silent coercion in to_ruleset.

$this->assertSame( $expected, $theme_json->get_root_layout_rules( WP_Theme_JSON::ROOT_BLOCK_SELECTOR, $metadata ) );
}

Expand Down Expand Up @@ -7054,4 +7055,44 @@ public function test_sanitize_preserves_null_schema_behavior() {
$this->assertSame( 'string-value', $settings['appearanceTools'], 'Appearance tools should be string value' );
$this->assertSame( array( 'nested' => 'value' ), $settings['custom'], 'Custom should be array value' );
}

/**
* @covers WP_Theme_JSON::to_ruleset
*
* @ticket 64848
*/
public function test_to_ruleset_skips_non_scalar_values_and_casts_numerics() {
$reflection = new ReflectionMethod( WP_Theme_JSON::class, 'to_ruleset' );
if ( PHP_VERSION_ID < 80100 ) {
$reflection->setAccessible( true );
}
$declarations = array(
array(
'name' => 'color',
'value' => 'red',
),
array(
'name' => 'opacity',
'value' => true,
),
array(
'name' => 'margin',
'value' => 0,
),
array(
'name' => 'padding',
'value' => false,
),
array(
'name' => 'gap',
'value' => array(),
),
);
$result = $reflection->invoke( null, '.test', $declarations );
$this->assertStringContainsString( 'color: red;', $result, 'Color declaration should be included' );
$this->assertStringContainsString( 'margin: 0;', $result, 'Numeric value should be cast to string' );
$this->assertStringNotContainsString( 'opacity', $result, 'Boolean value should be skipped' );
$this->assertStringNotContainsString( 'padding', $result, 'Boolean value should be skipped' );
$this->assertStringNotContainsString( 'gap', $result, 'Array value should be skipped' );
}
}
Loading