WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 2 years ago

#21534 closed defect (bug) (fixed)

Walker: has_children is only set if $args[0] is an array

Reported by: betzster Owned by: betzster
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords: has-patch dev-feedback
Focuses: Cc:

Description

You can't depend on has_children because it doesn't get set if $args[0] is an object. This is obviously an easy fix, just check if it's an object and set it in that case.

Related: #15214

Attachments (3)

has_children.patch (623 bytes) - added by betzster 5 years ago.
Check if $args[0] is an object, and also set has_children in that case.
21534.diff (839 bytes) - added by DrewAPicture 4 years ago.
21534.2.diff (552 bytes) - added by nacin 4 years ago.

Download all attachments as: .zip

Change History (15)

@betzster
5 years ago

Check if $args[0] is an object, and also set has_children in that case.

#1 @chriskrycho
5 years ago

Any particular reason this hasn't gotten pushed in, yet? The patch works correctly; I came up with the same solution independently and popped over to report it myself.

#2 follow-up: @SergeyBiryukov
4 years ago

  • Keywords needs-refresh added

As far as I can see, $args is always an array in core. A piece of code to reproduce the issue might be helpful.

The patch needs a refresh after [23346].

@DrewAPicture
4 years ago

#3 in reply to: ↑ 2 ; follow-up: @DrewAPicture
4 years ago

  • Keywords needs-refresh removed

Replying to SergeyBiryukov:

As far as I can see, $args is always an array in core. A piece of code to reproduce the issue might be helpful.

$args itself is an array, but seems like the first index might not always be.

21534.diff covers the refresh of sorts.

#4 in reply to: ↑ 3 @SergeyBiryukov
4 years ago

  • Keywords commit 3.7-early added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 3.4.1 to 3.0

Found an explanation in #15214.

In wp_nav_menu(), $args is an object:
http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/nav-menu-template.php#L145
Via walk_nav_menu_tree(), it ends up being $args[0] in Walker::display_element().

#5 @wonderboymusic
4 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#6 follow-up: @nacin
4 years ago

I don't have an objection to this, but just to confirm, does it make more sense to do 21534.diff than inside the nav menu API?

#7 in reply to: ↑ 6 @westi
4 years ago

Replying to nacin:

I don't have an objection to this, but just to confirm, does it make more sense to do 21534.diff than inside the nav menu API?

I wonder if we should switch the nav menu API to not use objects, or if that will break plugins...

Seems strange that it does use objects instead of arrays.

@nacin
4 years ago

#8 @nacin
4 years ago

Would 21534.2.diff fix this? It appears we used an object for some kind of convenience or whatever, but clearly walk_nav_menu_tree() should have received it in array form for it to be passed to Walker::walk().

#9 @nacin
4 years ago

  • Milestone changed from 3.7 to Future Release

This is not a regression, and seems to me like either 21534.2.diff would make sense as a specific fix, or casting $args[0] from an object to an array in Walker::display_element() would make sense as a general fix.

#10 @chriscct7
2 years ago

  • Keywords dev-feedback added; 3.7-early removed

#11 @wonderboymusic
2 years ago

  • Keywords commit removed

#12 @wonderboymusic
2 years ago

  • Milestone changed from Future Release to 4.0
  • Resolution set to fixed
  • Status changed from new to closed

[28824] with unit tests

Note: See TracTickets for help on using tickets.