Make WordPress Core

Opened 10 years ago

Last modified 6 years ago

#28140 new defect (bug)

Position for first menu item on menu is 0 when it should be 1

Reported by: danielbachhuber's profile danielbachhuber Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

When the first menu item is added to a menu and no position is explicitly set, its position is set as 0 instead of 1.

Offending line: https://core.trac.wordpress.org/browser/tags/3.9/src/wp-includes/nav-menu.php#L344

Attachments (3)

28140.diff (1.4 KB) - added by rohan013 10 years ago.
Change the default value menu-item-position to 1
28140.2.diff (1.4 KB) - added by rohan013 10 years ago.
28140.3.patch (3.5 KB) - added by birgire 6 years ago.

Download all attachments as: .zip

Change History (10)

@rohan013
10 years ago

Change the default value menu-item-position to 1

#2 @rohan013
10 years ago

  • Keywords has-patch added; needs-patch removed

#3 @SergeyBiryukov
10 years ago

  • Keywords reporter-feedback added

When the first menu item is added to a menu and no position is explicitly set, its position is set as 0 instead of 1.

Could you describe the issue with more details, and why it should be 1?

#4 @danielbachhuber
10 years ago

  • Keywords reporter-feedback removed

Because 0 is an invalid menu position and it breaks things?

The historical context is in that pull request. I don't recall the exact specifics off the top of my head, but I vaguely remember that the positioning for a menu would be "0,2,3" without the hack (which broke other things).

@rohan013
10 years ago

#5 @rohan013
10 years ago

Fixed some errors

#6 @chriscct7
8 years ago

Patch is good, still needs unit tests

@birgire
6 years ago

#7 @birgire
6 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

The position of menu items is stored as 0,2,3, ... in the menu_order field of the wp_posts table, as @danielbachhuber mentioned.

We note that it's not possible to test this behavior directly with wp_get_nav_menu_items(), because the position (menu_order) is adjusted from 0,2,3,... to 1,2,3,... with this part of that function:

$i = 1;
foreach ( $items as $k => $item ) {
    $items[$k]->{$args['output_key']} = $i++;
}

where $args['output_key'] is menu_order.

See here:

https://core.trac.wordpress.org/browser/tags/4.8.2/src/wp-includes/nav-menu.php#L657-L660

In 28140.3.patch we

  • Adjust the patch in 28140.2.diff with 1 == (int) $args['menu-item-position'] instead of 0 == (int) $args['menu-item-position']
  • Add a test to check if the position (menu_order) of two newly added menu items is 1 and then 2.
  • Add a test to check if the position (menu_order) from wp_get_nav_menu_items() is 1 and 2.
Note: See TracTickets for help on using tickets.