Commit fc6cf2c9 authored by Benni Mack's avatar Benni Mack
Browse files

[BUGFIX] Mark shortcut pages as active in HMENU

This change moves the method resolveShortcutPage()
into PageRepository as it is now also used in determining
if a page is marked as active / current.

The method `resolveShortcutPage()` is much more convenient
when working with $page records instead of the properties,
instead of `getPageShortcut()`.

This also allows MenuProcessor to have a proper "active"
and "current" values set properly without any workarounds.

Resolves: #85138
Resolves: #80841
Resolves: #87923
Releases: main, 11.5
Change-Id: Ic401fb42696757199c5974120f24250c467d9b75
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/72860


Tested-by: Stefan Bürk's avatarStefan Bürk <stefan@buerk.tech>
Tested-by: core-ci's avatarcore-ci <typo3@b13.com>
Tested-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Tested-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
Reviewed-by: Stefan Bürk's avatarStefan Bürk <stefan@buerk.tech>
Reviewed-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Reviewed-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
parent af512dbd
......@@ -97,6 +97,7 @@ class PageRepository implements LoggerAwareInterface
'_MP_PARAM',
'_ORIG_uid',
'_ORIG_pid',
'_SHORTCUT_ORIGINAL_PAGE_UID',
'_PAGES_OVERLAY',
'_PAGES_OVERLAY_UID',
'_PAGES_OVERLAY_LANGUAGE',
......@@ -990,7 +991,16 @@ class PageRepository implements LoggerAwareInterface
self::DOKTYPE_RECYCLER,
self::DOKTYPE_BE_USER_SECTION,
];
$savedWhereGroupAccess = '';
// "getMenu()" does not allow to hand over $disableGroupCheck, for this reason it is manually disabled and re-enabled afterwards.
if ($disableGroupCheck) {
$savedWhereGroupAccess = $this->where_groupAccess;
$this->where_groupAccess = '';
}
$pageArray = $this->getMenu($idArray[0] ?: $thisUid, '*', 'sorting', 'AND pages.doktype NOT IN (' . implode(', ', $excludedDoktypes) . ')');
if ($disableGroupCheck) {
$this->where_groupAccess = $savedWhereGroupAccess;
}
$pO = 0;
if ($shortcutMode == self::SHORTCUT_MODE_RANDOM_SUBPAGE && !empty($pageArray)) {
$pO = (int)random_int(0, count($pageArray) - 1);
......@@ -1039,6 +1049,52 @@ class PageRepository implements LoggerAwareInterface
return $page;
}
/**
* Check if page is a shortcut, then resolve the target page directly.
* This is a better method than "getPageShortcut()" and should be used instead, as this automatically checks for $page records
* and returns the shortcut pages directly.
*
* This method also provides a runtime cache around resolving the shortcut resolving, in order to speed up link generation
* to the same shortcut page.
*
* @see \TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController::getPageAndRootline()
*/
public function resolveShortcutPage(array $page, bool $resolveRandomSubpages = false, bool $disableGroupAccessCheck = false): array
{
if ((int)($page['doktype'] ?? 0) !== self::DOKTYPE_SHORTCUT) {
return $page;
}
$shortcutMode = (int)($page['shortcut_mode'] ?? self::SHORTCUT_MODE_NONE);
$shortcutTarget = (string)($page['shortcut'] ?? '');
$cacheIdentifier = 'shortcuts_resolved_' . ($disableGroupAccessCheck ? '1' : '0') . '_' . $page['uid'] . '_' . $this->sys_language_uid . '_' . $page['sys_language_uid'];
// Only use the runtime cache if we do not support the random subpages functionality
if ($resolveRandomSubpages === false) {
$cachedResult = $this->getRuntimeCache()->get($cacheIdentifier);
if (is_array($cachedResult)) {
return $cachedResult;
}
}
$shortcut = $this->getPageShortcut(
$shortcutTarget,
$shortcutMode,
$page['uid'],
20,
[],
$disableGroupAccessCheck,
$resolveRandomSubpages
);
if (!empty($shortcut)) {
$page = $shortcut;
$page['_SHORTCUT_ORIGINAL_PAGE_UID'] = $page['uid'];
}
if ($resolveRandomSubpages === false) {
$this->getRuntimeCache()->set($cacheIdentifier, $page);
}
return $page;
}
/**
* Returns the redirect URL for the input page row IF the doktype is set to 3.
*
......
......@@ -1233,7 +1233,7 @@ abstract class AbstractMenuContentObject
}
// Creating link:
$addParams = ($this->mconf['addParams'] ?? '') . $MP_params;
if (($this->mconf['collapse'] ?? false) && $this->isActive($this->menuArr[$key]['uid'], $this->getMPvar($key))) {
if (($this->mconf['collapse'] ?? false) && $this->isActive($this->menuArr[$key] ?? [], $this->getMPvar($key))) {
$thePage = $this->sys_page->getPage($this->menuArr[$key]['pid']);
$LD = $this->menuTypoLink($thePage, $mainTarget, $addParams, $typeOverride, $overrideId);
} else {
......@@ -1376,36 +1376,69 @@ abstract class AbstractMenuContentObject
}
/**
* Returns TRUE if the page with UID $uid is active (in the current rootline)
* Returns TRUE if the given page is active (in the current rootline)
*
* @param int $uid Page uid to evaluate.
* @param array $page Page record to evaluate.
* @param string $MPvar MPvar for the current position of item.
* @return bool TRUE if page with $uid is active
* @return bool TRUE if $page is active
*/
protected function isActive($uid, $MPvar)
protected function isActive(array $page, $MPvar)
{
// Check for always active PIDs:
if (in_array((int)$uid, $this->alwaysActivePIDlist, true)) {
// Check for always active PIDs
$uid = (int)($page['uid'] ?? 0);
if (in_array($uid, $this->alwaysActivePIDlist, true)) {
return true;
}
$testUid = $uid . ($MPvar ? ':' . $MPvar : '');
if ($uid && in_array('ITEM:' . $testUid, $this->rL_uidRegister, true)) {
return true;
}
try {
$page = $this->sys_page->resolveShortcutPage($page);
$shortcutPage = (int)($page['_SHORTCUT_ORIGINAL_PAGE_UID'] ?? 0);
if ($shortcutPage) {
if (in_array($shortcutPage, $this->alwaysActivePIDlist, true)) {
return true;
}
$testUid = $shortcutPage . ($MPvar ? ':' . $MPvar : '');
if (in_array('ITEM:' . $testUid, $this->rL_uidRegister, true)) {
return true;
}
}
} catch (\Exception $e) {
// Shortcut could not be resolved
return false;
}
return false;
}
/**
* Returns TRUE if the page with UID $uid is the CURRENT page (equals $this->getTypoScriptFrontendController()->id)
* Returns TRUE if the page is the CURRENT page (equals $this->getTypoScriptFrontendController()->id)
*
* @param int $uid Page uid to evaluate.
* @param array $page Page record to evaluate.
* @param string $MPvar MPvar for the current position of item.
* @return bool TRUE if page $uid = $this->getTypoScriptFrontendController()->id
* @return bool TRUE if resolved page ID = $this->getTypoScriptFrontendController()->id
*/
protected function isCurrent($uid, $MPvar)
protected function isCurrent(array $page, $MPvar)
{
$testUid = $uid . ($MPvar ? ':' . $MPvar : '');
return $uid && end($this->rL_uidRegister) === 'ITEM:' . $testUid;
$testUid = ($page['uid'] ?? 0) . ($MPvar ? ':' . $MPvar : '');
if (($page['uid'] ?? 0) && end($this->rL_uidRegister) === 'ITEM:' . $testUid) {
return true;
}
try {
$page = $this->sys_page->resolveShortcutPage($page);
$shortcutPage = (int)($page['_SHORTCUT_ORIGINAL_PAGE_UID'] ?? 0);
if ($shortcutPage) {
$testUid = $shortcutPage . ($MPvar ? ':' . $MPvar : '');
if (end($this->rL_uidRegister) === 'ITEM:' . $testUid) {
return true;
}
}
} catch (\Exception $e) {
// Shortcut could not be resolved
return false;
}
return false;
}
/**
......@@ -1492,16 +1525,16 @@ abstract class AbstractMenuContentObject
$natVal = $this->isSubMenu($this->menuArr[$key]['uid'] ?? 0);
break;
case 'ACT':
$natVal = $this->isActive(($this->menuArr[$key]['uid'] ?? 0), $this->getMPvar($key));
$natVal = $this->isActive(($this->menuArr[$key] ?? []), $this->getMPvar($key));
break;
case 'ACTIFSUB':
$natVal = $this->isActive(($this->menuArr[$key]['uid'] ?? 0), $this->getMPvar($key)) && $this->isSubMenu($this->menuArr[$key]['uid']);
$natVal = $this->isActive(($this->menuArr[$key] ?? []), $this->getMPvar($key)) && $this->isSubMenu($this->menuArr[$key]['uid']);
break;
case 'CUR':
$natVal = $this->isCurrent(($this->menuArr[$key]['uid'] ?? 0), $this->getMPvar($key));
$natVal = $this->isCurrent(($this->menuArr[$key] ?? []), $this->getMPvar($key));
break;
case 'CURIFSUB':
$natVal = $this->isCurrent(($this->menuArr[$key]['uid'] ?? 0), $this->getMPvar($key)) && $this->isSubMenu($this->menuArr[$key]['uid']);
$natVal = $this->isCurrent(($this->menuArr[$key] ?? []), $this->getMPvar($key)) && $this->isSubMenu($this->menuArr[$key]['uid']);
break;
case 'USR':
$natVal = (bool)$this->menuArr[$key]['fe_group'];
......
......@@ -1100,7 +1100,7 @@ class TypoScriptFrontendController implements LoggerAwareInterface
// whether a translation of the page overwrites the shortcut
// target and we need to follow the new target
$this->originalShortcutPage = $this->page;
$this->page = $this->sys_page->getPageShortcut($this->page['shortcut'], $this->page['shortcut_mode'], $this->page['uid']);
$this->page = $this->sys_page->resolveShortcutPage($this->page, true);
$this->id = $this->page['uid'];
}
// If the page is a mountpoint which should be overlaid with the contents of the mounted page,
......@@ -2005,7 +2005,7 @@ class TypoScriptFrontendController implements LoggerAwareInterface
if (!empty($originalShortcutPageOverlay['shortcut']) && $originalShortcutPageOverlay['shortcut'] != $this->id) {
// the translation of the original shortcut page has a different shortcut target!
// set the correct page and id
$shortcut = $this->sys_page->getPageShortcut($originalShortcutPageOverlay['shortcut'], $originalShortcutPageOverlay['shortcut_mode'], $originalShortcutPageOverlay['uid']);
$shortcut = $this->sys_page->resolveShortcutPage($originalShortcutPageOverlay, true);
$this->id = ($this->contentPid = $shortcut['uid']);
$this->page = $this->sys_page->getPage($this->id);
// Fix various effects on things like menus f.e.
......
......@@ -120,8 +120,8 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
// Mount pages are always local and never link to another domain,
$addMountPointParameters = !empty($MPvarAcc);
// Add "&MP" var, only if the original page was NOT a shortcut to another domain
if ($addMountPointParameters && !empty($page['_SHORTCUT_PAGE_UID'])) {
$siteOfTargetPage = GeneralUtility::makeInstance(SiteFinder::class)->getSiteByPageId((int)$page['_SHORTCUT_PAGE_UID']);
if ($addMountPointParameters && !empty($page['_SHORTCUT_ORIGINAL_PAGE_UID'])) {
$siteOfTargetPage = GeneralUtility::makeInstance(SiteFinder::class)->getSiteByPageId((int)$page['_SHORTCUT_ORIGINAL_PAGE_UID']);
$currentSite = $this->getCurrentSite();
if ($siteOfTargetPage !== $currentSite) {
$addMountPointParameters = false;
......@@ -334,35 +334,11 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
*/
protected function resolveShortcutPage(array $page, PageRepository $pageRepository, bool $disableGroupAccessCheck): array
{
if ((int)($page['doktype'] ?? 0) !== PageRepository::DOKTYPE_SHORTCUT) {
return $page;
}
$shortcutMode = (int)($page['shortcut_mode'] ?? PageRepository::SHORTCUT_MODE_NONE);
$savedWhereGroupAccess = '';
try {
if ($disableGroupAccessCheck) {
$savedWhereGroupAccess = $pageRepository->where_groupAccess;
$pageRepository->where_groupAccess = '';
}
$shortcut = $pageRepository->getPageShortcut(
$page['shortcut'] ?? '',
$shortcutMode,
$page['uid'],
20,
[],
$disableGroupAccessCheck,
false
);
if (!empty($shortcut)) {
$page = $shortcut;
$page['_SHORTCUT_PAGE_UID'] = $page['uid'];
}
$page = $pageRepository->resolveShortcutPage($page, false, $disableGroupAccessCheck);
} catch (\Exception $e) {
// Keep the existing page record if shortcut could not be resolved
}
if ($disableGroupAccessCheck) {
$pageRepository->where_groupAccess = $savedWhereGroupAccess;
}
return $page;
}
......
......@@ -824,6 +824,86 @@ class SlugLinkGeneratorTest extends AbstractTestCase
self::assertSame($expectation, $json);
}
public function hierarchicalMenuSetsActiveStateProperlyDataProvider(): array
{
return [
'regular page' => [
'https://acme.us/',
1310,
'1300',
[
[
'title' => 'EN: Products',
'link' => 'https://products.acme.com/products',
'active' => 1,
'current' => 0,
'children' => [
[
'title' => 'EN: Planets',
'link' => 'https://products.acme.com/products/planets',
'active' => 1,
'current' => 1,
],
[
'title' => 'EN: Spaceships',
'link' => 'https://products.acme.com/products/spaceships',
'active' => 0,
'current' => 0,
],
[
'title' => 'EN: Dark Matter',
'link' => 'https://products.acme.com/products/dark-matter',
'active' => 0,
'current' => 0,
],
],
],
],
],
'resolved shortcut' => [
'https://blog.acme.com/',
2100,
'1930',
[
[
'title' => 'Our Blog',
'link' => '/authors',
'active' => 1,
'current' => 1,
],
],
],
];
}
/**
* @test
* @dataProvider hierarchicalMenuSetsActiveStateProperlyDataProvider
*/
public function hierarchicalMenuSetsActiveStateProperly(string $hostPrefix, int $sourcePageId, string $menuPageIds, array $expectation): void
{
$response = $this->executeFrontendSubRequest(
(new InternalRequest($hostPrefix))
->withPageId($sourcePageId)
->withInstructions([
$this->createHierarchicalMenuProcessorInstruction([
'levels' => 2,
'special' => 'list',
'special.' => [
'value' => $menuPageIds,
],
'includeSpacer' => 1,
'titleField' => 'title',
]),
]),
);
$json = json_decode((string)$response->getBody(), true);
$json = $this->filterMenu($json, ['title', 'active', 'current', 'link']);
self::assertSame($expectation, $json);
}
public function directoryMenuIsGeneratedDataProvider(): array
{
return [
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment