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

Introduce a custom version of the enqueue_block_styles_assets function #36089

Conversation

david-binda
Copy link
Contributor

Description

The WordPress enqueue_block_styles_assets function contains a bug which may lead to fatal errors for full site editing enabled themes which register custom styles and are missing a HTML template for request (for instance 404.html). As described in #35912 and #35903

The bug was patched in WordPress core ( see https://core.trac.wordpress.org/ticket/54323 ), but since the Gutenberg
11.8.0 introduced the following line in 16633ff

add_filter( 'should_load_separate_core_block_assets', '__return_true' );

it, in my humble opinion, also should patch the issue, in order to prevent fatal errors for the themes matching the above mentioned criteria.

How has this been tested?

In the WordPress 5.8 install, I have enabled the latest version of the blockbase theme (see https://github.com/Automattic/themes/tree/trunk/blockbase ) which does not include the 404.html block-template (which is still present in the version distributed via WordPress.org themes repository).

Further, I have enabled the Gutenberg v11.8.0 and experienced a fatal error on a URL which results in 404 page, and experienced a fatal error.

Applying the patch introduced in this PR fixed the fatal error.

Screenshots

Types of changes

Introduces a custom override of the enqueue_block_styles_assets with a patch introduced to WordPress in https://core.trac.wordpress.org/changeset/51941

The original function is being unhooked from the enqueue_block_assets and the new Gutenberg override is hooked.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

The WordPress `enqueue_block_styles_assets` function contains a bug
which may lead to fatal errors for full site editing enabled themes
which register custom styles and are missing a HTML template for
request (for instance 404.html).

The bug was patched in WordPress core ( see
https://core.trac.wordpress.org/ticket/54323 ), but since the Gutenberg
11.8.0 introduces the following line:

```
add_filter( 'should_load_separate_core_block_assets', '__return_true' );
```

it also should patch the issue, in order to prevent fatal errors for the
themes matching the above mentioned criteria.

See WordPress#35912 and WordPress#35903
Add the reference to the WordPress `enqueue_block_styles_assets` function to docblock
Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

Seems pretty straight-forward... basically a copy-paste of the updated function from core.
However, I think we should only do this for older versions of WP and not for versions where the core patch has already been applied. 👍

Comment on lines 74 to 75
remove_action( 'enqueue_block_assets', 'enqueue_block_styles_assets', 30 );
add_action( 'enqueue_block_assets', 'gutenberg_enqueue_block_styles_assets', 30 );
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this only run if the WP version is lower than 5.8.2?
Since the patch has already been applied to core, it makes sense to only target previous versions.

Suggested change
remove_action( 'enqueue_block_assets', 'enqueue_block_styles_assets', 30 );
add_action( 'enqueue_block_assets', 'gutenberg_enqueue_block_styles_assets', 30 );
if ( version_compare( get_bloginfo( 'version' ), '5.8.2', '<' ) ) {
remove_action( 'enqueue_block_assets', 'enqueue_block_styles_assets', 30 );
add_action( 'enqueue_block_assets', 'gutenberg_enqueue_block_styles_assets', 30 );
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion @aristath ! You're right, there is no need to replace the callback by the patched version, if WordPress 5.8.2 is patched.

I have addressed that by the code you are suggesting here, and have also added an extra check for making sure the callback is only being replaced when it's actually hooked (eg.: in WordPress < 5.3 and possibly when removed by developer)

@skorasaurus
Copy link
Member

Can this be closed since the Plugin now requires WP 6.0?

@t-hamano
Copy link
Contributor

Can this be closed since the Plugin #46102?

I think so too. As of the time I write this comment, the Gutenberg plugin requires WordPress 6.3.

@david-binda Thank you for your work!

@t-hamano t-hamano closed this Dec 16, 2023
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.

4 participants