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

PHP: backport from 6.1 again. #46363

Closed
wants to merge 5 commits into from
Closed

PHP: backport from 6.1 again. #46363

wants to merge 5 commits into from

Conversation

spacedmonkey
Copy link
Member

What?

Change from WordPress/wordpress-develop@4be7661 by @hellofromtonya.

Fixes issue reported by #46250 (comment) by @mirka

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

Comment on lines 27 to 46
/**
* Container for keep track of registered blocks.
*
* @since 6.1.0
* @var array
*/
protected static $blocks_cache = array(
'core' => array(),
'blocks' => array(),
'theme' => array(),
'user' => array(),
);

/**
* Container for data coming from the blocks.
*
* @since 6.1.0
* @var WP_Theme_JSON
*/
protected static $blocks = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes were not part of Core commit https://core.trac.wordpress.org/changeset/54493. I'll revert them.

Comment on lines 80 to 89
/**
* Checks whether the registered blocks were already processed for this origin.
*
* @since 6.1.0
*
* @param string $origin Data source for which to cache the blocks.
* Valid values are 'core', 'blocks', 'theme', and 'user'.
* @return bool True on success, false otherwise.
*/
protected static function has_same_registered_blocks( $origin ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with this one. 6.1.0 did not add this method to WP_Theme_JSON_Data. I'll revert this change too.

Comment on lines 27 to 34
/**
* Container for data coming from the blocks.
*
* @since 6.1.0
* @var WP_Theme_JSON
*/
protected static $blocks = null;

Copy link
Contributor

Choose a reason for hiding this comment

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

This change is not in Core 6.1. Needs to be reverted too.

}

return false;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Also need to add the changes for WP_Theme_JSON_Resolver::get_theme_data(). Why? Gutenberg's class extends from WP_Theme_JSON_Resolver_6_0 which will overload Core's get_theme_data() method, meaning Core's won't be used. That can cause unexpected differences in the way the plugin works vs Core.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
/**
* Returns the theme's data.
*
* Data from theme.json will be backfilled from existing
* theme supports, if any. Note that if the same data
* is present in theme.json and in theme supports,
* the theme.json takes precedence.
*
* @since 5.8.0
* @since 5.9.0 Theme supports have been inlined and the `$theme_support_data` argument removed.
* @since 6.0.0 Added an `$options` parameter to allow the theme data to be returned without theme supports.
*
* @param array $deprecated Deprecated. Not used.
* @param array $options {
* Options arguments.
*
* @type bool $with_supports Whether to include theme supports in the data. Default true.
* }
* @return WP_Theme_JSON Entity that holds theme data.
*/
public static function get_theme_data( $deprecated = array(), $options = array() ) {
return WP_Theme_JSON_Resolver::get_theme_data( $deprecated, $options );
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

The above suggested code will bypass WP_Theme_JSON_Resolver_6_0::get_theme_data() implementation, to call the version in Core 6.1.

Copy link
Contributor

Choose a reason for hiding this comment

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

But wait a minute. This won't work if the site is running < WP 6.1 with the plugin. So the entire WP_Theme_JSON_Resolver::get_theme_data() code needs to be copied here it seems.

Comment on lines +28 to +40
/**
* Container for keep track of registered blocks.
*
* @since 6.1.0
* @var array
*/
protected static $blocks_cache = array(
'core' => array(),
'blocks' => array(),
'theme' => array(),
'user' => array(),
);

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/**
* Container for keep track of registered blocks.
*
* @since 6.1.0
* @var array
*/
protected static $blocks_cache = array(
'core' => array(),
'blocks' => array(),
'theme' => array(),
'user' => array(),
);

Not needed, as this class will inherit it from Core's WP_Theme_JSON_Resolver class.

Copy link
Contributor

Choose a reason for hiding this comment

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

After talking with Jonny, this code is needed for sites that are running < WP 6.1 with the plugin.

Also need:

protected static $blocks = null;

this too.

Sorry for the confusion.

@spacedmonkey
Copy link
Member Author

@hellofromtonya What do you want to me to do?

@oandregal
Copy link
Member

I was looking at this fatal error with Gutenberg 14.8.2 and WordPress 6.0 introduced at #46250 and found this PR was missing from the 14.8 release. The issue is now fixed as of #46750 and this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants