WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 2 days ago

#45361 assigned defect (bug)

Integer menu slugs should not be supported

Reported by: desrosj Owned by: audrasjb
Milestone: 5.2 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-patch reporter-feedback
Focuses: Cc:

Description

Currently, menus can be registered with an integer slug. This can cause unintended issues that are difficult to debug.

Consider the following snippet:

register_nav_menus( array(
	'primary' => 'Primary',
	1         => 'First',
	2         => 'Second',
) );

The assumption is that the resulting list of menus would match what was passed. Instead, the result is this:

array(
    'primary' => 'Primary',
    0         => 'First',
    1         => 'Second',
)

This would cause wp_nav_menu( array( 'theme_location' => 1 ) ) to return the wrong menu.

This is happening because array_merge() is used to add new nav menu locations to the global $_wp_registered_nav_menus variable. array_merge() renumbers numeric indexes, incrementing keys starting from zero in the result array.

My proposal is to add a _doing_it_wrong() warning when registering a nav menu with a numeric index. This will at least inform developers of the potential issue, and would also encourage better practices of using a string slug for nav menus.

Casting the integer as a string could also be explored, that is likely to be complicated for backward compatibility.

Attachments (1)

45361.diff (666 bytes) - added by audrasjb 3 weeks ago.
Doing it wrong: Menu location slug should not be a int.

Download all attachments as: .zip

Change History (5)

#1 @desrosj
3 months ago

For context, I encountered this while working on #45018.

#2 @audrasjb
3 weeks ago

  • Owner set to audrasjb
  • Status changed from new to assigned

Great idea Jonathan, thanks\\
I'll try some experimentation on my side to see what is the best solution.

#3 @audrasjb
3 weeks ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 5.2

Hi,

45361.diff adds a _doing_it_wrong() warning message:

"Menu location slug (key) for menu "%s" should not be a int."

%s is the value of the $locations array, not the key, since numeric indexes can be different from what developers used.

@audrasjb
3 weeks ago

Doing it wrong: Menu location slug should not be a int.

#4 @audrasjb
2 days ago

  • Keywords reporter-feedback added
Note: See TracTickets for help on using tickets.