PageService->getRootline ignores $pageUid since TYPO3 v10 (+Solution)

dev-rke commented 4 years ago

Hi there,

thanks for this extension, it saved me a lot of time in recent years. :-) Since vhs 6 i found out, that there is a problem resolving rootlines since TYPO3 v10.

Environment

vhs v6.0.3 TYPO3 v10.4.6

Current Behaviour

When calling the PageService->getRootline() method with the $pageUid argument, the full rootline gets returned. Always. The $pageUid parameter is ignored. This affects all situations where the rootline is used, e.g. v:menu(), v:page.rootline() or the SlideViewHelperTrait.

Expected Behaviour

The rootline should be filtered to begin at the given pageUid and should not return all pages.

Issue analysis

Lets have a look at the current implementation:

if (method_exists($pageRepository, 'getRootLine')) {
    if (true === (boolean)$disableGroupAccessCheck) {
        $pageRepository->where_groupAccess = '';
    }
    $rootline = $pageRepository->getRootLine($pageUid);
} elseif (isset($GLOBALS['TSFE'])) {
    $rootline = (array)$GLOBALS['TSFE']->rootLine;
} else {
    $rootline = [];
}

The code will try to resolve rootline from PageRepository by default and filters for pageUid. When the PageRepository does not exist (since TYPO3 v10), it will resolve rootline from TSFE, but does NOT filter by the given $pageUid. Therefore a different (and wrong) behaviour is created. This issue will not rise up on TYPO3 v9, as the PageRepository->getRootline() method still exists and will be called by default.

Solution

My recommendation is to rewrite the code using the new RootlineUtility, which is available since TYPO3 v9. Futhermore i switched default behaviour to the RootlineUtility, as the logic around PageRepository should be removed as soon as TYPO3 8 will not be supported anymore by vhs:

if(class_exists(RootlineUtility::class)) {
    $rootline = (new RootlineUtility($pageUid))->get();
} elseif (method_exists($pageRepository, 'getRootLine')) {
    if (true === (boolean) $disableGroupAccessCheck) {
        $pageRepository->where_groupAccess = '';
    }
    $rootline = $pageRepository->getRootLine($pageUid);
} else {
    $rootline = [];
}

I will provide a pull request on this.

NamelessCoder commented 4 years ago

Merged - thanks!

dev-rke commented 4 years ago

Hi @NamelessCoder .

might it be a better idea, to just migrate completely to the RootlineUtility and drop the class_exists? I just found out, the RootlineUtility exists since TYPO3 v8, see: https://github.com/TYPO3/TYPO3.CMS/blob/v8.7.32/typo3/sysext/frontend/Classes/Page/PageRepository.php#L952

Furthermore i have another issue with my navigation, which can be solved when imlementing the new MenuProcessor - if it involves patching vhs, i might come up with another patch in a few days. ;-)

FluidTYPO3 / vhs