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

Initialize Block Theme Previews hooks on plugins_loaded action #5107

Closed

Conversation

okmttdhr
Copy link

@okmttdhr okmttdhr commented Aug 29, 2023

This PR ensures theme preview hooks are added after the plugins_loaded action fires. This change aligns with best practices and helps avoid the fatal error on Block Theme Previews.

  • Encapsulated the action and filter hooks related to theme previews into a function initialize_theme_preview_hooks
  • Hooked initialize_theme_preview_hooks to the plugins_loaded action in default-filters.php

Trac ticket: https://core.trac.wordpress.org/ticket/59000
Fixes: WordPress/gutenberg#53284

Why

Currently, the action and filter hooks related to theme previews are added in the global scope, which could lead to unexpected behaviors with other plugins or internal functionalities. See https://core.trac.wordpress.org/ticket/59000#comment:15 and WordPress/gutenberg#53284. This change ensures that the hooks are added at the appropriate time in the lifecycle, specifically after all plugins have been loaded.

Testing

  • Navigate to /wp-admin/themes.php and click the Live Preview button on any Block theme (e.g. Twenty Twenty-Two).
  • Verify that Block Theme Previews functionality works as expected.
  • Test with third-party plugins, such as CoBlocks, to ensure there is no fatal error on Block Theme Previews.

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@okmttdhr okmttdhr marked this pull request as ready for review August 29, 2023 06:05
@okmttdhr okmttdhr force-pushed the fix/theme-preview-init-hooks branch from 57c71db to d691049 Compare August 29, 2023 06:07
@azaozz
Copy link
Contributor

azaozz commented Aug 29, 2023

Thinking this looks good, thanks.

Wondering if any unit tests may be possible here? Or maybe better to add tests for the underlying problem: calling $wp_theme->get_stylesheet() before pluggable.php is loaded.

@okmttdhr okmttdhr force-pushed the fix/theme-preview-init-hooks branch 2 times, most recently from 3870a46 to 2381d8a Compare August 30, 2023 05:47
@okmttdhr
Copy link
Author

Thank you for your review, @azaozz!
I could write a test to guarantee that plugin_loaded triggers the initialization. What do you think?

@okmttdhr okmttdhr requested a review from azaozz August 30, 2023 05:51
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
@okmttdhr
Copy link
Author

okmttdhr commented Sep 5, 2023

Hello, @azaozz @mukeshpanchal27 @mtias 👋
Could you help us move this forward and fix the fatal error on theme previews?

@okmttdhr
Copy link
Author

📝 It's committed to trunk in https://core.trac.wordpress.org/changeset/56529; leaving the PR open for backporting to the 6.3.2.

@okmttdhr okmttdhr closed this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants