Make WordPress Core

Opened 12 months ago

Last modified 7 weeks ago

#58932 reviewing defect (bug)

wp_setup_nav_menu_item() throws PHP warning when using virtual menu-items

Reported by: apedog's profile apedog Owned by: joemcgill's profile joemcgill
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.4
Component: Menus Keywords: has-patch reporter-feedback
Focuses: Cc:

Description

I believe this bug was introduced in 47211

TLDR

wp_setup_nav_menu_item() calls get_post() but does no sanity checking before calling get_post_states() with what might be a null. Adding a simple if ( $menu_post !== null ) before calling get_post_states() resolves this issue.

I've added a patch.

Details

I have an old plugin (pre-dating above commit) that renders custom dynamically-generated drop-down menus (eg. recent posts, yearly archives).
The plugin is somewhat based on https://www.ibenic.com/understanding-wordpress-menu-items/ but is a bit more involved. It's a stable and simple plugin.

The way the plugin works is it adds a single menu-item meta-box in the Menu editor page. However on the front it dynamically generates submenu items that don't show on the Edit Menu page. On the front the plugin creates virtual/fake WP_Post objects at runtime that are fed into Walker_Nav_Menu_Checklist. Therefore these objects have no corresponding $menu_post in the database.

Since WordPress 5.4, core throws multiple Attempt to read property "post_status|ID" on null notices on every page load. These notices have been upgraded to warnings in PHP 8.

wp-admin/includes/template.php:2288
get_post_states()
wp-includes/nav-menu.php:839
wp_setup_nav_menu_item()
wp-includes/nav-menu.php:839
array_map()
wp-includes/nav-menu.php:727
wp_get_nav_menu_items()
wp-admin/nav-menus.php:596

Attachments (1)

changeset-58932.diff (671 bytes) - added by apedog 12 months ago.

Download all attachments as: .zip

Change History (6)

#1 @apedog
12 months ago

Perhaps a similar sanity check could be added to get_post_states() if parameter $post is null.

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


7 weeks ago

#3 @joemcgill
7 weeks ago

  • Owner set to joemcgill
  • Status changed from new to reviewing

#5 @joemcgill
7 weeks ago

  • Keywords reporter-feedback added

Thanks @apedog. This change makes sense to me since get_post() can return null on failure. I've refreshed the diff in this PR so we can make sure tests are passing. It would be great to add an additional unit test that would demonstrate the bug.

Note: See TracTickets for help on using tickets.