Prevents an infinite loop when the parent menu item of a menu item is itself.#3566
Prevents an infinite loop when the parent menu item of a menu item is itself.#3566mhkuu wants to merge 1 commit intoWordPress:trunkfrom
Conversation
… itself. Co-authored-by: Hugo de Vos <hdvos93@gmail.com>
peterwilsoncc
left a comment
There was a problem hiding this comment.
Added a couple of notes inline.
| /** | ||
| * @ticket 56926 | ||
| */ | ||
| public function test_if_parent_is_self() { |
There was a problem hiding this comment.
| public function test_if_parent_is_self() { | |
| public function test_self_parented_menu_items_should_not_cause_infinite_recursion() { |
| // Do not calculate the depth when the current item has itself as parent. | ||
| while ( $menu_item_parent && (int) $menu_item_parent !== $menu_item_key ) { |
There was a problem hiding this comment.
What happens if an item lists it's own grandchild as a parent, does that cause an infinite loop too? (Same question applies for further descendants)
There was a problem hiding this comment.
if an item lists it's own grandchild as a parent, does that cause an infinite loop too
Yea, probably. Since this seems to be coming from plugins thinking it needs a "sensible maximum depth", i.e. the while() loop can be stopped after x number of times. For number of sub-menus such max number would be something like 50 perhaps (can you imagine what a menu would look like with 50 levels of sub-menus?).
There was a problem hiding this comment.
Another thing here: (int) $menu_item_parent !== $menu_item_key should probably compare int to int, i.e. typecast both sides. Looking above, there doesn't seem to be a guarantee that $menu_item_key is an int, just like $menu_item_parent.
|
Discussing this with @azaozz it was decided to go with an alternative approach to reduce the chance of further issues looping through the menu item tree. I've just committed this in https://core.trac.wordpress.org/changeset/54999 / 1a85350 so will close this pull request off. Thanks for your work on this patch, although it wasn't included it is appreciated and you received props in the commit for the ticket. |
|
Thanks @peterwilsoncc, looks like a more fail-safe approach indeed! 👍 |
Co-authored-by: Hugo de Vos hdvos93@gmail.com
Created during Yoast Contributor Day on Friday 4 November.
Prevents an infinite loop when the parent menu item of a menu item is itself. We do not know the root cause of this issue, but with this PR, we add a safeguard to make sure the menu is still being output. This PR also adds a unit test to confirm the output is as expected.
Trac ticket: https://core.trac.wordpress.org/ticket/56926
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.