WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 8 months ago

#45733 new defect (bug)

Identical nav menu labels prevent Customizer from loading

Reported by: brianburton Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: reporter-feedback
Focuses: Cc:

Description

Description of Problem

When my theme is installed it performs a setup process to create menus required by the theme. I discovered that if the labels of the nav "anchors" and the nav menus are identical, it will cause an error and prevent the customizer from loading.

Sample Code

register_nav_menus( array(
        'super'   => esc_html__( 'Super Navigation', 'textdomain' )
) );

$superId = wp_create_nav_menu( 'Super Navigation' );

set_theme_mod( 'nav_menu_locations', array(
        'super'   => $superId
) );

When I open the Customizer on the front end, the only thing visible is an empty customizer window and the preview remains blank.

https://i.imgur.com/6qr9wYq.jpg

Checking the PHP error logs I discovered this:

PHP Catchable fatal error:  Object of class WP_Error could not be converted to string in \wordpress\wp-includes\general-template.php on line 4277

Workaround

The workaround is to give the nav "anchors" and nav menus unique names.

Solution

I'm not familiar enough with WP core development to know which solution would be best, however I would recommend one of the following:

  1. Make the following change to the __checked_selected_helper() function in general-template.php (tested as working):
    function __checked_selected_helper( $helper, $current, $echo, $type ) {
            if (is_wp_error($helper))
                    return;
    
            if ( (string) $helper === (string) $current )
    
  1. Have wp_create_nav_menu() return an instance of WP_Error if it is called with an existing label (preferred).

Change History (4)

#1 @joyously
8 months ago

  1. is better than 2.

Even better is that a theme should not create menus. And any code creating something should check for duplicates before creating it.

#2 @brianburton
8 months ago

  • Summary changed from Identical nav menu names cause prevent Customizer from loading to Identical nav menu labels prevent Customizer from loading

Edit to fix the mangled title.

To be fair it's not obvious that the nav locations and nav menus can't share the same label and I can't find any mention of it in the docs. So checking for duplicates would seem superfluous not knowing what we know now, especially since the labels are hard coded and not from user input.

The reason that I prefer the second solution is because the error will be apparent immediately and not in a specific use case such as only when the Customizer is open.

#3 @joyously
8 months ago

I'm not sure why you think calling wp_create_nav_menu( 'Super Navigation' ) would work anyway. This example is using the string that was translated, not the ID that was registered.

And please don't gloss over the fact that themes should not write to any options in the database except their own. They should not have side effects in the database. They can come and go at any time. But I will assume that you will be doing this code in a plugin, not a theme.

#4 in reply to: ↑ description @dlh
8 months ago

  • Keywords reporter-feedback added
  • Version changed from 5.0.2 to 4.3

Hi @brianburton,

  1. Have wp_create_nav_menu() return an instance of WP_Error if it is called with an existing label (preferred).

wp_create_nav_menu() does return a WP_Error: https://github.com/WordPress/wordpress-develop/blob/29cb0f5ad52863494634229532d0a1bbf9993f45/src/wp-includes/nav-menu.php#L234-L247

As I understand the code sample, the WP_Error is then saved as the theme mod, which is not intended.

In addition to @joyously's comments, could the code check the return value before saving the theme mod?

Note: See TracTickets for help on using tickets.