WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 2 months ago

#15533 closed defect (bug) (fixed)

wp_get_nav_menu_items order doesn't work

Reported by: markoheijnen Owned by: DrewAPicture
Milestone: 4.9 Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords: needs-docs
Focuses: Cc:

Description (last modified by scribu)

Argument order doesn't work when output is ARRAY_A since on the end of function wp_get_nav_menu_items there is a usort on the $items array what will be returned.

Since out ARRAY_A is the default output this can give strange behaviour.

The code that gives the problem:

	if ( ARRAY_A == $args['output'] ) {
		$GLOBALS['_menu_item_sort_prop'] = $args['output_key'];
		usort($items, '_sort_nav_menu_items');
		$i = 1;
		foreach( $items as $k => $item ) {
			$items[$k]->$args['output_key'] = $i++;
		}
	}

Change History (15)

#1 @scribu
7 years ago

  • Description modified (diff)

#2 @nacin
7 years ago

  • Version changed from 3.1 to 3.0

#3 @c3mdigital
4 years ago

  • Keywords close 2nd-opinion needs-codex added
  • Resolution set to invalid
  • Status changed from new to closed

The 'orderby' argument is ignored when output is set to ARRAY_A but you can specify orderby in the 'output_key' argument then order works as expected. Is there a reason this function uses the output_key argument instead of orderby?

I think we just need to update the codex.

#4 @nacin
4 years ago

Seems like there is something legitimate here, though unsure what. Please re-open or open a new ticket for reconsideration.

#5 @markoheijnen
4 years ago

I'm not sure it this should be closed. Can't we just use the value of orderby as the value for output_key when output_key isn't provided?

#6 @SergeyBiryukov
4 years ago

  • Keywords close removed
  • Resolution invalid deleted
  • Status changed from closed to reopened

#7 @DrewAPicture
4 years ago

  • Keywords needs-codex removed

Feel free to re-tag for docs changes if there ends up being no no consensus for leaving this ticket open.

#8 @chriscct7
23 months ago

  • Keywords needs-docs added; 2nd-opinion removed
  • Owner set to DrewAPicture
  • Severity changed from minor to normal
  • Status changed from reopened to assigned

Reassigning to drew per last comment

#9 @DrewAPicture
23 months ago

  • Status changed from assigned to accepted

#10 @DrewAPicture
12 months ago

  • Keywords good-first-bug added
  • Owner DrewAPicture deleted
  • Status changed from accepted to assigned

@c3mdigital I'll bet the reason output_key exists is to explicitly make it possible to differentiate the order of posts queried via get_posts() vs displayed via wp_get_nav_menu_items().

I think all we really need here is some clarifying documentation for the purpose/intent output_key argument.

This ticket was mentioned in Slack in #docs by morganestes. View the logs.


11 months ago

#12 @welcher
4 months ago

  • Keywords needs-patch added

#13 @DrewAPicture
2 months ago

So, some interesting history here.

The sorting logic was originally introduced in [13344], and in that case, the output order was manually overwritten with the value of $item in the foreach.

When the menus UI came in in [14248], we switched to using an incrementer on the output_key field for each post in the list. At that time, we started passing order => 'ASC' in the get_posts() args, and the _sort_nav_menu_items() callback was introduced along with the $_menu_item_sort_prop global – needed for use in the sort callback.

The use of the incrementer is puzzling in that it only makes sense in the context that output_key is menu_order. For instance, in wp_nav_menu_update_menu_items(), the output_key argument is passed as ID. This makes sense in terms of wanting the unordered posts, but less less sense when you consider that the post IDs are then being incremented in the foreach.

In [38928] when wp_list_sort() was introduced, we started passing a hard-coded order value of 'ASC', completely disregarding the $args['order'] value. Now, obviously, this ticket was created 7 years ago, which makes anything past [14557] kind of irrelevant, but interesting nonetheless.

All that said, I think the best thing to do here is explain in the parameter notation for $args that 'order' will be ignored when 'output' is ARRAY_A.

#14 @DrewAPicture
2 months ago

  • Keywords good-first-bug needs-patch removed
  • Milestone changed from Awaiting Review to 4.9

#15 @DrewAPicture
2 months ago

  • Owner set to DrewAPicture
  • Resolution set to fixed
  • Status changed from assigned to closed

In 40942:

Docs: Add full documentation for arguments accepted by wp_get_nav_menu_items().

Also provide differentiation between arguments as passed to get_posts() vs used for ordering outputted menu items.

By and large, arguments in the $args array are intended to directly affect how nav_menu_item posts are retrieved. When the default ARRAY_A is used for 'output', the 'order' and 'orderby' arguments are essentially ignored, instead giving preference to a hard-coded order of 'ASC' and an orderby value derived from the non-get_posts() argument 'output_key'.

Fixes #15533.

Note: See TracTickets for help on using tickets.