Theme_JSON: Prevent implicit coercion in `to_ruleset` by ramonjd · Pull Request #76392 · WordPress/gutenberg

ramonjd · 2026-03-11T09:00:27Z

Fix: Prevent implicit coercion in `to_ruleset`

What

to_ruleset used string concatenation ($element['name'] . ': ' . $element['value'] . ';'), so PHP implicitly coerced non-string values (e.g. booleans → '1'/'', arrays → 'Array'). That could emit invalid or misleading CSS.

At the same time, pass a style theme.json path in test_get_styles_with_appearance_tools() to simulate a style node. Before it was settings.

Discovered while stomping around Theme_JSON in #73117

How

Cast numeric values to string explicitly.
Skip declarations whose value is not a string or number (booleans, arrays, objects).

Test

test_to_ruleset_skips_non_scalar_values_and_casts_numerics checks that:

Strings pass through.
Numerics are cast to string and included.
Booleans and arrays are skipped.

ramonjd · 2026-03-11T09:12:32Z

lib/class-wp-theme-json-gutenberg.php

+				$value = $element['value'];
+				if ( is_numeric( $value ) ) {
+					$value = (string) $value;
+				}
+				// Skip declarations whose value is not a plain string (booleans, arrays, objects, etc.).
+				if ( ! is_string( $value ) ) {
+					return $carry;
+				}
+				return $carry .= $element['name'] . ': ' . $value . ';';
+			},


This fixes the root issues. $element['value'] could be anything and PHP silently coerces it without the checks.

tellthemachines · 2026-03-11T23:06:10Z

phpunit/class-wp-theme-json-test.php

-		$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;}';
+		// When appearanceTools enables blockGap but no explicit CSS value is provided in styles,
+		// the layout rules that use blockGap as a placeholder (margin-block-start, gap) should
+		// not be emitted. A boolean feature flag is not a CSS value.


When appearanceTools enables blockGap but no explicit CSS value is provided in styles

Shouldn't this case be covered by the default gap value?

Thanks for flagging. I'll check more closely today with my morning glasses on. From what I saw a boolean value was sneaking its way through, and I don't know why. 🤔

I'm not sure setting the default value is right here.

There's a comment that explicitly states the default value is set if block gap is not available.

// Use a fallback gap value if block gap support is not available.

otherwise it's null in other cases.

What I'm seeing rather, and the issue I was trying to debug, is that the test_get_styles_with_appearance_tools() test passes the settings node path and not the styles node.

// Here $node === { settings: { ..., "spacing": { "blockGap": true, ... } } } $block_gap_value = static::get_property_value( $node, array( 'spacing', 'blockGap' ) );

It's setting the value to true which is the supports value, not a blockGap style value.

If there's no block gap value, then margin-block-start/end will fall through thanks to this condition:

if ( null !== $block_gap_value && false !== $block_gap_value && '' !== $block_gap_value ) {

I think it's just a matter of updating the test here and tightening up the to_ruleset coercion I think.

ramonjd · 2026-03-12T03:35:26Z

phpunit/class-wp-theme-json-test.php

 			'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;}';


Notice --wp--style--block-gap: ; the value was always empty. It was a quirk of passing settings as the path in the test that made the following condition pass because the value was true:

if ( null !== $block_gap_value && false !== $block_gap_value && '' !== $block_gap_value ) {

…ture flags. Update tests to reflect changes in blockGap placeholder behavior.

…This update ensures that only plain string values are processed, improving the handling of CSS rulesets.

…oving unnecessary comments and ensuring only valid CSS declarations are processed. Update test expectations to align with changes in blockGap placeholder behavior.

… values are skipped and numerics are cast to strings.

github-actions · 2026-03-12T03:42:42Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ramonjd <[email protected]>
Co-authored-by: tellthemachines <[email protected]>
Co-authored-by: andrewserong <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

ramonjd · 2026-03-12T03:42:44Z

If I've got things right according to folks, I'll prep a core patch.

github-actions · 2026-03-12T04:07:04Z

Flaky tests detected in a54f8c7.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22990608681
📝 Reported issues:

[Flaky Test] Should insert content using the global inserter #57525 in /test/e2e/specs/widgets/editing-widgets.spec.js

tellthemachines · 2026-03-12T04:16:35Z

phpunit/class-wp-theme-json-test.php

 		);

-		$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;}';


The empty --wp--style--block-gap: ; is still here though, is that expected?

Good question.

It's because blockGap isn't present: https://github.com/WordPress/gutenberg/blob/trunk/lib/class-wp-theme-json-gutenberg.php#L3525-L3531

So it's expected, but not ideal I guess.

We could skip adding those styles if there's no blockGap?

I left that bit alone as I was afraid of blowing things up.

andrewserong

LGTM, good catch!

My hunch is that the empty --wp--style--block-gap isn't too big a deal since in practice Gutenberg's core theme.json will always have a value for the block gap.

We could skip adding those styles if there's no blockGap?

No harm in skipping if there isn't a value, but also not a blocker if you wanted to land this code quality improvement without addressing it.

andrewserong · 2026-03-12T06:16:17Z

phpunit/class-wp-theme-json-test.php

+	/**
+	 * @covers WP_Theme_JSON_Gutenberg::to_ruleset
+	 */
+	public function test_to_ruleset_skips_non_scalar_values_and_casts_numerics() {


Whoops, I approved before checking the test results 😅

Does this one need a call to $reflection->setAccessible( true );?

The failure is:

WP_Theme_JSON_Gutenberg_Test::test_to_ruleset_skips_non_scalar_values_and_casts_numerics
ReflectionException: Trying to invoke protected method WP_Theme_JSON_Gutenberg::to_ruleset() from scope ReflectionMethod

LOL I deleted it because my local linter said it was deprecated. The tests pass for me. 🙃 I'll add it back it.

Thanks!!

Ah, it might just be the old PHP versions tests where it's needed, that might explain it!

Oh because of a PHP version mismatch prolly

tellthemachines · 2026-03-12T06:25:57Z

My hunch is that the empty --wp--style--block-gap isn't too big a deal since in practice Gutenberg's core theme.json will always have a value for the block gap.

True, but can this only happen for block gap? We should probably make sure not to output properties without values at some point in the flow.

andrewserong · 2026-03-12T06:30:16Z

True, but can this only happen for block gap?

From a quick skim I think so: looks like it's just this logic that assumes that array( 'styles', 'spacing', 'blockGap' ) will always have a value (because in core / GB it does). So looks like maybe it just needs an if statement there?

gutenberg/lib/class-wp-theme-json-gutenberg.php

Lines 3445 to 3453 in 5f647a0

    
           if ( isset( $this->theme_json['settings']['spacing']['blockGap'] ) ) { 
        
           	$block_gap_value = static::get_property_value( $this->theme_json, array( 'styles', 'spacing', 'blockGap' ) ); 
        
           	$css            .= ":where(.wp-site-blocks) > * { margin-block-start: $block_gap_value; margin-block-end: 0; }"; 
        
           	$css            .= ':where(.wp-site-blocks) > :first-child { margin-block-start: 0; }'; 
        
           	$css            .= ':where(.wp-site-blocks) > :last-child { margin-block-end: 0; }'; 
        
           	// For backwards compatibility, ensure the legacy block gap CSS variable is still available. 
        
           	$css .= static::ROOT_CSS_PROPERTIES_SELECTOR . " { --wp--style--block-gap: $block_gap_value; }"; 
        
           }

… method accessibility (deprecated in 8.5)

ramonjd self-assigned this Mar 11, 2026

ramonjd added [Type] Bug An existing feature does not function as intended [Feature] Layout Layout block support, its UI controls, and style output. labels Mar 11, 2026

ramonjd commented Mar 11, 2026

View reviewed changes

tellthemachines reviewed Mar 11, 2026

View reviewed changes

ramonjd commented Mar 12, 2026

View reviewed changes

ramonjd requested a review from andrewserong March 12, 2026 03:40

ramonjd changed the title ~~Fix CSS value handling in WP_Theme_JSON_Gutenberg to skip boolean feature flags~~ Fix: Prevent implicit coercion in to_ruleset Mar 12, 2026

ramonjd changed the title ~~Fix: Prevent implicit coercion in to_ruleset~~ Theme_JSON: Prevent implicit coercion in to_ruleset Mar 12, 2026

ramonjd added 5 commits March 12, 2026 14:41

Fix CSS value handling in WP_Theme_JSON_Gutenberg to skip boolean fea…

9c2ac2f

…ture flags. Update tests to reflect changes in blockGap placeholder behavior.

Enhance WP_Theme_JSON_Gutenberg to skip non-string CSS declarations. …

c2953a7

…This update ensures that only plain string values are processed, improving the handling of CSS rulesets.

Refactor WP_Theme_JSON_Gutenberg to improve CSS value handling by rem…

a29fb13

…oving unnecessary comments and ensuring only valid CSS declarations are processed. Update test expectations to align with changes in blockGap placeholder behavior.

Add test for WP_Theme_JSON_Gutenberg::to_ruleset to ensure non-scalar…

284bdc3

… values are skipped and numerics are cast to strings.

Remove comments

738991a

ramonjd force-pushed the update/fix-to-ruleset-casting branch from 140d7aa to 738991a Compare March 12, 2026 03:41

lintbean

6378006

ramonjd marked this pull request as ready for review March 12, 2026 03:42

tellthemachines reviewed Mar 12, 2026

View reviewed changes

andrewserong approved these changes Mar 12, 2026

View reviewed changes

andrewserong reviewed Mar 12, 2026

View reviewed changes

Update test for WP_Theme_JSON_Gutenberg::to_ruleset to set reflection…

62bb47b

… method accessibility (deprecated in 8.5)

ramonjd mentioned this pull request Mar 12, 2026

WP_Theme_JSON: Prevent implicit coercion in to_ruleset WordPress/wordpress-develop#11236

Open

Remove comment, add changelog

a54f8c7

ramonjd enabled auto-merge (squash) March 13, 2026 00:03

ramonjd merged commit 3709d01 into trunk Mar 13, 2026
49 of 50 checks passed

ramonjd deleted the update/fix-to-ruleset-casting branch March 13, 2026 00:16

github-actions bot added this to the Gutenberg 22.8 milestone Mar 13, 2026

Conversation

ramonjd commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix: Prevent implicit coercion in to_ruleset

What

How

Test

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ramonjd Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ramonjd commented Mar 12, 2026

Uh oh!

github-actions bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewserong left a comment

Choose a reason for hiding this comment

Uh oh!

andrewserong Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tellthemachines commented Mar 12, 2026

Uh oh!

andrewserong commented Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ramonjd commented Mar 11, 2026 •

edited

Loading

Fix: Prevent implicit coercion in `to_ruleset`

ramonjd Mar 12, 2026 •

edited

Loading

github-actions bot commented Mar 12, 2026 •

edited

Loading

github-actions bot commented Mar 12, 2026 •

edited

Loading

andrewserong Mar 12, 2026 •

edited

Loading