From a396dadda6faa04fe9beb0c4ce72cb5cf7cd947c Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Thu, 7 Jul 2022 10:45:00 +0200
Subject: [PATCH] [TASK] Clean up TMENU code

This change reduces the logic within TMENU in order
to further unify link generation within the menu rendering.

Resolves: #97865
Releases: main
Change-Id: I495b167b7e6e302fc7e8b04757eab1eb8e359528
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/75043
Tested-by: core-ci <typo3@b13.com>
Tested-by: Nikita Hovratov <nikita.h@live.de>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Nikita Hovratov <nikita.h@live.de>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Benni Mack <benni@typo3.org>
---
 .../Menu/TextMenuContentObject.php            | 70 +++++++------------
 1 file changed, 27 insertions(+), 43 deletions(-)

diff --git a/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php b/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php
index 782e4fa2bbc7..afbc34232b1d 100644
--- a/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php
+++ b/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php
@@ -24,16 +24,6 @@ use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
  */
 class TextMenuContentObject extends AbstractMenuContentObject
 {
-    protected ?ContentObjectRenderer $cObjectForCurrentMenu = null;
-
-    protected string $menuContent = '';
-    protected int $totalMenuItems = 0;
-
-    /**
-     * @var array[]
-     */
-    protected array $subMenuObjSuffixes = [];
-
     /**
      * Traverses the ->result array of menu items configuration (made by ->generate()) and renders each item.
      * An instance of ContentObjectRenderer is also made and for each menu item rendered it is loaded with
@@ -48,46 +38,42 @@ class TextMenuContentObject extends AbstractMenuContentObject
         }
 
         $frontendController = $this->getTypoScriptFrontendController();
-        $this->cObjectForCurrentMenu = GeneralUtility::makeInstance(ContentObjectRenderer::class);
-        $this->menuContent = '';
-        $this->totalMenuItems = count($this->result);
+        $cObjectForCurrentMenu = GeneralUtility::makeInstance(ContentObjectRenderer::class);
+        $menuContent = '';
         $typoScriptService = GeneralUtility::makeInstance(TypoScriptService::class);
-        $this->subMenuObjSuffixes = $typoScriptService->explodeConfigurationForOptionSplit(['sOSuffix' => $this->mconf['submenuObjSuffixes'] ?? null], $this->totalMenuItems);
+        $subMenuObjSuffixes = $typoScriptService->explodeConfigurationForOptionSplit(['sOSuffix' => $this->mconf['submenuObjSuffixes'] ?? null], count($this->result));
+        $explicitSpacerRenderingEnabled = ($this->mconf['SPC'] ?? false);
         foreach ($this->result as $key => $val) {
             $frontendController->register['count_HMENU_MENUOBJ']++;
             $frontendController->register['count_MENUOBJ']++;
 
             // Initialize the cObj with the page record of the menu item
-            $this->cObjectForCurrentMenu->start($this->menuArr[$key], 'pages', $this->request);
+            $cObjectForCurrentMenu->start($this->menuArr[$key], 'pages', $this->request);
             $this->I = [];
             $this->I['key'] = $key;
             $this->I['val'] = $val;
             $this->I['title'] = $this->getPageTitle($this->menuArr[$key]['title'] ?? '', $this->menuArr[$key]['nav_title'] ?? '');
             $this->I['title.'] = $this->I['val']['stdWrap.'] ?? [];
-            $this->I['title'] = $this->cObjectForCurrentMenu->stdWrapValue('title', $this->I ?? []);
+            $this->I['title'] = $cObjectForCurrentMenu->stdWrapValue('title', $this->I ?? []);
             $this->I['uid'] = $this->menuArr[$key]['uid'] ?? 0;
             $this->I['mount_pid'] = $this->menuArr[$key]['mount_pid'] ?? 0;
             $this->I['pid'] = $this->menuArr[$key]['pid'] ?? 0;
             $this->I['spacer'] = $this->menuArr[$key]['isSpacer'] ?? false;
             // Make link tag
-            $this->I['val']['additionalParams'] = $this->cObjectForCurrentMenu->stdWrapValue('additionalParams', $this->I['val']);
+            $this->I['val']['additionalParams'] = $cObjectForCurrentMenu->stdWrapValue('additionalParams', $this->I['val']);
             $this->I['linkHREF'] = $this->link((int)$key, (string)($this->I['val']['altTarget'] ?? ''), ($this->mconf['forceTypeValue'] ?? ''));
             if (empty($this->I['linkHREF'])) {
                 $this->I['val']['doNotLinkIt'] = 1;
             }
-            // Title attribute of links:
-            $titleAttrValue = $this->cObjectForCurrentMenu->stdWrapValue('ATagTitle', $this->I['val']);
+            // Title attribute of links
+            $titleAttrValue = $cObjectForCurrentMenu->stdWrapValue('ATagTitle', $this->I['val']);
             if ($titleAttrValue !== '') {
                 $this->I['linkHREF']['title'] = $titleAttrValue;
             }
 
-            // stdWrap for doNotLinkIt
-            $this->I['val']['doNotLinkIt'] = $this->cObjectForCurrentMenu->stdWrapValue('doNotLinkIt', $this->I['val']);
+            $this->I['val']['doNotLinkIt'] = (bool)$cObjectForCurrentMenu->stdWrapValue('doNotLinkIt', $this->I['val']);
             // Compile link tag
-            if (!$this->I['val']['doNotLinkIt']) {
-                $this->I['val']['doNotLinkIt'] = 0;
-            }
-            if (!$this->I['spacer'] && $this->I['val']['doNotLinkIt'] != 1) {
+            if (!$this->I['spacer'] && !$this->I['val']['doNotLinkIt']) {
                 $this->setATagParts();
             } else {
                 $this->I['A1'] = '';
@@ -102,17 +88,17 @@ class TextMenuContentObject extends AbstractMenuContentObject
                 $wrapPartsAfter = explode('|', $this->I['val']['linkWrap'] ?? '');
             }
             if (($this->I['val']['stdWrap2'] ?? false) || isset($this->I['val']['stdWrap2.'])) {
-                $stdWrap2 = (string)(isset($this->I['val']['stdWrap2.']) ? $this->cObjectForCurrentMenu->stdWrap('|', $this->I['val']['stdWrap2.']) : '|');
+                $stdWrap2 = (string)(isset($this->I['val']['stdWrap2.']) ? $cObjectForCurrentMenu->stdWrap('|', $this->I['val']['stdWrap2.']) : '|');
                 $wrapPartsStdWrap = explode($this->I['val']['stdWrap2'] ?: '|', $stdWrap2);
             } else {
                 $wrapPartsStdWrap = ['', ''];
             }
             // Make before, middle and after parts
             $this->I['parts'] = [];
-            $this->I['parts']['before'] = $this->getBeforeAfter('before');
+            $this->I['parts']['before'] = $this->getBeforeAfter('before', $cObjectForCurrentMenu);
             $this->I['parts']['stdWrap2_begin'] = $wrapPartsStdWrap[0];
             // stdWrap for doNotShowLink
-            $this->I['val']['doNotShowLink'] = $this->cObjectForCurrentMenu->stdWrapValue('doNotShowLink', $this->I['val']);
+            $this->I['val']['doNotShowLink'] = $cObjectForCurrentMenu->stdWrapValue('doNotShowLink', $this->I['val']);
             if (!$this->I['val']['doNotShowLink']) {
                 $this->I['parts']['notATagBeforeWrap_begin'] = $wrapPartsAfter[0] ?? '';
                 $this->I['parts']['ATag_begin'] = $this->I['A1'];
@@ -123,37 +109,34 @@ class TextMenuContentObject extends AbstractMenuContentObject
                 $this->I['parts']['notATagBeforeWrap_end'] = $wrapPartsAfter[1] ?? '';
             }
             $this->I['parts']['stdWrap2_end'] = $wrapPartsStdWrap[1];
-            $this->I['parts']['after'] = $this->getBeforeAfter('after');
+            $this->I['parts']['after'] = $this->getBeforeAfter('after', $cObjectForCurrentMenu);
             // Passing I to a user function
             if ($this->mconf['IProcFunc'] ?? false) {
                 $this->I = $this->userProcess('IProcFunc', $this->I);
             }
             // Merge parts + beforeAllWrap
             $this->I['theItem'] = implode('', $this->I['parts']);
-            // allWrap:
-            $allWrap = $this->cObjectForCurrentMenu->stdWrapValue('allWrap', $this->I['val']);
-            $this->I['theItem'] = $this->cObjectForCurrentMenu->wrap($this->I['theItem'], $allWrap);
+            $allWrap = $cObjectForCurrentMenu->stdWrapValue('allWrap', $this->I['val']);
+            $this->I['theItem'] = $cObjectForCurrentMenu->wrap($this->I['theItem'], $allWrap);
             if ($this->I['val']['subst_elementUid'] ?? false) {
                 $this->I['theItem'] = str_replace('{elementUid}', (string)$this->I['uid'], $this->I['theItem']);
             }
-            // allStdWrap:
             if (is_array($this->I['val']['allStdWrap.'] ?? null)) {
-                $this->I['theItem'] = $this->cObjectForCurrentMenu->stdWrap($this->I['theItem'], $this->I['val']['allStdWrap.']);
+                $this->I['theItem'] = $cObjectForCurrentMenu->stdWrap($this->I['theItem'], $this->I['val']['allStdWrap.']);
             }
-            $explicitSpacerRenderingEnabled = ($this->mconf['SPC'] ?? false);
             $isSpacerPage = $this->I['spacer'] ?? false;
             // If rendering of SPACERs is enabled, also allow rendering submenus with Spacers
             if (!$isSpacerPage || $explicitSpacerRenderingEnabled) {
                 // Add part to the accumulated result + fetch submenus
-                $this->I['theItem'] .= $this->subMenu($this->I['uid'], $this->subMenuObjSuffixes[$key]['sOSuffix'] ?? '', $key);
+                $this->I['theItem'] .= $this->subMenu($this->I['uid'], $subMenuObjSuffixes[$key]['sOSuffix'] ?? '', $key);
             }
-            $part = $this->cObjectForCurrentMenu->stdWrapValue('wrapItemAndSub', $this->I['val']);
-            $this->menuContent .= $part ? $this->cObjectForCurrentMenu->wrap($this->I['theItem'], $part) : $this->I['theItem'];
+            $part = $cObjectForCurrentMenu->stdWrapValue('wrapItemAndSub', $this->I['val']);
+            $menuContent .= $part ? $cObjectForCurrentMenu->wrap($this->I['theItem'], $part) : $this->I['theItem'];
         }
         if (is_array($this->mconf['stdWrap.'] ?? null)) {
-            $this->menuContent = (string)$this->cObjectForCurrentMenu->stdWrap($this->menuContent, $this->mconf['stdWrap.']);
+            $menuContent = (string)$cObjectForCurrentMenu->stdWrap($menuContent, $this->mconf['stdWrap.']);
         }
-        return $this->cObjectForCurrentMenu->wrap($this->menuContent, $this->mconf['wrap'] ?? '');
+        return $cObjectForCurrentMenu->wrap($menuContent, $this->mconf['wrap'] ?? '');
     }
 
     /**
@@ -165,13 +148,14 @@ class TextMenuContentObject extends AbstractMenuContentObject
      * - afterWrap
      *
      * @param string $pref Can be "before" or "after" and determines which kind of stdWrap to process (basically this is the prefix of the TypoScript properties that are read from the ->I['val'] array
+     * @param ContentObjectRenderer $cObjectForCurrentMenu
      * @return string The resulting HTML
      */
-    protected function getBeforeAfter(string $pref): string
+    protected function getBeforeAfter(string $pref, ContentObjectRenderer $cObjectForCurrentMenu): string
     {
-        $processedPref = $this->cObjectForCurrentMenu->stdWrapValue($pref, $this->I['val']);
+        $processedPref = $cObjectForCurrentMenu->stdWrapValue($pref, $this->I['val']);
         if (isset($this->I['val'][$pref . 'Wrap'])) {
-            return $this->cObjectForCurrentMenu->wrap($processedPref, $this->I['val'][$pref . 'Wrap']);
+            return $cObjectForCurrentMenu->wrap($processedPref, $this->I['val'][$pref . 'Wrap']);
         }
         return $processedPref;
     }
-- 
GitLab