-
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
Update the Gutenberg plugin to require at least the WP 6.1 version #50079
Conversation
Flaky tests detected in 9e2575bbcc84984ca2d6dfb379be570fd56368ee. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4804708630
|
9e2575b
to
3d3af3c
Compare
Thanks for starting this PR, nice to see. There might be a few more cleanup tasks to do:
These are the things I found, there might be other stuff that I missed. |
@fullofcaffeine, can we remove site editor route backward compatibility - |
Thanks for the ping. I've done a quick smoke test, removing the 6.1 compat code in Below is the diff. I'm happy to throw up a separate PR if it's not included in this one: diffdiff --git a/packages/style-engine/class-wp-style-engine-css-declarations.php b/packages/style-engine/class-wp-style-engine-css-declarations.php
index 6e7fdfc58e..a91bcab520 100644
--- a/packages/style-engine/class-wp-style-engine-css-declarations.php
+++ b/packages/style-engine/class-wp-style-engine-css-declarations.php
@@ -17,22 +17,6 @@ if ( class_exists( 'WP_Style_Engine_CSS_Declarations' ) ) {
* @access private
*/
class WP_Style_Engine_CSS_Declarations {
- /**
- * An array of valid CSS custom properties.
- * CSS custom properties are permitted by safecss_filter_attr()
- * since WordPress 6.1. See: https://core.trac.wordpress.org/ticket/56353.
- *
- * This whitelist exists so that the Gutenberg plugin maintains
- * backwards compatibility with versions of WordPress < 6.1.
- *
- * It does not need to be backported to future versions of WordPress.
- *
- * @var array
- */
- protected static $valid_custom_declarations = array(
- '--wp--style--unstable-gallery-gap' => 'gap',
- );
-
/**
* An array of CSS declarations (property => value pairs).
*
@@ -140,23 +124,6 @@ class WP_Style_Engine_CSS_Declarations {
*/
protected static function filter_declaration( $property, $value, $spacer = '' ) {
$filtered_value = wp_strip_all_tags( $value, true );
-
- /**
- * Allows a specific list of CSS custom properties starting with `--wp--`.
- *
- * CSS custom properties are permitted by safecss_filter_attr()
- * since WordPress 6.1. See: https://core.trac.wordpress.org/ticket/56353.
- *
- * This condition exists so that the Gutenberg plugin maintains
- * backwards compatibility with versions of WordPress < 6.1.
- *
- * It does not need to be backported to future versions of WordPress.
- */
- if ( '' !== $filtered_value && isset( static::$valid_custom_declarations[ $property ] ) ) {
- return safecss_filter_attr( static::$valid_custom_declarations[ $property ] . ":{$spacer}{$value}" ) ?
- "{$property}:{$spacer}{$value}" : '';
- }
-
if ( '' !== $filtered_value ) {
return safecss_filter_attr( "{$property}:{$spacer}{$filtered_value}" );
}
|
Thank you, @ramonjd! I'll include Style Engine cleanup here 👍 |
I removed the remaining compat code beside |
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.
Thanks for handling this George! In my testing everything looked good. We can probably remove some PHP tests too, but we can do it later too.
Thanks for testing, Nik! I'm thinking of merging this, and then I can do a follow-up for the |
@noahtallen, failure is caused by the core change. See #50185 |
Hi! This PR has caused a regression in layout styles, due to the core |
Is it specific to the grid layout? As far as I can see it hasn't yet been shipped to core (at the time of writing). Added comment over here. |
No, all the work from #47858 has regressed too. Edit: we seem to have this problem every time we remove support for an older WP version; perhaps the code to run the Gutenberg versions of these classes when the plugin is enabled shouldn't live inside the |
Thank you, @tellthemachines.
That is very true 😞 I will start looking into how we can fix this in a more future-proof way. What would be the best way to test that the new method works correctly for layouts? Just make sure changes from #47858 are correctly applied. Side note: It would be great to add regression tests for layout(s). I belive we briefly talked about this during WordCamp Asia. |
I started working on the fix in #50310. I would appreciate the testing instructions for the regression. |
The easiest way to test this is with the following markup:
The first paragraph inside the group should have the following CSS rule:
And both paragraphs should have:
Whereas in the current state of trunk, both paragraphs have:
And the second paragraph has:
|
Yes that is very true! We definitely need to add tests before stabilising the layout support. Which I hope will happen soon 😅 |
Thanks for the note, @talldan. I think the block view script handlers were not ported in WP 6.1, and the code was in the wrong compat layer. @gziolo, @aristath, can you confirm which WP version includes |
It looks like there's also this code here related to view scripts: gutenberg/lib/experimental/blocks.php Lines 8 to 79 in e6450b2
If I delete that it works again. 🤷 |
That would be very surprising that it impacts the Navigation block. I'll investigate today. |
@talldan, if I add both these filters, then the |
We probably need to deregister scripts registered in WP core, similar to this line:
We have a similar logic for individual block styles in: Line 188 in d54dc62
|
Out of interest, what's the issue with removing the code in the |
See #50364. It's all about preventing registering a script using the same handle twice. When it happens, |
I found this PR via a git bisect while investigating #50416. That issue appears to be due to the same problem @tellthemachines flagged. A quick test of #50310 shows it fixes the issue in #50416 as well. |
@Mamaduka Sorry for missing this one! :/ -- next time if I don't reply in a timely manner, feel free to ping me via Slack in the WP core channel (my handle is the same there). |
What?
WordPress 6.2 was released a month ago. It's time to bump the minimum required version for the Gutenberg plugin and remove the compat layer.
It also fixes #45870.
How?
compat/wordpress-6.1
directory.Testing Instruction