Use final URLs after redirects in menus

[lukasbestle]

Some of our templates redirect to another page (e.g. child page). If we include those pages in menus, the user needs to be redirected even though we already know the final target. This is currently mostly the case with docs/guide and license.

This PR implements two new page methods:

• menuUrl(): Returns the final URL after redirects to be used in menus. Overridden in the page model where relevant, otherwise falls back to the page URL.
• isActiveInMenu(): Variant of isActive() that respects menuUrl(). This fixes a bug in the menu for legal pages where the License page wouldn't show an active state because it immediately redirects to its last child.

[distantnative]

@lukasbestle Could you please share why we need to replace ->isOpen() with ->isActiveInMenu()? Based on the description above, I would this that isOpen is exactly made for that.

And how about if we overwrite the url() method for those instead of creating a new menuUrl() for all object? Do you see a case where those models still need their original url()?

[lukasbestle]

@distantnative On isOpen(): That's basically the opposite of what we need. We explicitly don't want to highlight the main menu item if a subpage is active. We do use isOpen for controlling whether the menu is expanded by default, but the other use we had of isOpen for the submenu was wrong IMO. Inside a submenu, we are already displaying the page itself and there are no additional children. Or have I misunderstood the structure?

About replacing the url() method: That's what I had before, but unfortunately it won't work for this use case. If the parent returns the URL of its child, the child's url() method then causes an infinite loop because it uses its parent's url() as a base for adding its own UID at the end.

[lukasbestle]

Thinking about it again, I think you are right. We don't have recursive submenus, only two levels (main and one sub level). So in the sub level, the menu item should be highlighted even if one of its children is active. So isOpen() is correct there. And I can't think of a case when that would be broken by redirects.

[distantnative]

"Changes requested" shows up so harsh and negative. I'll keep using "Comments" again going forward. Just wanted to indicate that those issues need to be solved before we can merge, but the UI makes it look so negative.