WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.34k stars 4.13k forks source link

Site Logo Block: problems with Core/Plugin block loading #33177

Open creativecoder opened 3 years ago

creativecoder commented 3 years ago

Description

Issues with loading the Site Logo block

Gutenberg plugin with WP 5.7.2

The hooks that update the site_logo option from the custom_logo theme mod, when the logo is updated in the Customizer, do not run. The result is that the logo cannot be updated from the Customizer.

I believe this is happening because _sync_custom_logo_to_site_logo is added via the setup_theme action, which has already been executed when the site-logo.php file is included, so the hooking into setup_theme has no effect at that point.

Gutenberg plugin with WP 5.8 RC

Both the Core php file and the plugin php file for the block are included. This results in all hooked functions being registered twice. I haven't seen any bugs with this yet, but I'm sure it could lead to unexpected results.

Step-by-step reproduction instructions

Expected behaviour

The logo image set in the Customizer shows as the site logo

Actual behaviour

The previous logo is displayed.

WordPress information

Device information

kathrynwp commented 2 years ago

@creativecoder As it looks like this issue was fixed in this PR I'm going to go ahead and close this out, but if you're still experiencing the original issue, feel free to add a comment with additional details and I’ll be glad to reopen it.

creativecoder commented 2 years ago

Hi @kathrynwp, thanks for taking a look. This is actually a different issue than what that PR addressed.

The fundamental problem has to do with

For the Site logo block, now that the Core and Gutenberg versions are more in sync, there's probably no noticeable effect of either of those problems. But they are both lurking beneath the surface waiting to cause bugs when running the Gutenberg plugin.

kathrynwp commented 2 years ago

Thanks for the clarification, I've reopened this!

annezazu commented 9 months ago

👋🏼 Hey folks. I'm doing a sweep of high priority labeled issues to ensure the label remains actionable and relevant. As a result, I've removed the label from this issue because the work here has stalled and it's not clear whether this problem remains. In order to add the label back, @creativecoder can you provide a current status and confirm whether you believe it needs to be high priority so we can mobilize efforts? Ideally, anything with that label is getting actively worked on and moved forward.

creativecoder commented 9 months ago

Thank @annezazu ! It appears the underlying problem is still there, as I don't see any changes to how site-logo/index.php is loaded in Gutenberg.

However, this is no longer high priority, as the site-logo block php file is duplicated in Core, where it is loaded earlier (before the setup_theme hook) and so doesn't have the same problem with loading/hook order.

So this is now more a latent issue in Gutenberg that will reveal itself if

annezazu commented 9 months ago

Thank you so much for this detailed recap and for helping confirm the label removal 🙌🏼