#56946 closed defect (bug) (fixed)
WP 6.1-RC6: `menu-item-has-children` class is not being applied correctly
Reported by: | webmandesign | Owned by: | peterwilsoncc |
---|---|---|---|
Milestone: | 6.1.1 | Priority: | normal |
Severity: | normal | Version: | 6.1 |
Component: | Menus | Keywords: | has-patch has-unit-tests commit fixed-major |
Focuses: | Cc: |
Description
When a theme has `wp_nav_menu()` depth
parameter set higher than 0
, WordPress 6.1-RC6 stops applying menu-item-has-children
class for sub-menu items correctly.
This only happens when menu contains certain number of menu and sub-menu items - haven't found out the exact logic here though. But, for example, the issue is triggered when a theme has depth=4
and menu has this structure:
Level 1 - Level 2 - - Level 3 - - - Level 4 - - - - Level 5 - - - - - Level 6
The issue is not present if depth parameter is not set or is set to 0
.
For testing check out my Michelle theme, for example, and use theme unit test with "All pages" menu set for "Primary" menu location.
This worked perfectly fine with previous versions of WordPress, so I think it is a WP6.1 bug.
For more info please visit https://wordpress.org/support/topic/wp-6-1-rc3-menu-item-has-children-class-is-not-being-applied-correctly/ thread where I've possibly found a bug in the code and also proposed a solution.
Attachments (3)
Change History (37)
This ticket was mentioned in PR #3547 on WordPress/wordpress-develop by @webmandesign.
2 years ago
#1
- Keywords has-patch added
#2
@
2 years ago
- Component changed from General to Menus
- Milestone changed from Awaiting Review to 6.1.1
#5
@
2 years ago
Just run into the same problem, missing menu-item-has-children
class, which caused menus to break on some of our production sites.
If it's helpful, one menu structure where this happened is not so deeply nested as the original example. Depth is set to 4.
- Level 1 - Level 2 - Level 3 - Level 3 - Level 2 - Level 3 - Level 3
#6
@
2 years ago
The patch works fine on my side and fix the issue (see before-patch and after-patch screenshot).
Tested with Twentytwenty theme and setting a custom depth of 4 for the primary menu.
#9
@
2 years ago
Thanks @webmandesign for the ticket and PR. Left one nit-pick feedback other then it PR LGTM. Can you please add unit tests?
#10
@
2 years ago
@mukesh27 I'm sorry, I don't know how to add unit tests for that code. Can somebody else take over from here?
#12
in reply to:
↑ 11
@
2 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
2 years ago
This ticket was mentioned in PR #3568 on WordPress/wordpress-develop by @LeonidasMilossis.
2 years ago
#14
- Keywords has-unit-tests added; needs-unit-tests removed
Enhances https://github.com/WordPress/wordpress-develop/pull/3547 with unit tests
Trac ticket: https://core.trac.wordpress.org/ticket/56946
#15
@
2 years ago
I've added my PR that enhances @webmandesign 's one with unit tests that verify the bugfix.
Considering it's my first contribution, apologies if I shouldn't have created a new PR.
#16
@
2 years ago
We're making use of that class extensively with our theme and are currently receiving tons of reports. Will there be a hotfix for that? Or, do you guys any idea how we can fix this temporarily? It's potentially breaking 20k+ websites for us right now.
#17
follow-up:
↓ 21
@
2 years ago
@davidvongries I also receive support issues regarding this bug. I'm providing this quick temporary workaround for my theme users. It only prevents triggering the issue, not fixing it: https://gist.github.com/webmandesign/0f56c1a9a219ab3ce251a5844d33924b Check it out, maybe it will help you too. You can even put it into a plugin.
#18
@
2 years ago
@davidvongries Also, it seems this will be fixed in WP6.1.1 which is due next week.
#19
follow-up:
↓ 20
@
2 years ago
We are still having issues with line 223:
$menu_items_with_children[ $menu_item->ID ] < $args->depth ) |
This extra check was added in v.6.1 but is this necessary?
#20
in reply to:
↑ 19
@
2 years ago
Replying to nuvoPoint:
We are still having issues with line 223:
( $args->depth <= 0 $menu_items_with_children[ $menu_item->ID ] < $args->depth ) This extra check was added in v.6.1 but is this necessary?
At least for my case when debugging $menu_items_with_children[ $menu_item->ID ] gives menu items id's to connect(expected with that name), and those would close to all of the time be bigger than $args->depth.
In my case wp_nav_menu is called with $args->depth = 0.
The extra check seems not to be necessary in my opinion also.
I did not take time to understand why this check could be needed based on the related code.
Just wanted to share.
#21
in reply to:
↑ 17
@
2 years ago
@webmandesign, Thanks for sharing! Here's what I did: https://github.com/MapSteps/Page-Builder-Framework/commit/99d0c4785d65b888aaaf0f3e90d24f668c8d8aab
It's not great as it allows people to add more depth than they should but it's better than having a broken menu altogether, at least until the official fix is released.
#23
@
2 years ago
I redid my testing with the PR from @LeonidasMilossis and everything fine.
The tests break with the faulty code, as expected, and they are green with the fixed code.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
2 years ago
#25
@
2 years ago
We are experiencing issues due to this bug. I can confirm that after manually applying the patch in this thread that it has resolved the issue.
#26
@
2 years ago
- Keywords commit added
Marking commit ready with the second linked pull request.
This includes the code changes from the first PR with the addition of some tests to ensure the class is added as expected.
#27
@
2 years ago
- Owner set to peterwilsoncc
- Resolution set to fixed
- Status changed from new to closed
In 54801:
#28
@
2 years ago
- Keywords fixed-major added
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for merging to the 6.1 branch.
Fixes incorrect
menu-item-has-children
class application by fixingforeach
loop nesting.Trac ticket: https://core.trac.wordpress.org/ticket/56946