Infinite loop bug in 2.7.5

Hi Andrija,

Thanks for reporting this, it has also just been reported here:

https://www.ads-software.com/support/topic/latest-update-causing-issues-site-wont-load/

I’m not able to replicate it here unfortunately. Do you happen to see why this is happening on some sites, but not all? Are they maybe using different PHP versions? Could you confirm that changing:

while( $item->menu_item_parent != array_pop( $stack ) ) {

to

while( array_pop( $stack ) !== null && ( $item->menu_item_parent != array_pop( $stack ) ) ) {

.. fixes the issue on the sites that are affected?

Thanks!
Tom

This snippet won’t work correctly.

while( array_pop( $stack ) !== null && ( $item->menu_item_parent != array_pop( $stack ) ) ) {

e.g.

$stack = [
  1,
  2,
  3
];

$item->menu_item_parent = 1;

array_pop( $stack ) !== null // true, because array_pop( $stack ) is 1 here
$item->menu_item_parent != array_pop( $stack ) // false, because 1 !== 2

What fixes it should be:

$stack_item = array_pop( $stack );
while( $stack_item !== null && $item->menu_item_parent != $stack_item ) {
	$level--;
	$stack_item = array_pop( $stack );
}

The site loads, but the menu looks pretty broken afterwards.

• This reply was modified 4 years, 6 months ago by Andrija Vu?ini?.

This also works: https://plugins.trac.www.ads-software.com/changeset/2278372/

The thing is, menu looks broken. Comparing the logic between the two, I believe the new one can produce negative levels, which isn’t really expected? You’d need a safeguard there also.

Hi Andrija,

Thanks for your suggestions. I think I’ll need to be able to replicate this myself in order to fix this correctly, so I’ve rolled back the relevant changes and tagged 2.7.6. There are a couple of users in another thread who will hopefully install that now using WP Rollback. I’ll wait to hear back and then roll out 2.7.6.

Thanks again,
Tom

@megamenu Hi Tom,

would you mind pinging me on wp.org Slack, I can try to give you the $items arrays that are passed to apply_depth_to_menu_items.

Hi Andrija,

That would be really handy, thanks! Could you put it on pastebin.com and post the link here? I can log into Slack if you prefer, I just need to remember how to log in ??

Regards,
Tom

Here goes: https://pastebin.com/96X2qXTb

It will expire in a day, so please get it by then. Feel free to mark this issue as resolved.

Ping here if you want me to review/test the new changes.

Hi @aidvu,

That’s really helpful, thanks!

I’m having trouble working with that array (I’m getting “Fatal error: Uncaught Error: Call to undefined method WP_Post::__set_state()”, and I can’t work out how to convert it into a usable format), could you try serializing the items instead?

die(serialize($items));

Regards,
Tom

Can’ grab the data anymore.

Just replace

WP_Post::__set_state(

with

(object)

and remove the ) where the __set_state( call was.

Regards.

Hi @aidvu,

Got it, thanks.

I see the problem. The 25th item 31044 has a menu order of 25 and has a parent of 30605. The parent item (30605) should already be in the array of items somewhere, because the items are ordered by menu_order, and it should be impossible for a child menu item to have a higher menu_order than its parent.

In fact, 30605 has a menu order of 35 and therefore appears after its children within the array of items.

In short, the items are not in the correct order. A parent is appearing after its children.

I’m not sure how that has happened but it seems like there is a bug further up the chain somewhere (or maybe something is interfering, possibly a plugin that auto populates sub menu items?). MMM does reorder menu items itself, but I’m fairly certain it hasn’t done any reordering by the time this code is executed.

I think I will fall back to the old/not so clever way of finding the menu item depth, and use the same technique to work out the depth of a menu item on the backend, without relying on jQuery (that was the problem that kicked off this series of changes).

Hopefully that makes sense and thanks again for your help!

Regards,
Tom

I think you could probably have a function that calculates all the item depths in 2 passes, and put it in a static var. Then just on consecutive calls return the static (cached) values.

I’m glad you figured it out.

Regards,
Andrija