Global styles values are not reset in site editor under certain scenarios

[oandregal]

Take the following steps:

• Using TwentyTwentyTwo and any WordPress version after ​rev54517, for example, WordPress 6.2 beta 3.
• Go to the site editor.
• Open the global styles sidebar and set the padding values to something else (222px, for example).
• Save the changes and reload the editor.
• Open the global styles sidebar and try resetting the padding values (for example, by using the "Reset to defaults" option displayed under the three dot menu of the global styles sidebar).

The expected result is that the values would be reset. However, they are not.

Note that if this is saved and then site editor reloaded, the proper data is shown.

Trac ticket https://core.trac.wordpress.org/ticket/57824
Reverts ​https://github.com/WordPress/wordpress-develop/pull/3441

## What

This PR fixes a bug by which the reset function of the global styles sidebar wouldn't work as expected in the site editor.

## How

By reverting ​https://github.com/WordPress/wordpress-develop/pull/3441 which was the one that introduced the issue.

That PR changed the get_merged_data code to (pseudo-code):

{{{php
function get_merged_data( $origin ) {

$result = static::get_core_data();

if ( 'block' === $origin ) { $result->merge( static::get_block_data() ); }
if ( 'theme' === $origin ) { $result->merge( static::get_theme_data() ); }
if ( 'custom' === $origin ) { $result->merge( static::get_custom_data() ); }

return $result;

}
}}}

However, by doing so, the base object ($result) is modifying the $core object directly, and $core ends up storing the consolidated values instead of only the ones coming from the theme.json provided by core.

This is problematic because $core data is cached. Take, for example, the following scenario:

{{{php
$data = get_merged_data( 'custom' );
$data = get_merged_data( 'theme' );
}}}

The expected output for $data is that it should not have data coming from the 'custom' origin, however, it does.

The fix is reverting the change and use an empty object as base (pseudo-code):

{{{php
function get_merged_data( $origin ) {

$result = new WP_Theme_JSON();

$result->merge( static::get_core_data() );
if ( 'block' === $origin ) { $result->merge( static::get_block_data() ); }
if ( 'theme' === $origin ) { $result->merge( static::get_theme_data() ); }
if ( 'custom' === $origin ) { $result->merge( static::get_custom_data() ); }

return $result;

}
}}}

cc @youknowriad as the original reporter

@ntsekouras @Mamaduka as editor tech leads: can this bug be fixed for 6.2 despite it being introduced in the 6.1 cycle?

can this bug be fixed for 6.2 despite it being introduced in the 6.1 cycle?

Yes, but it might be something to be also included in a 6.1 patch release.

So this did solve the issue for me without Gutenberg but with Gutenberg it's still broken, I'm assuming this class is overridden in Gutenberg?

So this did solve the issue for me without Gutenberg but with Gutenberg it's still broken, I'm assuming this class is overridden in Gutenberg?

Yeah, ported it at ​https://github.com/WordPress/gutenberg/pull/48644

[hellofromTonya]

Revert of [54517]. See #56467 from 6.1.0 cycle.

Marking as a gutenberg-merge for tracking.

[audrasjb]

In 55448:

Editor: Ensure Global styles values are reset in the site editor.

This changeset fixes a bug by which the reset function of the global styles sidebar would not work as expected in the site editor. It reverts [54517] and adds related unit tests.

Props oandregal, ntsekouras, youknowriad, hellofromTonya.
Fixes #57824
See #56467

Committed in https://core.trac.wordpress.org/changeset/55448

Trac ticket https://core.trac.wordpress.org/ticket/57824

The test introduced in ​https://github.com/WordPress/wordpress-develop/pull/4145 wasn't actually reporting the bug. Unless we find a way in that it works to signal a regression, it should be removed to avoid an illusion of security.

Not a blocker, but let's follow up with the tests after Beta 4 to add a $message parameter to each of the assertions and tidy up the docblock and multiline comments. slightly_smiling_face

I prepared ​https://github.com/WordPress/wordpress-develop/pull/4157 to remove the test, as it was still not working as expected (to catch the regression, as mentioned at ​https://github.com/WordPress/wordpress-develop/pull/4145#discussion_r1120471830). Unless we find a way in that it works to signal a regression, it should be removed to avoid an illusion of security.

[oandregal]

Since this bug has been introduced in the 6.1 cycle, I understand we'll want to cherry-pick the fix to that branch as well.

I see this ticket is marked as 6.1 version, is that enough? Will someone pick this up when time comes? Or should I go ahead and cherry-pick the fix for that branch? I haven't done this before, so I may be missing very obvious things, happy to learn!