[BUGFIX] Use correct language in l18n_cfg checks 67/58267/8
authorDaniel Goerz <daniel.goerz@posteo.de>
Thu, 13 Sep 2018 13:12:25 +0000 (15:12 +0200)
committerOliver Hader <oliver.hader@typo3.org>
Thu, 13 Sep 2018 20:00:47 +0000 (22:00 +0200)
The check for pages.l18n_cfg is wrong now, as this is
always done against pages.sys_language_uid=0 records (as
"resolvePage" is called right before).

So, these if-statements have to go further south.

On top, the getPageOverlay call needs to be done - on top.

resolvePage -> get default language of page
getPageOverlay -> put the wanted translation on top

A fix for pages.alias has to be in place, as DataHandler
cannot handle pages.alias yet.

Resolves: #86242
Releases: master
Change-Id: Ief99e5f934f6e9d31973b9543cb9a6e599d2d33c
Reviewed-on: https://review.typo3.org/58267
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
typo3/sysext/frontend/Classes/Page/PageRepository.php
typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/PlainScenario.yaml
typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
typo3/sysext/frontend/Tests/Functional/SiteHandling/LinkGeneratorTest.php
typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugLinkGeneratorTest.php

index 040b815..4d9dcd0 100644 (file)
@@ -599,6 +599,7 @@ class PageRepository implements LoggerAwareInterface
                     // Unset vital fields that are NOT allowed to be overlaid:
                     unset($row['uid']);
                     unset($row['pid']);
+                    unset($row['alias']);
                     $overlays[$origUid] = $row;
                 }
             }
index 18a14ad..83be0eb 100644 (file)
@@ -64,27 +64,12 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
         }
 
         // Looking up the page record to verify its existence:
-        $pageRepository = $this->buildPageRepository();
-        $page = $this->resolvePage(
-            $pageRepository,
-            $linkDetails,
-            $conf,
-            $disableGroupAccessCheck
-        );
+        $page = $this->resolvePage($linkDetails, $conf, $disableGroupAccessCheck);
 
         if (empty($page)) {
             throw new UnableToLinkException('Page id "' . $linkDetails['typoLinkParameter'] . '" was not found, so "' . $linkText . '" was not linked.', 1490987336, null, $linkText);
         }
 
-        $languageField = $GLOBALS['TCA']['pages']['ctrl']['languageField'] ?? null;
-        $language = (int)($page[$languageField] ?? 0);
-        if ($language === 0 && GeneralUtility::hideIfDefaultLanguage($page['l18n_cfg'])) {
-            throw new UnableToLinkException('Default language of page  "' . $linkDetails['typoLinkParameter'] . '" is hidden, so "' . $linkText . '" was not linked.', 1529527301, null, $linkText);
-        }
-        if ($language > 0 && !isset($page['_PAGES_OVERLAY']) && GeneralUtility::hideIfNotTranslated($page['l18n_cfg'])) {
-            throw new UnableToLinkException('Fallback to default language of page "' . $linkDetails['typoLinkParameter'] . '" is disabled, so "' . $linkText . '" was not linked.', 1529527488, null, $linkText);
-        }
-
         foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['typolinkProcessing']['typolinkModifyParameterForPageLinks'] ?? [] as $classData) {
             $hookObject = GeneralUtility::makeInstance($classData);
             if (!$hookObject instanceof TypolinkModifyLinkConfigForPageLinksHookInterface) {
@@ -176,6 +161,27 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
             unset($params);
         }
 
+        // get config.linkVars and prepend them before the actual GET parameters
+        $queryParameters = [];
+        parse_str($addQueryParams, $queryParameters);
+        if ($tsfe->linkVars) {
+            $globalQueryParameters = [];
+            parse_str($tsfe->linkVars, $globalQueryParameters);
+            $queryParameters = array_replace_recursive($globalQueryParameters, $queryParameters);
+        }
+        // Disable "?id=", for pages with no site configuration, this is added later-on anyway
+        unset($queryParameters['id']);
+
+        // Override language property if not being set already
+        if (isset($queryParameters['L']) && !isset($conf['language'])) {
+            $conf['language'] = (int)$queryParameters['L'];
+        }
+
+        // Now overlay the page in the target language, in order to have valid title attributes etc.
+        if (isset($conf['language']) && $conf['language'] > 0 && $conf['language'] !== 'current') {
+            $page = $tsfe->sys_page->getPageOverlay($page, (int)$conf['language']);
+        }
+
         // Check if the target page has a site configuration
         try {
             $siteOfTargetPage = GeneralUtility::makeInstance(SiteMatcher::class)->matchByPageId((int)$page['uid']);
@@ -185,35 +191,14 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
             $siteOfTargetPage = null;
             $currentSite = null;
         }
+
         // Link to a page that has a site configuration
         if ($siteOfTargetPage instanceof Site) {
-            $queryParameters = [];
-            parse_str($addQueryParams, $queryParameters);
-            // get config.linkVars and prepend
-            if ($tsfe->linkVars) {
-                $globalQueryParameters = [];
-                parse_str($tsfe->linkVars, $globalQueryParameters);
-                if (!empty($globalQueryParameters)) {
-                    // override $globalQueryParameters with $queryParameters
-                    $queryParameters = array_replace_recursive(
-                        $globalQueryParameters,
-                        $queryParameters
-                    );
-                }
-            }
-            // Override language property if not being set already
-            if (isset($queryParameters['L']) && !isset($conf['language'])) {
-                $conf['language'] = (int)$queryParameters['L'];
-            }
-            unset($queryParameters['id'], $queryParameters['L']);
+            // No need for any L parameter with Site handling
+            unset($queryParameters['L']);
             if ($pageType) {
                 $queryParameters['type'] = (int)$pageType;
             }
-
-            if (isset($conf['language'])) {
-                $page = $tsfe->sys_page->getPageOverlay($page, (int)$conf['language']);
-            }
-
             // Generate the URL
             $url = $this->generateUrlForPageWithSiteConfiguration($page, $siteOfTargetPage, $queryParameters, $sectionMark, $conf);
 
@@ -239,7 +224,21 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
                 }
             }
         } else {
-            list($url, $target) = $this->generateUrlForPageWithoutSiteConfiguration($page, $addQueryParams, $conf, $pageType, $sectionMark, $target, $MPvarAcc);
+            // If the typolink.language parameter was set, ensure that this is added to L query parameter
+            if (!isset($queryParameters['L']) && MathUtility::canBeInterpretedAsInteger($conf['language'] ?? false)) {
+                $queryParameters['L'] = $conf['language'];
+            }
+            list($url, $target) = $this->generateUrlForPageWithoutSiteConfiguration($page, $queryParameters, $conf, $pageType, $sectionMark, $target, $MPvarAcc);
+        }
+
+        $languageField = $GLOBALS['TCA']['pages']['ctrl']['languageField'] ?? null;
+        $languageOfPageRecord = (int)($page[$languageField] ?? 0);
+
+        if ($languageOfPageRecord === 0 && GeneralUtility::hideIfDefaultLanguage($page['l18n_cfg'])) {
+            throw new UnableToLinkException('Default language of page  "' . $linkDetails['typoLinkParameter'] . '" is hidden, so "' . $linkText . '" was not linked.', 1529527301, null, $linkText);
+        }
+        if ($languageOfPageRecord > 0 && !isset($page['_PAGES_OVERLAY']) && GeneralUtility::hideIfNotTranslated($page['l18n_cfg'])) {
+            throw new UnableToLinkException('Fallback to default language of page "' . $linkDetails['typoLinkParameter'] . '" is disabled, so "' . $linkText . '" was not linked.', 1529527488, null, $linkText);
         }
 
         // If link is to an access restricted page which should be redirected, then find new URL:
@@ -275,15 +274,16 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
      * language parent, adjusts `$linkDetails['pageuid']` (for hook processing)
      * and modifies `$configuration['language']` (for language URL generation).
      *
-     * @param PageRepository $pageRepository
      * @param array $linkDetails
      * @param array $configuration
      * @param bool $disableGroupAccessCheck
      * @return array
      */
-    protected function resolvePage(PageRepository $pageRepository, array &$linkDetails, array &$configuration, bool $disableGroupAccessCheck): array
+    protected function resolvePage(array &$linkDetails, array &$configuration, bool $disableGroupAccessCheck): array
     {
-        // Looking up the page record to verify its existence:
+        $pageRepository = $this->buildPageRepository();
+        // Looking up the page record to verify its existence
+        // This is used when a page to a translated page is executed directly.
         $page = $pageRepository->getPage($linkDetails['pageuid'], $disableGroupAccessCheck);
 
         if (empty($page) || !is_array($page)) {
@@ -294,10 +294,12 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
         $languageParentField = $GLOBALS['TCA']['pages']['ctrl']['transOrigPointerField'] ?? null;
         $language = (int)($page[$languageField] ?? 0);
 
-        if ($language <= 0 || empty($page[$languageParentField])) {
+        // The page that should be linked is actually a default-language page, nothing to do here.
+        if ($language === 0 || empty($page[$languageParentField])) {
             return $page;
         }
 
+        // Let's fetch the default-language page now
         $languageParentPage = $pageRepository->getPage(
             $page[$languageParentField],
             $disableGroupAccessCheck
@@ -306,8 +308,10 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
             return $page;
         }
 
+        // Set the "pageuid" to the default-language page ID.
         $linkDetails['pageuid'] = (int)$languageParentPage['uid'];
         $configuration['language'] = $language;
+        $linkDetails['parameters'] .= '&L=' . $language;
         return $languageParentPage;
     }
 
@@ -375,7 +379,7 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
      * Generate a URL for a page without site configuration
      *
      * @param array $page
-     * @param string $additionalQueryParams
+     * @param array $additionalQueryParams
      * @param array $conf
      * @param string $pageType
      * @param string $sectionMark
@@ -383,20 +387,12 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
      * @param array $MPvarAcc
      * @return array
      */
-    protected function generateUrlForPageWithoutSiteConfiguration(array $page, string $additionalQueryParams, array $conf, string $pageType, string $sectionMark, string $target, array $MPvarAcc): array
+    protected function generateUrlForPageWithoutSiteConfiguration(array $page, array $additionalQueryParams, array $conf, string $pageType, string $sectionMark, string $target, array $MPvarAcc): array
     {
-        // new 'language' property takes precedence over '&L=1' if numeric
-        // here 'additionalParams=&L={language-value}' will be overridden
-        if (MathUtility::canBeInterpretedAsInteger($conf['language'] ?? '')) {
-            $queryParameters = [];
-            parse_str($additionalQueryParams, $queryParameters);
-            $queryParameters['L'] = $conf['language'];
-            $additionalQueryParams = http_build_query(
-                $queryParameters,
-                '',
-                '&',
-                PHP_QUERY_RFC3986
-            );
+        // Build a string out of the query parameters
+        $additionalQueryParams = http_build_query($additionalQueryParams, '', '&', PHP_QUERY_RFC3986);
+        if (!empty($additionalQueryParams)) {
+            $additionalQueryParams = '&' . $additionalQueryParams;
         }
 
         $tsfe = $this->getTypoScriptFrontendController();
index 061f6cd..7b50644 100644 (file)
@@ -76,6 +76,15 @@ entities:
             - self: {id: 1310, title: 'EN: Planets'}
             - self: {id: 1320, title: 'EN: Spaceships'}
             - self: {id: 1330, title: 'EN: Dark Matter'}
+        - self: {id: 1400, title: 'EN: ACME in your Region', root: true}
+          languageVariants:
+            - self: {id: 1401, title: 'FR: ACME in your Region', language: 1}
+            - self: {id: 1402, title: 'FR-CA: ACME in your Region', language: 2}
+          children:
+            - self: {id: 1410, title: 'EN: Groups', l18n_cfg: 1}
+              languageVariants:
+                - self: {id: 1411, title: 'FR: Groups', language: 1}
+                - self: {id: 1412, title: 'FR-CA: Groups', language: 2}
         - self: {id: 1500, title: 'Internal'}
           children:
             - self: {id: 1510, title: 'Whitepapers', visitorGroups: -2, extendToSubpages: true}
index 095cdfe..cacf4a9 100644 (file)
@@ -76,6 +76,15 @@ entities:
             - self: {id: 1310, title: 'EN: Planets', slug: '/products/planets'}
             - self: {id: 1320, title: 'EN: Spaceships', slug: '/products/spaceships'}
             - self: {id: 1330, title: 'EN: Dark Matter', slug: '/products/dark-matter'}
+        - self: {id: 1400, title: 'EN: ACME in your Region', root: true, slug: '/acme-in-your-region'}
+          languageVariants:
+            - self: {id: 1401, title: 'FR: ACME in your Region', language: 1, slug: '/acme-dans-votre-region'}
+            - self: {id: 1402, title: 'FR-CA: ACME in your Region', language: 2, slug: '/acme-dans-votre-quebec'}
+          children:
+            - self: {id: 1410, title: 'EN: Groups', slug: '/acme-in-your-region/groups', l18n_cfg: 1}
+              languageVariants:
+                - self: {id: 1411, title: 'FR: Groups', language: 1, slug: '/acme-dans-votre-region/groupes'}
+                - self: {id: 1412, title: 'FR-CA: Groups', language: 2, slug: '/acme-dans-votre-quebec/groupes'}
         - self: {id: 1500, title: 'Internal', slug: '/my-acme'}
           children:
             - self: {id: 1510, title: 'Whitepapers', visitorGroups: -2, extendToSubpages: true, slug: '/my-acme/whitepapers'}
index 0fd63b5..294ece9 100644 (file)
@@ -258,6 +258,12 @@ class LinkGeneratorTest extends AbstractTestCase
             // acme.com -> products.acme.com (nested sub-site)
             [1100, 1300, 0, 'index.php?id=1300&L=0'],
             [1100, 1310, 0, 'index.php?id=1310&L=0'],
+            // acme.com -> products.acme.com (nested sub-site, l18n_cfg=1)
+            [1100, 1410, 0, ''],
+            [1100, 1410, 1, 'index.php?id=1410&L=1'],
+            [1100, 1410, 2, 'index.php?id=1410&L=2'],
+            [1100, 1411, 0, 'index.php?id=1410&L=1'],
+            [1100, 1412, 0, 'index.php?id=1410&L=2'],
             // acme.com -> archive (outside site)
             [1100, 3100, 0, 'index.php?id=3100&L=0'],
             [1100, 3100, 1, 'index.php?id=3100&L=1'],
@@ -627,6 +633,7 @@ class LinkGeneratorTest extends AbstractTestCase
                             ],
                         ],
                     ],
+                    ['title' => 'EN: ACME in your Region', 'link' => 'index.php?id=1400'],
                     ['title' => 'Internal', 'link' => 'index.php?id=1500'],
                     ['title' => 'About us', 'link' => 'index.php?id=1600'],
                     [
index 6613d2f..d010e41 100644 (file)
@@ -281,6 +281,12 @@ class SlugLinkGeneratorTest extends AbstractTestCase
             // acme.com -> products.acme.com (nested sub-site)
             ['https://acme.us/', 1100, 1300, 0, 'https://products.acme.com/products'],
             ['https://acme.us/', 1100, 1310, 0, 'https://products.acme.com/products/planets'],
+            // acme.com -> products.acme.com (nested sub-site, l18n_cfg=1)
+            ['https://acme.us/', 1100, 1410, 0, ''],
+            ['https://acme.us/', 1100, 1410, 1, 'https://acme.fr/acme-dans-votre-region/groupes'],
+            ['https://acme.us/', 1100, 1410, 2, 'https://acme.ca/acme-dans-votre-quebec/groupes'],
+            ['https://acme.us/', 1100, 1411, 0, 'https://acme.fr/acme-dans-votre-region/groupes'],
+            ['https://acme.us/', 1100, 1412, 0, 'https://acme.ca/acme-dans-votre-quebec/groupes'],
             // acme.com -> archive (outside site)
             ['https://acme.us/', 1100, 3100, 0, '/index.php?id=3100&L=0'],
             ['https://acme.us/', 1100, 3100, 1, '/index.php?id=3100&L=1'],
@@ -677,6 +683,7 @@ class SlugLinkGeneratorTest extends AbstractTestCase
                             ],
                         ],
                     ],
+                    ['title' => 'EN: ACME in your Region', 'link' => '/acme-in-your-region'],
                     ['title' => 'Internal', 'link' => '/my-acme'],
                     ['title' => 'About us', 'link' => '/about'],
                     [