From a295896482f5c1b81ca25e4728127a26c052d455 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Sat, 29 Sep 2018 20:57:24 +0200
Subject: [PATCH] [BUGFIX] Skip special doktype parent pages in slug generation

When checking the parent page for an existing slug, the parent page
should not be used if the parent page is a sys folder, spacer or
recycler (pages.doktype), but the traversal goes up.

This makes it a bit easier for editors to work with better-speaking
URLs and keep the rootline traversal as best practice.

Resolves: #86456
Releases: master
Change-Id: I2c46d096e41fb3a325fd42bf86b8968b79d6305b
Reviewed-on: https://review.typo3.org/58473
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
---
 .../core/Classes/DataHandling/SlugHelper.php  | 29 ++++++++++++-------
 .../Unit/DataHandling/SlugHelperTest.php      | 16 +++++-----
 2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/typo3/sysext/core/Classes/DataHandling/SlugHelper.php b/typo3/sysext/core/Classes/DataHandling/SlugHelper.php
index 397063a8527d..4c4fa3786e99 100644
--- a/typo3/sysext/core/Classes/DataHandling/SlugHelper.php
+++ b/typo3/sysext/core/Classes/DataHandling/SlugHelper.php
@@ -164,14 +164,7 @@ class SlugHelper
         if ($this->configuration['generatorOptions']['prefixParentPageSlug'] ?? false) {
             $languageFieldName = $GLOBALS['TCA'][$this->tableName]['ctrl']['languageField'] ?? null;
             $languageId = (int)($recordData[$languageFieldName] ?? 0);
-            $rootLine = $this->resolveRootLine($pid);
-            $parentPageRecord = reset($rootLine);
-            if ($languageId > 0) {
-                $localizedParentPageRecord = BackendUtility::getRecordLocalization('pages', $parentPageRecord['uid'], $languageId);
-                if (!empty($localizedParentPageRecord)) {
-                    $parentPageRecord = reset($localizedParentPageRecord);
-                }
-            }
+            $parentPageRecord = $this->resolveParentPageRecord($pid, $languageId);
             if (is_array($parentPageRecord)) {
                 // If the parent page has a slug, use that instead of "re-generating" the slug from the parents' page title
                 if (!empty($parentPageRecord['slug'])) {
@@ -535,11 +528,25 @@ class SlugHelper
     }
 
     /**
+     * Fetch a parent page, but exclude spacers, recyclers and sys-folders and all doktypes > 200
      * @param int $pid
-     * @return array
+     * @param int $languageId
+     * @return array|null
      */
-    protected function resolveRootLine(int $pid): array
+    protected function resolveParentPageRecord(int $pid, int $languageId): ?array
     {
-        return BackendUtility::BEgetRootLine($pid, '', true, ['nav_title']);
+        $parentPageRecord = null;
+        $rootLine = BackendUtility::BEgetRootLine($pid, '', true, ['nav_title']);
+        do {
+            $parentPageRecord = array_shift($rootLine);
+            // do not use spacers (199), recyclers and folders and everything else
+        } while (!empty($rootLine) && (int)$parentPageRecord['doktype'] >= 199);
+        if ($languageId > 0) {
+            $localizedParentPageRecord = BackendUtility::getRecordLocalization('pages', $parentPageRecord['uid'], $languageId);
+            if (!empty($localizedParentPageRecord)) {
+                $parentPageRecord = reset($localizedParentPageRecord);
+            }
+        }
+        return $parentPageRecord;
     }
 }
diff --git a/typo3/sysext/core/Tests/Unit/DataHandling/SlugHelperTest.php b/typo3/sysext/core/Tests/Unit/DataHandling/SlugHelperTest.php
index e72b4d66b23c..0f364d3cb3ac 100644
--- a/typo3/sysext/core/Tests/Unit/DataHandling/SlugHelperTest.php
+++ b/typo3/sysext/core/Tests/Unit/DataHandling/SlugHelperTest.php
@@ -379,16 +379,14 @@ class SlugHelperTest extends UnitTestCase
     public function generatePrependsSlugsForPages(string $input, string $expected)
     {
         $GLOBALS['dummyTable']['ctrl'] = [];
-        $rootLine = [
-            [
-                'uid' => '13',
-                'pid' => '10',
-                'title' => 'Parent Page',
-            ]
+        $parentPage = [
+            'uid' => '13',
+            'pid' => '10',
+            'title' => 'Parent Page',
         ];
         $subject = $this->getAccessibleMock(
             SlugHelper::class,
-            ['resolveRootLine'],
+            ['resolveParentPageRecord'],
             [
                 'pages',
                 'slug',
@@ -401,9 +399,9 @@ class SlugHelperTest extends UnitTestCase
             ]
         );
         $subject->expects(static::at(0))
-            ->method('resolveRootLine')->with(13)->willReturn($rootLine);
+            ->method('resolveParentPageRecord')->with(13)->willReturn($parentPage);
         $subject->expects(static::at(1))
-            ->method('resolveRootLine')->with(10)->willReturn([]);
+            ->method('resolveParentPageRecord')->with(10)->willReturn(null);
         static::assertEquals(
             $expected,
             $subject->generate(['title' => $input, 'uid' => 13], 13)
-- 
GitLab