WordPress.org
Get WordPress

Make WordPress Core

Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#63484 closed defect (bug) (duplicate)

Attempt to read property "name" on bool - wp-admin/nav-menus.php :1221

Reported by: apermo's profile apermo Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords:
Focuses: Cc:

Description

nav-menus.php lines 1215-1225

<?php if ( ! empty( $menu_locations[ $location ] ) && $menu_locations[ $location ] !== $nav_menu_selected_id ) : ?>
        <span class="theme-location-set">
        <?php
                printf(
                        /* translators: %s: Menu name. */
                        _x( '(Currently set to: %s)', 'menu location' ),
                        wp_get_nav_menu_object( $menu_locations[ $location ] )->name
                );
        ?>
        </span>
<?php endif; ?>

PHPDoc of wp_get_nav_menu_object()

<?php
/**
 * Returns a navigation menu object.
 *
 * @since 3.0.0
 *
 * @param int|string|WP_Term $menu Menu ID, slug, name, or object.
 * @return WP_Term|false Menu object on success, false if $menu param isn't supplied or term does not exist.
 */
function wp_get_nav_menu_object( $menu )

I have not yet digged deeper into this issue. But this occasionally happened to my editors when they tried to update our Spanish menu.

Change History (8)

This ticket was mentioned in ​Slack in #core-test by oglekler. ​View the logs.
8 months ago

#2 @avinashbhosale
8 months ago

  • Keywords reporter-feedback added

@apermo - how is possible that your editors are being able to add an invalid menu item to your spanish menu? Can you share an screencast? Could it be a plugin conflict? Could you provide some more info on this? thanks

Last edited 8 months ago by avinashbhosale (previous) (diff)

#4 follow-up: @apermo
8 months ago

@avinashbhosale I have no method how to reproduce it. And since it is a logical flaw, I have not tried to.

The issue is that the restult of wp_get_nav_menu_object() is used without confirming that it did not return false.

Adding is_nav_menu() to the condition like @sabernhardt suggested in #37026 should fix it.

@sabernhardt: Should we mark this one as duplicate?

#5 @sabernhardt
8 months ago

  • Keywords reporter-feedback removed

The ticket could be closed as a duplicate if this only needs to remove the PHP notice, though I do not think the cause is exactly the same.

If you want to look into it further, your 'nav_menu_locations' theme_mod might refer to a menu that does not exist (anymore). You could:

  1. Temporarily add print_r($locations); print_r($menu_locations); before the foreach loop on line 1202, inside the menu-theme-locations fieldset. (var_dump() should work too, but I think print_r() looks cleaner.)
  2. Edit the Spanish menu, and compare the arrays that appear in the 'Display location' section. You likely will have the same key (such as 'primary') in both arrays.
  3. Check each of your menus for the integer(s) assigned in the $menu_locations array. You could find the menu IDs in the select-menu-to-edit dropdown, or you could select each menu and find the menu= ID in the URL.
  4. Remove the temporary code.

If $menu_locations includes a menu that has been removed, that could be a separate issue because removing the menu hopefully would update the setting.

#6 @apermo
8 months ago

  • Resolution set to duplicate
  • Status changed from new to closed

@sabernhardt I'm happy to close this in favor of #37026. In my case it was likely some hiccup. And no further debugging will be needed.

#7 @sabernhardt
8 months ago

  • Milestone Awaiting Review deleted

OK. Thanks!

#8 in reply to: ↑ 4 @SirLouen
8 months ago

Replying to apermo:

@avinashbhosale I have no method how to reproduce it. And since it is a logical flaw, I have not tried to.

Just FYI, the massive amount of abandoned tickets (with patches) due to a lack of reproduction reports. My theory is that, barely anyone, would be interested in pushing on this kind of ticket, when on the other side there are plenty of reports with a lot more additional devices like reproduction reports, patches, unit tests, etc… waiting for review.

  1. Many of them, are simply not reproducible (despite being a logical flaw)
  2. Others are reproducible, but by wrong conditions implemented in some plugins.

And yes, I agree that sorting this logic issue will lead to better code, as sometimes refactoring or preemptively sorting future deprecation notices are good practices. But for some reason, most people, as I say, prioritize full reports than half-baked reports that end in the graveyard of forgotten tickets.

My advice: whenever you can, build a minimal reproduction model. I built it as an example for this report the other day in the #core-test triage session (I'm posting in #37026 to showcase). But again, this is 100% optional, and is just meant to help to push things forward.

Note: See TracTickets for help on using tickets.