I have tested this item ✅ successfully on 068d79d

With a good merge tool like e.g. Beyond Compare the diff looks much better than on GitHub.

you can just add ?w=1 on the end of the URL to ignore whitespace ;)

So the only thing that kinda concerns me is when Nic rewrote this code (#5499) he clearly seemed to think there was some specific use case here because he commented it all up. I mean it might well just be an oversight but is it possible to have an empty menu element tag that will be an empty XMLElement or something stupid and non-strict type checks will treat them differently...

Before my modified code is

// Just do not create the menu if $menuElement not exist		
if (!$menuElement)		
{			
    return true;		
}

Therefore there is no way for the modified removed code to run... even phpStorm says so :)

Right. If there were typesafe comparisons used, then a null could make a difference, but as it is now, I think the PR here is right.

Before my modified code is

I understand - but that line was added in that linked PR. That's the only reason I'm hesitating a bit. You're probably right. Just trying to figure out any weird edge cases in my head

I have tested this item ✅ successfully on 068d79d

RTC

@wilsonge Reviewed the code carefully. This PR is right and safe to merge. You can trust three of us :D.

fiiine. i conceed :) thanks guys!