From e73ded515ec46789af887831041b340a42cee467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 28 Feb 2023 17:39:56 +0100 Subject: [PATCH 1/4] Add test --- .../tests/theme/wpThemeJsonResolver.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/phpunit/tests/theme/wpThemeJsonResolver.php b/tests/phpunit/tests/theme/wpThemeJsonResolver.php index 9b9185b224a39..0270c46e03136 100644 --- a/tests/phpunit/tests/theme/wpThemeJsonResolver.php +++ b/tests/phpunit/tests/theme/wpThemeJsonResolver.php @@ -848,6 +848,40 @@ static function( $element ) { } + public function test_get_merged_data_returns_origin_proper(){ + // Make sure the theme has a theme.json + // though it doesn't have any data for styles.spacing.padding. + switch_theme( 'block-theme' ); + + // Make sure the user defined some data for styles.spacing.padding. + wp_set_current_user( self::$administrator_id ); + $user_cpt = WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme(), true ); + $config = json_decode( $user_cpt['post_content'], true ); + $config['styles']['spacing']['padding'] = array( + 'top' => '23px', + 'left' => '23px', + 'bottom' => '23px', + 'right' => '23px', + ); + $user_cpt['post_content'] = wp_json_encode( $config ); + wp_update_post( $user_cpt, true, false ); + + // Query data from the user origin and then for the theme origin. + $theme_json_user = WP_Theme_JSON_Resolver::get_merged_data( 'custom' ); + $padding_user = $theme_json_user->get_raw_data()['styles']['spacing']['padding']; + $theme_json_theme = WP_Theme_JSON_Resolver::get_merged_data( 'theme' ); + $padding_theme = $theme_json_theme->get_raw_data()['styles']['spacing']['padding']; + + $this->assertSame( '23px', $padding_user['top'] ); + $this->assertSame( '23px', $padding_user['right'] ); + $this->assertSame( '23px', $padding_user['bottom'] ); + $this->assertSame( '23px', $padding_user['left'] ); + $this->assertSame( '0px', $padding_theme['top'] ); + $this->assertSame( '0px', $padding_theme['right'] ); + $this->assertSame( '0px', $padding_theme['bottom'] ); + $this->assertSame( '0px', $padding_theme['left'] ); + } + /** * Data provider. * From a59f4403b92ab9fc73e7d3e0f45e3be9104b0dcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 28 Feb 2023 17:41:04 +0100 Subject: [PATCH 2/4] get_merged_data: use empty theme.json object as base instead of core The PR https://github.com/WordPress/wordpress-develop/pull/3441 introduced a bug by which the get_merged_data method is not cleared in certain situations. By doing (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; } ``` the base object ($result) is modifying the $core object directly, and $core ends up storing the consolidated values instead of only the $core ones. 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; } ``` --- src/wp-includes/class-wp-theme-json-resolver.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/class-wp-theme-json-resolver.php b/src/wp-includes/class-wp-theme-json-resolver.php index 2e6b7523b695c..d352cbdcbfdc8 100644 --- a/src/wp-includes/class-wp-theme-json-resolver.php +++ b/src/wp-includes/class-wp-theme-json-resolver.php @@ -559,7 +559,8 @@ public static function get_merged_data( $origin = 'custom' ) { _deprecated_argument( __FUNCTION__, '5.9.0' ); } - $result = static::get_core_data(); + $result = new WP_Theme_JSON(); + $result->merge( static::get_core_data() ); if ( 'default' === $origin ) { $result->set_spacing_sizes(); return $result; From 43495d097b43798786e2668e24db633ddea5f074 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 28 Feb 2023 18:28:41 +0100 Subject: [PATCH 3/4] Test: add comment and fix lint issues --- .../tests/theme/wpThemeJsonResolver.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/phpunit/tests/theme/wpThemeJsonResolver.php b/tests/phpunit/tests/theme/wpThemeJsonResolver.php index 0270c46e03136..cf23d815ba8ee 100644 --- a/tests/phpunit/tests/theme/wpThemeJsonResolver.php +++ b/tests/phpunit/tests/theme/wpThemeJsonResolver.php @@ -848,7 +848,16 @@ static function( $element ) { } - public function test_get_merged_data_returns_origin_proper(){ + /** + * Tests that get_merged_data returns the data merged up to the proper origin + * and that the core values have the proper data. + * + * @ticket 57824 + * + * @covers WP_Theme_JSON_Resolver::get_merged_data + * + */ + public function test_get_merged_data_returns_origin_proper() { // Make sure the theme has a theme.json // though it doesn't have any data for styles.spacing.padding. switch_theme( 'block-theme' ); @@ -858,10 +867,10 @@ public function test_get_merged_data_returns_origin_proper(){ $user_cpt = WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme(), true ); $config = json_decode( $user_cpt['post_content'], true ); $config['styles']['spacing']['padding'] = array( - 'top' => '23px', - 'left' => '23px', - 'bottom' => '23px', - 'right' => '23px', + 'top' => '23px', + 'left' => '23px', + 'bottom' => '23px', + 'right' => '23px', ); $user_cpt['post_content'] = wp_json_encode( $config ); wp_update_post( $user_cpt, true, false ); From 1029bb2a07262c9663b3f40ac0636a741e8a9f7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Tue, 28 Feb 2023 18:33:08 +0100 Subject: [PATCH 4/4] Align equal signs in the way the linter wants --- tests/phpunit/tests/theme/wpThemeJsonResolver.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/theme/wpThemeJsonResolver.php b/tests/phpunit/tests/theme/wpThemeJsonResolver.php index cf23d815ba8ee..b64953194367d 100644 --- a/tests/phpunit/tests/theme/wpThemeJsonResolver.php +++ b/tests/phpunit/tests/theme/wpThemeJsonResolver.php @@ -864,15 +864,15 @@ public function test_get_merged_data_returns_origin_proper() { // Make sure the user defined some data for styles.spacing.padding. wp_set_current_user( self::$administrator_id ); - $user_cpt = WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme(), true ); - $config = json_decode( $user_cpt['post_content'], true ); + $user_cpt = WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme(), true ); + $config = json_decode( $user_cpt['post_content'], true ); $config['styles']['spacing']['padding'] = array( 'top' => '23px', 'left' => '23px', 'bottom' => '23px', 'right' => '23px', ); - $user_cpt['post_content'] = wp_json_encode( $config ); + $user_cpt['post_content'] = wp_json_encode( $config ); wp_update_post( $user_cpt, true, false ); // Query data from the user origin and then for the theme origin.