PHP Notice: Trying to get property of non-object in wp-admin\nav-menus.php on line 836

[skylarkcob]

I'm using WordPress 4.5.2 and get this PHP Notice:

PHP Notice:  Trying to get property of non-object in wp-admin\nav-menus.php on line 836

When I export and import database for using on localhost (one database content for two sites), two sites using different theme and have different menu location.

My first site has menu location Mobile, when I use this database on new site with Twenty Sixteen theme, it doesn't have this location, so the notice message appeared.

The menu location on Menu Settings section looks like this:

Mobile (Currently set to: )

Plese add a conditional function to check if is a nav menu object before get its name.

wp_get_nav_menu_object( $menu_locations[ $location ] )->name

It's not an error, but I don't want to see this notice message when coding theme. Thank you.

[Frozzare]

Added a condition checking if the menu can be used before it's used. This condition exists in other places in the core when wp_get_nav_menu_object is used.

[sabernhardt]

uses is_nav_menu() to check for false, an error or if the taxonomy is not 'nav_menu'

[sabernhardt]

Related: #63484

At first, checking for false seemed to be enough because the docblock for wp_get_nav_menu_object() said it returns either WP_Term or false.
<?php if ( ! empty( $menu_locations[ $location ] ) && $menu_locations[ $location ] !== $nav_menu_selected_id && wp_get_nav_menu_object( $menu_locations[ $location ] ) ) : ?>

However, is_nav_menu() also checks for an error or incorrect taxonomy, so I used that instead in the refreshed patch.
<?php if ( ! empty( $menu_locations[ $location ] ) && is_nav_menu( $menu_locations[ $location ] ) && $menu_locations[ $location ] !== $nav_menu_selected_id ) : ?>

[apermo]

#63484 was marked as a duplicate.

[apermo]

I've reviewed @sabernhardt s Patch and it will fix #63484 and this since the additional call of is_nav_menu( $menu_locations[ $location ] ) will prevent wp_get_nav_menu_object() from returning false.

One minor improvement on it, I've changed the order, so that is_nav_menu( $menu_locations[ $location ] ) as the most expensive function will be called last, it likely won't make any measurable difference since it's backend code that's not regularly called, but it's good practice to sort conditions from cheap to expensive. And there are performance tickets just doing this.

[apermo]

Improved order in if from cheapest to most expensive for good measure.

[SirLouen]

Combined Bug Reproduction and Patch Testing Report

Description

🟠 This report validates that the indicated patch works but with some caveats

Patch tested: https://core.trac.wordpress.org/attachment/ticket/37026/37026.3.patch

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: Windows 10/11
• Theme: Twenty Twenty-One 2.5
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Testing Instructions

I cannot really reproduce this through regular, only reproducible under certain forced conditions where regular menus are not being used (as the scenario commented by the OP). The only solution comes to adding a code to a plugin, to forcefully set an unexistent menu to an existing location.

1. Add the following code to a plugin:

   $locations = get_nav_menu_locations();
   $locations['primary'] = 12345; 
   set_theme_mod('nav_menu_locations', $locations);
   

2. 🐞 Error condition reproduced

Actual Results

1. 🟠 Issue resolved with patch with concerns

Additional Notes

• I have one main concern. With the patch, the fact is that the location is still assigned to an inexistent menu and nothing is informing of such. I feel that this patch is just hiding the error, but not dealing with it. In fact, the error is posing something that is intrinsically wrong. You can deal with the error by simply going into Menu > Manage Locations > And assigning the right Menu to the conflicting location.

• My suggestion would be to add a notice explaining what is going on with this error. Something like

"Your $menu_locations[ $location ] location has a wrong menu assigned. Please, assign a valid menu"

• I'm assuming that my error is a little different because it has been 9 years after the report was done.

Supplemental Artifacts

Error condition

[welcher]

This was reviewed in the 6.9 Bug Scrub today. The patch need a refresh and we're 1 day away from Beta 2 so I will move it the 7.0 milestone.

[audrasjb]

As per today's bug scrub, I'm leaving here some notes from the scrub:
​https://wordpress.slack.com/archives/C02RQBWTW/p1770137531074749
"Instead of an error message, I might make the text something like "(Currently set to: an invalid menu ID)". The theme-location-set span should also have an id attribute for aria-describedby."

• Creates a text string to display for invalid menu ID.
• Assigns id and aria-describedby to the connect the span with the checkboxes.

Trac 37026

Before the patch, an invalid menu ID can result in a PHP error:

With the patch, the text says that the ID is invalid (but it is not an error message):

Note: To assign an invalid ID, I needed to edit the theme_mods_twentytwentyone value in the database, in the *_options table.

@sabernhardt Request some changes on PR.

[sabernhardt]

As of today, the pull request shows this when the ID is not a valid menu:

(Currently set to: an unknown menu)

@audrasjb Are you intending to commit?

@westonruter yep

[audrasjb]

In 61658:

Menus: Prevent error in Menu location checkbox settings.

This changeset avoids throwing a PHP error when an invalid ID is assigned to a menu location, creates a text string to display when using an invalid menu ID, and assigns id and aria-describedby attributes to connect the span element with the related checkbox.

Props skylarkcob, Frozzare, sabernhardt, apermo, SirLouen, audrasjb, mukesh27, westonruter.
Fixes #37026.

fixed

committed in r61658