Skip to content
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

Fix gutenberg_get_block_editor_settings overriding other hooks #50760

Merged
merged 4 commits into from
May 22, 2023

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented May 19, 2023

What?

Other files that used the block_editor_settings_all hook and updated settings were getting overridden.

In gutenberg WP_Duotone_Gutenberg::add_editor_settings updated $settings['styles'].

In core wp_add_editor_classic_theme_styles also updated $settings['styles'].

I also expected that there may have been bugs with some of the __experimentalFeature settings since those were also completely being overridden and the renames weren't removed.

Why?

Bug fix.

  1. Duotone filters weren't showing in the post editor.
    See https://github.com/WordPress/gutenberg/blob/trunk/lib/block-supports/duotone.php#L54.
  2. /css/classic-themes.css wasn't being added for classic themes.
    See https://github.com/WordPress/wordpress-develop/blob/23b007b126fa73211b7db56a38e78e601e4abbc6/src/wp-includes/default-filters.php#L582.
  3. __experimentalFeatures renames weren't being handled.
    See https://github.com/WordPress/wordpress-develop/blob/23b007b126fa73211b7db56a38e78e601e4abbc6/src/wp-includes/block-editor.php#L443

How?

  • Update the hook priority to run first.
  • Copy the remaining __experimentalFeatures code from core.

Testing Instructions

1. Duotone filters weren't showing in the post editor.

  1. Set styles.blocks['core/cover'].filter.duotone to an existing filter either via the site editor or in theme.json.
  2. See that the filter you selected is correct in the post editor when adding a new cover block.

@jeryj, you reported this one to me, so would you be able to test it here?

2. /css/classic-themes.css wasn't being added for classic themes.

@scruffian fixed something similar in #44334, but I would expect this issue to show in the post editor. Would you be able to confirm this one?

3. __experimentalFeatures renames weren't being handled.

@oandregal did something similar back in #32059. Would you be able to confirm this one?

Testing Instructions for Keyboard

Screenshots or screencast

@ajlende ajlende added the [Type] Bug An existing feature does not function as intended label May 19, 2023
@ajlende ajlende requested a review from spacedmonkey as a code owner May 19, 2023 00:36
@ajlende ajlende self-assigned this May 19, 2023

return $settings;
}
add_filter( 'block_editor_settings_all', 'gutenberg_get_block_editor_settings', PHP_INT_MAX );
add_filter( 'block_editor_settings_all', 'gutenberg_get_block_editor_settings', 0 );
Copy link
Contributor Author

@ajlende ajlende May 19, 2023

Choose a reason for hiding this comment

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

Maybe 0 is too soon, but it has to be less than 10 because that's the priority that wp_add_editor_classic_theme_styles is called at (unless the intention is to remove that stylesheet with the plugin active).

Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit worried about this change. It means that every custom filter can override the settings the plugin depends on to be available. This could make debugging issues harder.

Maybe we should use the previous solution of filtering out global styles and recreating them.

I checked, and wp_add_editor_classic_theme_styles adds styles with isGlobalStyles set to false. You also left an inline note for Duotone styles.

// Remove existing global styles provided by core.
$existing_non_global_styles = array();
foreach ( $settings['styles'] as $style ) {
    if (
        ! isset( $style['isGlobalStyles'] ) && ! $style['isGlobalStyles']
    ) {
        $existing_non_global_styles[] = $style;
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It means that every custom filter can override the settings the plugin depends on to be available. This could make debugging issues harder.

This is a fair concern but I'm not sure that it will really be a problem. Maybe we can try it like this for now and if that does become an issue we can go back to the old way of doing it?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not against this change. However, considering that PHP_INT_MAX has been used as a priority for similar filters for years now, it would be hard to guess the side effects of this change.

$settings['__experimentalFeatures'] = gutenberg_get_global_settings();
// These settings may need to be updated based on data coming from theme.json sources.
Copy link
Member

Choose a reason for hiding this comment

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

There's no similar handler in the core; let's double-check if this is really needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scruffian
Copy link
Contributor

scruffian commented May 19, 2023

@scruffian fixed something similar in #44334, but I would expect this issue to show in the post editor. Would you be able to confirm this one?

Yes I can confirm this is broken in trunk and this PR fixes it.

Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

This fixes the issues for me. It might be good to have a review from @youknowriad before merging though.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

To be honest, it's very hard for me to know what impact this is going to have exactly. I understand the reasoning here though. Is there a way to avoid coping the same code from core?

@Mamaduka
Copy link
Member

I understand the reasoning here though. Is there a way to avoid coping the same code from core?

Unfortunately, we have to do that. At least while the WP_Theme_JSON classes are under active development. See #50310 (comment)

@ajlende
Copy link
Contributor Author

ajlende commented May 19, 2023

@Mamaduka and @youknowriad I can explain my reasoning a bit more for choosing 0 and what I've done to gauge the impact of this change.

  1. I searched all the code in core/gutenberg using this hook that modified either $settings['styles'] or $settings['__experimentalFeatures'], the two settings that are being replaced here. I found only the 3 places listed in the description. It doesn't account for plugins that may be able to modify it, but I think that's okay because of the next item.
  2. The intent of this hook is to replace the get_block_editor_settings code in core with the gutenberg_ implantation that will eventually be ported back to core. Any conflicts that plugins have with this implementation will eventually be a conflict with core when those changes land.
  3. Finally, when we are porting code to core, keeping the same structure of get_block_editor_settings in this hook means we can copy any compat/X.X versions into lib and then the lib version into get_block_editor_settings in core.

I did think of one reason that we may not want to use 0 as I was writing this.

  1. There is at least one instance of experimental code (added in #47290) in the dependencies of the gutenberg_ versions of the functions used that may not get ported to core. This particular instance doesn't affect either $settings['styles'] or $settings['__experimentalFeatures'].
    Ideally, I think this code would be better if it was under experimental as a separate hook. But someone may add an experimental piece of code the same way as this that would cause problems.

I also ran a word diff of the $settings['styles'] output in core vs plugin to get an idea of the differences. GitHub, unfortunately, doesn't present it very well.

diff --git a/Users/ajlende/Desktop/before.txt b/Users/ajlende/Desktop/after.txt
index f60bd05e3a..4c77b96ae9 100644
--- a/Users/ajlende/Desktop/before.txt
+++ b/Users/ajlende/Desktop/after.txt
@@ -5 +5 @@ Array
            [css] => body{--wp--preset--color--black: #000000;--wp--preset--color--cyan-bluish-gray: #abb8c3;--wp--preset--color--white: #ffffff;--wp--preset--color--pale-pink: #f78da7;--wp--preset--color--vivid-red: #cf2e2e;--wp--preset--color--luminous-vivid-orange: #ff6900;--wp--preset--color--luminous-vivid-amber: #fcb900;--wp--preset--color--light-green-cyan: #7bdcb5;--wp--preset--color--vivid-green-cyan: #00d084;--wp--preset--color--pale-cyan-blue: #8ed1fc;--wp--preset--color--vivid-cyan-blue: #0693e3;--wp--preset--color--vivid-purple: #9b51e0;--wp--preset--color--base: #002635;--wp--preset--color--contrast: #e6e6dc;--wp--preset--color--primary: #ff5a67;--wp--preset--color--secondary: #ffcc1b;--wp--preset--color--tertiary: #5dd7b9;--wp--preset--gradient--vivid-cyan-blue-to-vivid-purple: linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%);--wp--preset--gradient--light-green-cyan-to-vivid-green-cyan: linear-gradient(135deg,rgb(122,220,180) 0%,rgb(0,208,130) 100%);--wp--preset--gradient--luminous-vivid-amber-to-luminous-vivid-orange: linear-gradient(135deg,rgba(252,185,0,1) 0%,rgba(255,105,0,1) 100%);--wp--preset--gradient--luminous-vivid-orange-to-vivid-red: linear-gradient(135deg,rgba(255,105,0,1) 0%,rgb(207,46,46) 100%);--wp--preset--gradient--very-light-gray-to-cyan-bluish-gray: linear-gradient(135deg,rgb(238,238,238) 0%,rgb(169,184,195) 100%);--wp--preset--gradient--cool-to-warm-spectrum: linear-gradient(135deg,rgb(74,234,220) 0%,rgb(151,120,209) 20%,rgb(207,42,186) 40%,rgb(238,44,130) 60%,rgb(251,105,98) 80%,rgb(254,248,76) 100%);--wp--preset--gradient--blush-light-purple: linear-gradient(135deg,rgb(255,206,236) 0%,rgb(152,150,240) 100%);--wp--preset--gradient--blush-bordeaux: linear-gradient(135deg,rgb(254,205,165) 0%,rgb(254,45,45) 50%,rgb(107,0,62) 100%);--wp--preset--gradient--luminous-dusk: linear-gradient(135deg,rgb(255,203,112) 0%,rgb(199,81,192) 50%,rgb(65,88,208) 100%);--wp--preset--gradient--pale-ocean: linear-gradient(135deg,rgb(255,245,203) 0%,rgb(182,227,212) 50%,rgb(51,167,181) 100%);--wp--preset--gradient--electric-grass: linear-gradient(135deg,rgb(202,248,128) 0%,rgb(113,206,126) 100%);--wp--preset--gradient--midnight: linear-gradient(135deg,rgb(2,3,129) 0%,rgb(40,116,252) 100%);--wp--preset--gradient--base: linear-gradient(141deg, #021524, #002635 71%, #1f3e47);--wp--preset--gradient--contrast: linear-gradient(141deg, #9b9d96, #e6e6dc 71%, #ebe8e1);--wp--preset--gradient--primary: linear-gradient(141deg, #b23926, #ff5a67 71%, #f97895);--wp--preset--gradient--secondary: linear-gradient(141deg, #a19100, #ffcc1b 71%, #ffce74);--wp--preset--gradient--tertiary: linear-gradient(141deg, #289389, #5dd7b9 71%, [-#8cdab4);--wp--preset--duotone--dark-grayscale: url('#wp-duotone-dark-grayscale');--wp--preset--duotone--grayscale: url('#wp-duotone-grayscale');--wp--preset--duotone--purple-yellow: url('#wp-duotone-purple-yellow');--wp--preset--duotone--blue-red: url('#wp-duotone-blue-red');--wp--preset--duotone--midnight: url('#wp-duotone-midnight');--wp--preset--duotone--magenta-yellow: url('#wp-duotone-magenta-yellow');--wp--preset--duotone--purple-green: url('#wp-duotone-purple-green');--wp--preset--duotone--blue-orange: url('#wp-duotone-blue-orange');--wp--preset--duotone--contrast: url('#wp-duotone-contrast');--wp--preset--duotone--primary: url('#wp-duotone-primary');--wp--preset--duotone--secondary: url('#wp-duotone-secondary');--wp--preset--duotone--tertiary: url('#wp-duotone-tertiary');--wp--preset--font-size--small:-]{+#8cdab4);--wp--preset--font-size--small:+} 1rem;--wp--preset--font-size--medium: 1.111rem;--wp--preset--font-size--large: 1.444rem;--wp--preset--font-size--x-large: 2.778rem;--wp--preset--font-size--xx-large: 3.778rem;--wp--preset--font-family--raleway: 'Raleway', sans-serif;--wp--preset--spacing--30: clamp(1.5rem, 5vw, 2rem);--wp--preset--spacing--40: clamp(1.8rem, 1.8rem + ((1vw - 0.48rem) * 2.885), 3rem);--wp--preset--spacing--50: clamp(2.5rem, 8vw, 4.5rem);--wp--preset--spacing--60: clamp(3.75rem, 10vw, 7rem);--wp--preset--spacing--70: clamp(5rem, 5.25rem + ((1vw - 0.48rem) * 9.096), 8rem);--wp--preset--spacing--80: clamp(7rem, 14vw, 11rem);--wp--preset--shadow--natural: 6px 6px 9px rgba(0, 0, 0, 0.2);--wp--preset--shadow--deep: 12px 12px 50px rgba(0, 0, 0, 0.4);--wp--preset--shadow--sharp: 6px 6px 0px rgba(0, 0, 0, 0.2);--wp--preset--shadow--outlined: 6px 6px 0px -3px rgba(255, 255, 255, 1), 6px 6px rgba(0, 0, 0, 1);--wp--preset--shadow--crisp: 6px 6px 0px rgba(0, 0, 0, 1);}
@@ -19 +19 @@ Array
            [css] => body { margin: 0;--wp--style--global--content-size: 640px;--wp--style--global--wide-size: [-880px; }.wp-site-blocks-]{+880px;}.wp-site-blocks+} { padding-top: var(--wp--style--root--padding-top); padding-bottom: var(--wp--style--root--padding-bottom); }.has-global-padding { padding-right: var(--wp--style--root--padding-right); padding-left: var(--wp--style--root--padding-left); }.has-global-padding :where(.has-global-padding) { padding-right: 0; padding-left: 0; }.has-global-padding > .alignfull { margin-right: calc(var(--wp--style--root--padding-right) * -1); margin-left: calc(var(--wp--style--root--padding-left) * -1); }.has-global-padding :where(.has-global-padding) > .alignfull { margin-right: 0; margin-left: 0; }.has-global-padding > .alignfull:where(:not(.has-global-padding)) > [-:where([class*="wp-block-"]:not(.alignfull):not([class*="__"]),p,h1,h2,h3,h4,h5,h6,ul,ol)-]{+:where([class*="wp-block-"]:not(.alignfull):not([class*="__"]),.wp-block:not(.alignfull),p,h1,h2,h3,h4,h5,h6,ul,ol)+} { padding-right: var(--wp--style--root--padding-right); padding-left: var(--wp--style--root--padding-left); }.has-global-padding :where(.has-global-padding) > .alignfull:where(:not(.has-global-padding)) > [-:where([class*="wp-block-"]:not(.alignfull):not([class*="__"]),p,h1,h2,h3,h4,h5,h6,ul,ol)-]{+:where([class*="wp-block-"]:not(.alignfull):not([class*="__"]),.wp-block:not(.alignfull),p,h1,h2,h3,h4,h5,h6,ul,ol)+} { padding-right: 0; padding-left: 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; [-}.wp-site-blocks-]{+}:where(.wp-site-blocks)+} > * { margin-block-start: [-0;-]{+2rem;+} margin-block-end: 0; [-}.wp-site-blocks-]{+}:where(.wp-site-blocks)+} > [-* + *-]{+:first-child:first-child+} { margin-block-start: [-2rem;-]{+0; }:where(.wp-site-blocks) > :last-child:last-child { margin-block-end: 0;+} }body { --wp--style--block-gap: 2rem; [-}body .is-layout-flow-]{+}:where(body .is-layout-flow)+}  > [-*{margin-block-start: 0;margin-block-end: 0;}body .is-layout-flow-]{+:first-child:first-child{margin-block-start: 0;}:where(body .is-layout-flow)  > :last-child:last-child{margin-block-end: 0;}:where(body .is-layout-flow)+}  >[-* +-] *{margin-block-start: 2rem;margin-block-end: [-0;}body .is-layout-constrained-]{+0;}:where(body .is-layout-constrained)+}  > [-*{margin-block-start: 0;margin-block-end: 0;}body .is-layout-constrained-]{+:first-child:first-child{margin-block-start: 0;}:where(body .is-layout-constrained)  > :last-child:last-child{margin-block-end: 0;}:where(body .is-layout-constrained)+}  >[-* +-] *{margin-block-start: 2rem;margin-block-end: [-0;}body .is-layout-flex{gap:-]{+0;}:where(body .is-layout-flex) {gap: 2rem;}:where(body .is-layout-grid) {gap:+} 2rem;}body .is-layout-flow > .alignleft{float: left;margin-inline-start: 0;margin-inline-end: 2em;}body .is-layout-flow > .alignright{float: right;margin-inline-start: 2em;margin-inline-end: 0;}body .is-layout-flow > .aligncenter{margin-left: auto !important;margin-right: auto !important;}body .is-layout-constrained > .alignleft{float: left;margin-inline-start: 0;margin-inline-end: 2em;}body .is-layout-constrained > .alignright{float: right;margin-inline-start: 2em;margin-inline-end: 0;}body .is-layout-constrained > .aligncenter{margin-left: auto !important;margin-right: auto !important;}body .is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)){max-width: var(--wp--style--global--content-size);margin-left: auto !important;margin-right: auto !important;}body .is-layout-constrained > .alignwide{max-width: var(--wp--style--global--wide-size);}body .is-layout-flex{display: flex;}body .is-layout-flex{flex-wrap: wrap;align-items: center;}body .is-layout-flex > *{margin: {+0;}body .is-layout-grid{display: grid;}body .is-layout-grid > *{margin:+} 0;}body{background-color: var(--wp--preset--color--base);color: var(--wp--preset--color--contrast);font-family: var(--wp--preset--font-family--system-font);font-size: var(--wp--preset--font-size--medium);line-height: 1.4;--wp--style--root--padding-top: var(--wp--preset--spacing--40);--wp--style--root--padding-right: var(--wp--preset--spacing--30);--wp--style--root--padding-bottom: var(--wp--preset--spacing--40);--wp--style--root--padding-left: var(--wp--preset--spacing--30);}a:where(:not(.wp-element-button)){color: var(--wp--preset--color--contrast);text-decoration: underline;}a:where(:not(.wp-element-button)):hover{text-decoration: none;}a:where(:not(.wp-element-button)):focus{text-decoration: underline dashed;}a:where(:not(.wp-element-button)):active{color: var(--wp--preset--color--secondary);text-decoration: none;}h1, h2, h3, h4, h5, h6{font-family: var(--wp--preset--font-family--raleway);font-weight: 400;line-height: 1.4;}h1{font-size: 5rem;line-height: 1.2;}h2{font-size: var(--wp--preset--font-size--xx-large);line-height: 1.2;}h3{font-size: var(--wp--preset--font-size--x-large);}h4{font-size: 2rem;}h5{font-size: var(--wp--preset--font-size--large);font-weight: 400;text-transform: none;}h6{font-size: var(--wp--preset--font-size--medium);font-weight: 700;text-transform: none;}.wp-element-button, .wp-block-button__link{background-color: var(--wp--preset--color--primary);border-radius: 0;border-width: 0;color: var(--wp--preset--color--base);font-family: inherit;font-size: inherit;font-weight: bold;line-height: inherit;padding: calc(0.667em + 2px) calc(1.333em + 2px);text-decoration: none;}.wp-element-button:visited, .wp-block-button__link:visited{color: var(--wp--preset--color--contrast);}.wp-element-button:hover, .wp-block-button__link:hover{background-color: var(--wp--preset--color--primary);color: var(--wp--preset--color--base);box-shadow: 2px 2px 0 0 var(--wp--preset--color--base), 4px 4px 0 0 var(--wp--preset--color--contrast);}.wp-element-button:focus, .wp-block-button__link:focus{background-color: var(--wp--preset--color--primary);color: var(--wp--preset--color--base);box-shadow: 2px 2px 0 0 var(--wp--preset--color--base), 4px 4px 0 0 var(--wp--preset--color--contrast);}.wp-element-button:active, .wp-block-button__link:active{background-color: var(--wp--preset--color--contrast) !important;color: var(--wp--preset--color--base);box-shadow: 2px 2px 0 0 var(--wp--preset--color--base), 4px 4px 0 0 var(--wp--preset--color--contrast);}.wp-block-pullquote{background: var(--wp--preset--gradient--base);border-width: 1px 0;border-style: solid;color: var(--wp--preset--color--contrast);font-family: var(--wp--preset--font-family--dm-sans);font-size: 1.5em;line-height: 1.3;margin-top: var(--wp--preset--spacing--40) !important;margin-bottom: var(--wp--preset--spacing--40) !important;box-shadow: inset 0 0 0 1px var(--wp--preset--color--contrast), inset 0 0 0 calc( 0.5rem + 1px ) var(--wp--preset--color--base);}.wp-block-pullquote cite{font-size: var(--wp--preset--font-size--small);font-style: normal;text-transform: none;}.wp-block-navigation{font-family: var(--wp--preset--font-family--dm-sans);font-size: var(--wp--preset--font-size--medium);}.wp-block-navigation a:where(:not(.wp-element-button)){color: inherit;text-decoration: none;}.wp-block-navigation a:where(:not(.wp-element-button)):hover{text-decoration: underline;}.wp-block-navigation a:where(:not(.wp-element-button)):focus{text-decoration: underline dashed;}.wp-block-navigation a:where(:not(.wp-element-button)):active{text-decoration: none;}.wp-block-post-author{font-size: var(--wp--preset--font-size--small);}.wp-block-post-content a:where(:not(.wp-element-button)){color: var(--wp--preset--color--contrast);}.wp-block-post-excerpt{font-size: var(--wp--preset--font-size--medium);}.wp-block-post-date{font-size: var(--wp--preset--font-size--small);font-weight: 400;}.wp-block-post-date a:where(:not(.wp-element-button)){text-decoration: none;}.wp-block-post-date a:where(:not(.wp-element-button)):hover{text-decoration: underline;}.wp-block-post-terms{font-size: var(--wp--preset--font-size--small);}.wp-block-post-title{font-family: var(--wp--preset--font-family--dm-sans);font-size: 5rem;font-weight: 400;margin-top: 1.25rem;margin-bottom: 1.25rem;}.wp-block-post-title a:where(:not(.wp-element-button)){text-decoration: none;}.wp-block-post-title a:where(:not(.wp-element-button)):hover{text-decoration: underline;}.wp-block-post-title a:where(:not(.wp-element-button)):focus{text-decoration: underline dashed;}.wp-block-post-title a:where(:not(.wp-element-button)):active{color: var(--wp--preset--color--secondary);text-decoration: none;}.wp-block-comments-title{font-size: var(--wp--preset--font-size--large);margin-bottom: var(--wp--preset--spacing--40);}.wp-block-comment-author-name a:where(:not(.wp-element-button)){text-decoration: none;}.wp-block-comment-author-name a:where(:not(.wp-element-button)):hover{text-decoration: underline;}.wp-block-comment-author-name a:where(:not(.wp-element-button)):focus{text-decoration: underline dashed;}.wp-block-comment-author-name a:where(:not(.wp-element-button)):active{color: var(--wp--preset--color--secondary);text-decoration: none;}.wp-block-comment-date{font-size: var(--wp--preset--font-size--small);}.wp-block-comment-date a:where(:not(.wp-element-button)){text-decoration: none;}.wp-block-comment-date a:where(:not(.wp-element-button)):hover{text-decoration: underline;}.wp-block-comment-date a:where(:not(.wp-element-button)):focus{text-decoration: underline dashed;}.wp-block-comment-date a:where(:not(.wp-element-button)):active{color: var(--wp--preset--color--secondary);text-decoration: none;}.wp-block-comment-edit-link{font-size: var(--wp--preset--font-size--small);}.wp-block-comment-reply-link{font-size: var(--wp--preset--font-size--small);}.wp-block-comments-pagination{margin-top: var(--wp--preset--spacing--40);}.wp-block-comments-pagination a:where(:not(.wp-element-button)){text-decoration: none;}.wp-block-query h2{font-size: var(--wp--preset--font-size--x-large);}.wp-block-query-pagination{font-size: var(--wp--preset--font-size--small);font-weight: 400;}.wp-block-query-pagination a:where(:not(.wp-element-button)){text-decoration: none;}.wp-block-query-pagination a:where(:not(.wp-element-button)):hover{text-decoration: underline;}.wp-block-quote{border-color: var(--wp--preset--color--contrast);border-width: 0 0 0 1px;border-style: solid;padding-right: var(--wp--preset--spacing--30);padding-left: 2rem;}.wp-block-quote cite{font-size: var(--wp--preset--font-size--small);font-style: normal;}.wp-block-site-title{font-family: var(--wp--preset--font-family--dm-sans);font-size: var(--wp--preset--font-size--medium);font-weight: 700;line-height: 1.4;}.wp-block-site-title a:where(:not(.wp-element-button)){text-decoration: none;}.wp-block-site-title a:where(:not(.wp-element-button)):hover{text-decoration: underline;}.wp-block-site-title a:where(:not(.wp-element-button)):focus{text-decoration: underline dashed;}.wp-block-site-title a:where(:not(.wp-element-button)):active{color: var(--wp--preset--color--secondary);text-decoration: none;}.wp-block-cover > .wp-block-cover__image-background, .wp-block-cover > .wp-block-cover__video-background{filter: var(--wp--preset--duotone--primary);}.wp-block-image img, .wp-block-image [-.components-placeholder{filter: var(--wp--preset--duotone--primary);}.wp-block-image img,-]{+.wp-block-image__crop-area,+} .wp-block-image [-.wp-block-image__crop-area{border-radius:-]{+.components-placeholder{border-radius:+} 24px;border-color: var(--wp--preset--color--primary);border-width: 4px;border-style: [-solid;}.wp-block-navigation-link{font-family:-]{+solid;}.wp-block-image img, .wp-block-image .components-placeholder{filter: var(--wp--preset--duotone--primary);}.wp-block-navigation-link{font-family:+} var(--wp--preset--font-family--dm-sans);font-size: var(--wp--preset--font-size--medium);}.wp-block-navigation-link a:where(:not(.wp-element-button)):hover{text-decoration: none;}.wp-block-navigation-link a:where(:not(.wp-element-button)):focus{text-decoration: none;}.wp-block-page-list{font-family: var(--wp--preset--font-family--dm-sans);font-size: var(--wp--preset--font-size--medium);}.wp-block-page-list a:where(:not(.wp-element-button)):hover{text-decoration: none;}.wp-block-page-list a:where(:not(.wp-element-button)):focus{text-decoration: none;}.wp-block-separator{background-color: inherit;color: inherit;}

So I took the CSS from the two lines that were different, formatted them, and diffed them.

First, line 5.

--- /Users/ajlende/Desktop/before#5.css	2023-05-19 11:24:30.000000000 -0500
+++ /Users/ajlende/Desktop/after#5.css	2023-05-19 11:25:07.000000000 -0500
@@ -113,18 +113,6 @@
 		#5dd7b9 71%,
 		#8cdab4
 	);
-	--wp--preset--duotone--dark-grayscale: url('#wp-duotone-dark-grayscale');
-	--wp--preset--duotone--grayscale: url('#wp-duotone-grayscale');
-	--wp--preset--duotone--purple-yellow: url('#wp-duotone-purple-yellow');
-	--wp--preset--duotone--blue-red: url('#wp-duotone-blue-red');
-	--wp--preset--duotone--midnight: url('#wp-duotone-midnight');
-	--wp--preset--duotone--magenta-yellow: url('#wp-duotone-magenta-yellow');
-	--wp--preset--duotone--purple-green: url('#wp-duotone-purple-green');
-	--wp--preset--duotone--blue-orange: url('#wp-duotone-blue-orange');
-	--wp--preset--duotone--contrast: url('#wp-duotone-contrast');
-	--wp--preset--duotone--primary: url('#wp-duotone-primary');
-	--wp--preset--duotone--secondary: url('#wp-duotone-secondary');
-	--wp--preset--duotone--tertiary: url('#wp-duotone-tertiary');
 	--wp--preset--font-size--small: 1rem;
 	--wp--preset--font-size--medium: 1.111rem;
 	--wp--preset--font-size--large: 1.444rem;

This first diff is something that I changed. Those presets were moved in https://github.com/WordPress/gutenberg/blob/trunk/lib/block-supports/duotone.php?rgh-link-date=2023-05-19T00%3A36%3A31Z#L54, noted in the description. This will be ported to core.

Second, line 19.

--- /Users/ajlende/Desktop/before#19.css	2023-05-19 11:24:44.000000000 -0500
+++ /Users/ajlende/Desktop/after#19.css	2023-05-19 11:25:19.000000000 -0500
@@ -27,6 +27,7 @@
 	> .alignfull:where(:not(.has-global-padding))
 	> :where(
 		[class*='wp-block-']:not(.alignfull):not([class*='__']),
+		.wp-block:not(.alignfull),
 		p,
 		h1,
 		h2,
@@ -45,6 +46,7 @@
 	> .alignfull:where(:not(.has-global-padding))
 	> :where(
 		[class*='wp-block-']:not(.alignfull):not([class*='__']),
+		.wp-block:not(.alignfull),
 		p,
 		h1,
 		h2,
@@ -71,33 +73,43 @@
 	margin-left: auto;
 	margin-right: auto;
 }
-.wp-site-blocks > * {
-	margin-block-start: 0;
+:where(.wp-site-blocks) > * {
+	margin-block-start: 2rem;
 	margin-block-end: 0;
 }
-.wp-site-blocks > * + * {
-	margin-block-start: 2rem;
+:where(.wp-site-blocks) > :first-child:first-child {
+	margin-block-start: 0;
+}
+:where(.wp-site-blocks) > :last-child:last-child {
+	margin-block-end: 0;
 }
 body {
 	--wp--style--block-gap: 2rem;
 }
-body .is-layout-flow > * {
+:where(body .is-layout-flow) > :first-child:first-child {
 	margin-block-start: 0;
+}
+:where(body .is-layout-flow) > :last-child:last-child {
 	margin-block-end: 0;
 }
-body .is-layout-flow > * + * {
+:where(body .is-layout-flow) > * {
 	margin-block-start: 2rem;
 	margin-block-end: 0;
 }
-body .is-layout-constrained > * {
+:where(body .is-layout-constrained) > :first-child:first-child {
 	margin-block-start: 0;
+}
+:where(body .is-layout-constrained) > :last-child:last-child {
 	margin-block-end: 0;
 }
-body .is-layout-constrained > * + * {
+:where(body .is-layout-constrained) > * {
 	margin-block-start: 2rem;
 	margin-block-end: 0;
 }
-body .is-layout-flex {
+:where(body .is-layout-flex) {
+	gap: 2rem;
+}
+:where(body .is-layout-grid) {
 	gap: 2rem;
 }
 body .is-layout-flow > .alignleft {
@@ -148,6 +160,12 @@
 body .is-layout-flex > * {
 	margin: 0;
 }
+body .is-layout-grid {
+	display: grid;
+}
+body .is-layout-grid > * {
+	margin: 0;
+}
 body {
 	background-color: var(--wp--preset--color--base);
 	color: var(--wp--preset--color--contrast);
@@ -415,16 +433,17 @@
 	filter: var(--wp--preset--duotone--primary);
 }
 .wp-block-image img,
+.wp-block-image .wp-block-image__crop-area,
 .wp-block-image .components-placeholder {
-	filter: var(--wp--preset--duotone--primary);
-}
-.wp-block-image img,
-.wp-block-image .wp-block-image__crop-area {
 	border-radius: 24px;
 	border-color: var(--wp--preset--color--primary);
 	border-width: 4px;
 	border-style: solid;
 }
+.wp-block-image img,
+.wp-block-image .components-placeholder {
+	filter: var(--wp--preset--duotone--primary);
+}
 .wp-block-navigation-link {
 	font-family: var(--wp--preset--font-family--dm-sans);
 	font-size: var(--wp--preset--font-size--medium);

I am using the typography features that it seemed experimental part dealt with, and they don't show in the diff, so at least, they aren't affecting the $settings['styles'] that this hook overrides.


Hopefully this helps others think through the implications of this change. I, personally, think it's still worthwhile to try using a lower priority than PHP_INT_MAX, and I still think 0 is the right choice after my research.

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @ajlende!

Let's merge this. We can change hook priority in the future if needed.

@ajlende ajlende merged commit 7bac5ee into trunk May 22, 2023
@ajlende ajlende deleted the fix/gutenberg_get_block_editor_settings branch May 22, 2023 18:17
@github-actions github-actions bot added this to the Gutenberg 15.9 milestone May 22, 2023
@ramonjd ramonjd added the Needs PHP backport Needs PHP backport to Core label Jun 5, 2023
@ramonjd ramonjd removed the Needs PHP backport Needs PHP backport to Core label Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants