Global Styles: Prevent loss of custom variation data in API response

[aaronrobertshaw]

Fixes: #62303

What?

Ensures custom block style variation data isn't omitted from the global styles API response when saving styles in the site editor.

Why?

When saving global styles data without this fix, any tweaks to custom block style variations are omitted from the global styles endpoint response. This missing data makes it appear as though the tweaks have been lost until the editor is reloaded.

How?

Within the prepare_item_for_response method of the global styles controller, a fresh theme json instance is created. For such a request this never triggers the theme json data filters that custom block style variations rely on to register custom block styles. Unregistered block style variations get stripped out during theme json sanitization.

Rather than do all the heavy lifting of parsing theme.json partial files for the theme, applying data filters etc. this PR registers the custom block style variations found in the global styles post, directly.

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.

1. Comment out the duotone support actions that masked this issue
2. Add a custom block style variation to your theme.json (see Section styles: editing a block variation in Styles doesn't reflect in the editor #62303 for snippet)
3. Navigate to Appearance > Editor
4. Select a group block and apply the custom variation
5. Navigate to Styles > Blocks > Group > dark (variation name)
6. Change the background color for the variation and confirm it is reflected in the editor
7. Save the changes and confirm the background color is still correct in the editor
8. For good measure check the frontend
9. Uncomment the duotone support actions from step 1 and repeat the process. It should still work.

Screenshots or screencast

Before	After
Screen.Recording.2024-06-05.at.6.49.43.PM.mp4	Screen.Recording.2024-06-05.at.6.50.31.PM.mp4

[github-actions]

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: aaronrobertshaw <[email protected]>
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: andrewserong <[email protected]>
Co-authored-by: annezazu <[email protected]>

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

[github-actions]

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-rest-global-styles-controller-gutenberg.php

[aaronrobertshaw]

I did start on a unit test for this but got stuck with the global styles post not being updated and therefore getting unexpected/missing styles data in the test only. I'll try and sort this out tomorrow.

Snippet for unit test

diff --git a/phpunit/class-wp-rest-global-styles-controller-gutenberg-test.php b/phpunit/class-wp-rest-global-styles-controller-gutenberg-test.php
index f5b216a084..ff5b291b2f 100644
--- a/phpunit/class-wp-rest-global-styles-controller-gutenberg-test.php
+++ b/phpunit/class-wp-rest-global-styles-controller-gutenberg-test.php
@@ -513,6 +513,39 @@ class WP_REST_Global_Styles_Controller_Gutenberg_Test extends WP_Test_REST_Contr
 		$this->assertErrorResponse( 'rest_custom_css_illegal_markup', $response, 400 );
 	}
 
+	/**
+	 * @covers WP_REST_Global_Styles_Controller_Gutenberg::update_item
+	 */
+	public function test_update_item_with_custom_block_style_variations() {
+		wp_set_current_user( self::$admin_id );
+		if ( is_multisite() ) {
+			grant_super_admin( self::$admin_id );
+		}
+		$variations = array(
+			'dark' => array(
+				'color' => array(
+					'background' => '#000000',
+					'text'       => '#ffffff',
+				),
+			),
+		);
+		$request    = new WP_REST_Request( 'PUT', '/wp/v2/global-styles/' . self::$global_styles_id );
+		$request->set_body_params(
+			array(
+				'styles' => array(
+					'blocks' => array(
+						'core/group' => array(
+							'variations' => $variations,
+						),
+					),
+				),
+			)
+		);
+		$response = rest_get_server()->dispatch( $request );
+		$data     = $response->get_data();
+		$this->assertSame( $variations, $data['styles']['blocks']['core/group']['variations'] );
+	}
+
 	/**
 	 * @doesNotPerformAssertions
 	 */

[ramonjd]

I made some videos!

Things are working well for me. I tested using the provided snippet and modified Group and Column variations.

Before	After
2024-06-07.11.18.08.mp4	2024-06-07.11.19.00.mp4

I also tested loading and saving global styles revisions that have block variations.

The styles are loaded as expected.

I did start on a unit test for this but got stuck with the global styles post not being updated and therefore getting unexpected/missing styles data in the test only.

It seems to be something specific to the variations key. Throwing a general block style at it works.

The styles are stripped either before or after updating the post.

The incoming data to wp_update_post looks sound in that it contains the variation array.

However what get_post returns is not. $post->post_content is {"version":3,"isGlobalStylesUserThemeJSON":true}

I'm not sure why yet 😄

[aaronrobertshaw]

Thanks for digging into the test issue @ramonjd

The incoming data to wp_update_post looks sound in that it contains the variation array.

However what get_post returns is not. $post->post_content is {"version":3,"isGlobalStylesUserThemeJSON":true}

This is what I was seeing as well. I also tried creating an entirely separate post and testing the results of get_item but again there the variations data was missing. I can't work out what's going on just yet.

Do you think this fix is worth landing and following up on the test separately?

Maybe not as I can't see the backport for the fix being accepted without the test. My concern was more towards preventing this from blocking people testing the section styles feature in the 6.6 beta.

[andrewserong]

I don't have anything helpful to add, unfortunately, but just confirming that in the test environment when logging out the $post in the following line, I'm seeing what you're both seeing (the variations data was there prior to calling wp_update_post, but isn't in the response from get_post( $request['id'] )).

gutenberg/lib/class-wp-rest-global-styles-controller-gutenberg.php

Line 283 in 1380687

$post = get_post( $request['id'] );

It's very confusing, I wonder if there's a hook running that's doing an additional round of sanitization? 🤔
I've run out of time for the week to keep testing, but happy to help out next week if this is still open!

[ramonjd]

This is what I was seeing as well. I also tried creating an entirely separate post and testing the results of get_item but again there the variations data was missing. I can't work out what's going on just yet.

Very interesting.

Fetching this endpoint — await wp.apiFetch( { path: '/wp/v2/global-styles/11' } ); — calls the controller's method get_item directly. In the response I'm seeing the variations with the correct values after I've updated theme in the Site Editor.

Same goes for the POST request when updating them.

So why doesn't the test do this 😆

Must be missing something.

Do you think this fix is worth landing and following up on the test separately?

I agree it would be good to work out why the tests fail first, in case it's hiding a bigger issue.

[aaronrobertshaw]

It looks like the variations are being filtered due to the content save filters added in Gutenberg and Core.

My hunch is these filters get called before we have other block style variations registered via the theme.json data filters. So the variations are sanitized by the WP_Theme_JSON class. Basically, the same issue this PR was trying to solve for the update endpoint.

[ramonjd]

It looks like the variations are being filtered due to the content save filters added in Gutenberg and Core.

Great sleuthing. Another piece of the puzzle falls into place... 😄

[aaronrobertshaw]

I've updated the kses content save filter and the test is passing. From a quick test of the editor it appears the original change in the global styles controller is still required.

This all seems to point to the sanitization of variations no longer being useful. Long term, I believe the plan is to eventually allow the creation of block style variations via the Global Styles UI. At that point we'll probably need to remove the sanitization of variations too.

I'll leave this PR open for the moment in case we do need a quick fix for the WP6.6 beta. In the meantime, I'll work through what's required to remove the validation of variations without reintroducing the issues #49807 intended to solve.

[aaronrobertshaw]

Closing in favour of #62405