Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#45733 closed defect (bug) (invalid)

Identical nav menu labels prevent Customizer from loading

Reported by: brianburton's profile brianburton Owned by:
Milestone: 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 (5)

#1 @joyously
6 years 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
6 years 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
6 years 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
6 years 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?

#5 @dlh
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Closing for lack of feedback and no conclusive evidence of a bug in core, per comment:1 and comment:4.

Note: See TracTickets for help on using tickets.