-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Global Styles: Do not sanitize block style variations based on registration status #62405
Global Styles: Do not sanitize block style variations based on registration status #62405
Conversation
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/class-wp-theme-json-gutenberg.php ❔ phpunit/class-wp-theme-json-test.php |
unregister_block_type( 'test/clamp-me' ); | ||
|
||
// Results also include root site blocks styles. | ||
$this->assertSame( | ||
':root{--wp--preset--font-size--pickles: clamp(14px, 0.875rem + ((1vw - 3.2px) * 0.156), 16px);--wp--preset--font-size--toast: clamp(14.642px, 0.915rem + ((1vw - 3.2px) * 0.575), 22px);}: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(.is-layout-flex){gap: 0.5em;}:where(.is-layout-grid){gap: 0.5em;}.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;}:root :where(body){font-size: clamp(0.875em, 0.875rem + ((1vw - 0.2em) * 0.156), 1em);}:root :where(h1){font-size: clamp(50.171px, 3.136rem + ((1vw - 3.2px) * 3.893), 100px);}:root :where(.wp-block-test-clamp-me){font-size: clamp(27.894px, 1.743rem + ((1vw - 3.2px) * 1.571), 48px);}.has-pickles-font-size{font-size: var(--wp--preset--font-size--pickles) !important;}.has-toast-font-size{font-size: var(--wp--preset--font-size--toast) !important;}', | ||
$theme_json->get_stylesheet() | ||
$stylesheet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turned out the deregistration of the test block before generating the stylesheet was causing this test to fail on this branch. Not 100% sure why yet but I'll look into that on Monday.
Hi Aaron, after your braindump (thank you!) I took a look at this issue and realized there is an alternative way to fix it: we can register the block style variations defined by the theme on The The proposal is at WordPress/wordpress-develop#6756 I had some issues in my environment and ended up preparing it for core. |
); | ||
// Only add registered variations to this node's variations to | ||
// avoid CSS generated without a selector. | ||
if ( isset( $selectors[ $name ]['styleVariations'][ $variation ] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, what this PR does is:
- do not remove the unregistered variations when we build the theme.json in memory
- but make sure we only process the registered ones when we use them
This is the kind of thing that I mentioned: we are not removing sanitization but moving it everywhere. Each time we have to work with them, we need to make sure to use the ones that are registered.
Closing this PR in favour of moving the block style registration to |
Fixes: #62303
What?
Only sanitize the properties of block style variations within theme.json instead of also omitting any that are not currently registered.
Why?
With the introduction of the Section Styles feature that leverages extended block style variations, there are a number of methods through which block style variations can be registered. This primarily happens via the theme.json data filters in the
WP_Theme_JSON_Resolver
class.Unfortunately, there are several places where global styles data is processed (such as the global styles REST API endpoint) that do not apply these filters and instantiate the
WP_Theme_JSON
class directly. The result then is some block style variations are not registered and are incorrectly stripped during the sanitization of the theme.json data.How?
This PR:
Testing Instructions
There are some duotone support actions in Gutenberg that trigger all the theme json filters. So to test this fix fully, those actions need to be temporarily disabled.
Screenshots or screencast
Screen.Recording.2024-06-05.at.6.49.43.PM.mp4
Screen.Recording.2024-06-05.at.6.50.31.PM.mp4