Improve handling of pages with tl_page.requireItem

[SeverinGloeckle]

Q	A
Fixed issues	Fixes #3450 , #3525

With the new routing, trying to generate a URL for a page with tl_page.requireItem=1 throws a RouteParametersException. This pull request prevents or improves the handling of this exception in breadcrumbs and navigation modules as well as in the frontend preview.

Improved cases:

1. Breadcrumbs, current item has requireItem
Generate the URL from the current request instead of re-generating it from the taget page.

Note: The URL is generated via strtok(Environment::get('request'), '?') so any query string is not added to the breadcrumbs URL.

2. Breadcrumbs, item in parent trail has requireItem
Skip pages with tl_page.requireItem=1 in the parent trail of the breadcrumbs, even if showHidden is enabled.

Note: These pages are skipped explicitly in the breadcrumbs (and not handled by the future proof fix below) to prevent spamming the log with error messages for failed URL generations.

3. Future proof breadcrumbs
Only add items with a non-empty URL ($href) to the breadcrumbs (except root page).

This way, if some page types will not generate a proper URL (after an exception or for pages with no public route in the future), they will neither get added to the breadcrumbs nor to the JSON-LD data.

4. Frontend preview
If a RouteParametersException is thrown for a page when converting the frontend preview URL and the reason for the exception is an InvalidParameterException on a page with tl_page.requireItem, catch the exception and do not set a URL for the preview, thus starting the preview on the root page.

[SeverinGloeckle]

There is still one case unhandled: If a page with tl_page.requireItem=1 is visible in any navigation, the RouteParametersException is thrown, but handled within Module::renderNavigation resulting in the page not being added to the navigation and an error message written to the system log.

In my opinion, there is no need to explicitly skip pages with tl_page.requireItem=1 in Module::renderNavigation since the current handling seems valid. To get rid of the error messages, an editor should flag these pages with tl_page.hide.

However, maybe the option / checkbox tl_page.hide should be automatically activated (and assigned with the disabled attribute) when a page is saved with tl_page.requireItem=1. What do you think?

[fritzmg]

Related: #3605

[SeverinGloeckle]

Thanks for the update @fritzmg!

This pull request fixes the handling of pages with tl_page.requireItem. These pages are routable, but only with a proper parameter which is not available in the frontend preview and in the breadcrumbs module (hence this fix).

Your pull request will improve the handling of unroutable pages like error pages, which could also end up in a breadcrumbs trail.

Can we get this fix merged for Contao 4.12 so the adjustments from @fritzmg may be used to also improve the breadcrumbs for Contao 4.13?

[leofeyer]

@SeverinGloeckle @aschempp How do we proceed here?

[aschempp]

I have rebased the PR and I think this can finally be merged. Two things are changed:

1. the breadcrumb menu no longer shows pages if no URL can be generated
2. pages with requireItem no longer have a preview link. The icon in the page list is no longer clickable. If the page URL (...preview?page=17) is called, it now redirects to the home page (fallback for any "unknown" preview link) instead of throwing an exception.

[leofeyer]

Thank you @SeverinGloeckle.