-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Navigation: Update the fallback block list to avoid a PHP Warning #58588
Navigation: Update the fallback block list to avoid a PHP Warning #58588
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core SVNIf you're a Core Committer, use this list when committing to
GitHub Merge commitsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Thanks @dd32 I'll try and get around to reviewing this today! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch. I've confirmed there is a PHP error when:
- Users Navigation block does not reference a post ID aka have a
ref
attribute. - There is either no
wp_navigation
post in the database, or there is but itspost_content
is empty after filtering out null blocks.
In the above scenario it will just return a core/page-list
block as the fallback for the sites Navigation.
Block Hooks API does assume block data is present when traversing and serializing these inner blocks to insert hooked blocks, which is where the error occurs as you've pointed out.
I've confirmed this PR does fix the error and the Block Hooks work continues to work as expected for the navigation so I'm going to approve.
cc @ockham and @youknowriad I'm not sure what next steps are for this. Does it need to be included in the next RC/release? I'm unfamiliar with the process myself.
This is in the block-library package and Gutenberg 17.7 will be entirely included in WordPress 6.5, which means there's no follow-ups required to this PR, it will be automatically backported to Core in the next package release/update. |
Co-authored-by: dd32 <dd32@git.wordpress.org> Co-authored-by: tjcafferkey <tomjcafferkey@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Thanks a lot for the fix @dd32. Apologies, I missed this earlier.
I think this place is fine 👍
It might not hurt to add an |
What?
This PR avoids PHP Warnings in
traverse_and_serialize_block()
which expects thatinnerContent
andattrs
are set.I'm unsure if this is the proper place for the fix, or if the above function needs updating to properly handle a
$block
variable of[ 'blockName' => 'core/block-name' ]
without the before mentioned parameters. I can see reasons for both.Based on
gutenberg/packages/block-library/src/navigation/index.php
Lines 1376 to 1381 in b859f54
innerBlocks
too like that?It appears this was recently introduced with #57754 cc @tjcafferkey
Why?
Note: These warnings are on wordpress.org/showcase with Gutenberg 17.x, warning weren't present on 16.8
The code as-is, causes the following notices on WordPress Trunk:
How?
Sets the missing array indexes.
Testing Instructions
Unknown. It's triggered on wordpress.org/showcase, I'm assuming that a navigation menu block is used without a fallback menu present.