[TASK] Enforce shortcut title to be set by controllers

The ShortcutRepository should not deal with generating
shortcut titles based on the provided arguments. This can
never be reliable, especially for custom extension code.
The appropriate title must be set by the calling controller
since this is the place where all necessary information,
to define such title, are available.

Therefore, adding a new shortcut button without defining
a display name is deprecated. All Core controllers are
adjusted to provide the necessary title themself.

Resolves: #93060
Releases: master
Change-Id: Ic15fe13769dec841868977a862464f8dd3c73c42
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/67096
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Benni Mack <benni@typo3.org>

typo3/sysext/backend/Classes/Backend/Shortcut/ShortcutRepository.php

@@ -193,6 +193,8 @@ public function shortcutExists(string $url): bool
      */
     public function addShortcut(string $url, string $module, string $parentModule = '', string $title = ''): bool
     {
+        // @todo $parentModule can not longer be set using public API.
+
         if (empty($url) || empty($module)) {
             return false;
         }
@@ -206,7 +208,6 @@ public function addShortcut(string $url, string $module, string $parentModule =
         }
 
         $languageService = $this->getLanguageService();
-        $title = $title ?: 'Shortcut';
         $titlePrefix = '';
         $type = 'other';
         $table = '';
@@ -232,49 +233,55 @@ public function addShortcut(string $url, string $module, string $parentModule =
             }
         }
 
-        // Check if given id is a combined identifier
-        if (!empty($queryParameters['id']) && preg_match('/^[\d]+:/', $queryParameters['id'])) {
-            try {
-                $resourceFactory = GeneralUtility::makeInstance(ResourceFactory::class);
-                $resource = $resourceFactory->getObjectFromCombinedIdentifier($queryParameters['id']);
-                $title = trim(sprintf(
-                    '%s (%s)',
-                    $titlePrefix,
-                    $resource->getName()
-                ));
-            } catch (ResourceDoesNotExistException $e) {
-            }
-        } else {
-            // Lookup the title of this page and use it as default description
-            $pageId = $pageId ?: $recordId ?: $this->extractPageIdFromShortcutUrl($url);
-            $page = $pageId ? BackendUtility::getRecord('pages', $pageId) : null;
-
-            if (!empty($page)) {
-                // Set the name to the title of the page
-                if ($type === 'other') {
-                    $title = sprintf(
+        // Only apply "magic" if title is not set
+        // @todo This is deprecated and can be removed in v12
+        if ($title === '') {
+            // Check if given id is a combined identifier
+            if (!empty($queryParameters['id']) && preg_match('/^[\d]+:/', $queryParameters['id'])) {
+                try {
+                    $resourceFactory = GeneralUtility::makeInstance(ResourceFactory::class);
+                    $resource = $resourceFactory->getObjectFromCombinedIdentifier($queryParameters['id']);
+                    $title = trim(sprintf(
                         '%s (%s)',
-                        $title,
-                        $page['title']
-                    );
-                } else {
-                    $title = sprintf(
-                        '%s %s (%s)',
                         $titlePrefix,
-                        $languageService->sL($GLOBALS['TCA'][$table]['ctrl']['title']),
-                        $page['title']
-                    );
+                        $resource->getName()
+                    ));
+                } catch (ResourceDoesNotExistException $e) {
+                }
+            } else {
+                // Lookup the title of this page and use it as default description
+                $pageId = $pageId ?: $recordId ?: $this->extractPageIdFromShortcutUrl($url);
+                $page = $pageId ? BackendUtility::getRecord('pages', $pageId) : null;
+
+                if (!empty($page)) {
+                    // Set the name to the title of the page
+                    if ($type === 'other') {
+                        $title = sprintf(
+                            '%s (%s)',
+                            $title,
+                            $page['title']
+                        );
+                    } else {
+                        $title = sprintf(
+                            '%s %s (%s)',
+                            $titlePrefix,
+                            $languageService->sL($GLOBALS['TCA'][$table]['ctrl']['title']),
+                            $page['title']
+                        );
+                    }
+                } elseif (!empty($table)) {
+                    $title = trim(sprintf(
+                        '%s %s',
+                        $titlePrefix,
+                        $languageService->sL($GLOBALS['TCA'][$table]['ctrl']['title'])
+                    ));
                 }
-            } elseif (!empty($table)) {
-                $title = trim(sprintf(
-                    '%s %s',
-                    $titlePrefix,
-                    $languageService->sL($GLOBALS['TCA'][$table]['ctrl']['title'])
-                ));
             }
         }
 
-        if ($title === 'Shortcut') {
+        // In case title is still empty try to set the modules short description label
+        // @todo This is deprecated and can be removed in v12
+        if ($title === '') {
             $moduleLabels = $this->moduleLoader->getLabelsForModule($module);
 
             if (!empty($moduleLabels['shortdescription'])) {
@@ -290,7 +297,7 @@ public function addShortcut(string $url, string $module, string $parentModule =
                 'userid' => $this->getBackendUser()->user['uid'],
                 'module_name' => $module . '|' . $parentModule,
                 'url' => $url,
-                'description' => $title,
+                'description' => $title ?: 'Shortcut',
                 'sorting' => $GLOBALS['EXEC_TIME'],
             ])
             ->execute();

typo3/sysext/backend/Classes/Controller/EditDocumentController.php

@@ -1805,7 +1805,10 @@ protected function registerShortcutButtonToButtonBar(ButtonBar $buttonBar, strin
                 }
             }
             $shortCutButton = $this->moduleTemplate->getDocHeaderComponent()->getButtonBar()->makeShortcutButton();
-            $shortCutButton->setModuleName('xMOD_alt_doc.php')->setArguments($arguments);
+            $shortCutButton
+                ->setModuleName('xMOD_alt_doc.php')
+                ->setDisplayName($this->getShortcutTitle($request))
+                ->setArguments($arguments);
             $buttonBar->addButton($shortCutButton, $position, $group);
         }
     }
@@ -2445,6 +2448,57 @@ protected function closeDocument($mode, ServerRequestInterface $request): ?Respo
         return new RedirectResponse($retUrl, 303);
     }
 
+    /**
+     * Returns the shortcut title for the current element
+     *
+     * @param ServerRequestInterface $request
+     * @return string
+     */
+    protected function getShortcutTitle(ServerRequestInterface $request): string
+    {
+        $queryParameters = $request->getQueryParams();
+        $languageService = $this->getLanguageService();
+
+        if (!is_array($queryParameters['edit'] ?? false)) {
+            return '';
+        }
+
+        // @todo There may be a more efficient way in using FormEngine FormData.
+        // @todo Therefore, the button initialization however has to take place at a later stage.
+
+        $table = (string)key($queryParameters['edit']);
+        $tableTitle = $languageService->sL($GLOBALS['TCA'][$table]['ctrl']['title'] ?? '') ?: $table;
+        $recordId = (int)key($queryParameters['edit'][$table]);
+        $action = (string)$queryParameters['edit'][$table][$recordId];
+
+        if ($action === 'new') {
+            return $languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.createNew') . ' ' . $tableTitle;
+        }
+
+        if ($action === 'edit') {
+            $record = BackendUtility::getRecord($table, $recordId);
+            $recordTitle = BackendUtility::getRecordTitle($table, $record) ?? '';
+            if ($table === 'pages') {
+                return sprintf($languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.editPage'), $tableTitle, $recordTitle);
+            }
+            if (!isset($record['pid'])) {
+                return $languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.edit');
+            }
+            $pageId = (int)$record['pid'];
+            if ($pageId === 0) {
+                return sprintf($languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.editRecordRootLevel'), $tableTitle, $recordTitle);
+            }
+            $pageRow = BackendUtility::getRecord('pages', $pageId);
+            $pageTitle = BackendUtility::getRecordTitle('pages', $pageRow);
+            if ($recordTitle !== '') {
+                return sprintf($languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.editRecord'), $tableTitle, $recordTitle, $pageTitle);
+            }
+            return sprintf($languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.editRecordNoTitle'), $tableTitle, $pageTitle);
+        }
+
+        return '';
+    }
+
     /**
      * @return \TYPO3\CMS\Core\Authentication\BackendUserAuthentication
      */

typo3/sysext/backend/Classes/Controller/HelpController.php

@@ -155,15 +155,11 @@ public function detailAction(ServerRequestInterface $request)
         $table = $request->getQueryParams()['table'] ?? $request->getParsedBody()['table'];
         $field = $request->getQueryParams()['field'] ?? $request->getParsedBody()['field'] ?? '*';
 
-        $mainKey = $table;
-
         $this->view->assignMultiple([
             'table' => $table,
-            'key' => $mainKey,
+            'key' => $table,
             'field' => $field,
-            'manuals' => $field === '*'
-                ? $this->tableManualRepository->getTableManual($mainKey)
-                : [$this->tableManualRepository->getSingleManual($mainKey, $field)],
+            'manuals' => $this->getManuals($request)
         ]);
     }
 
@@ -179,6 +175,7 @@ protected function registerDocheaderButtons(ServerRequestInterface $request)
         $action = $request->getQueryParams()['action'] ?? $request->getParsedBody()['action'] ?? 'index';
         $shortcutButton = $buttonBar->makeShortcutButton()
             ->setModuleName('help_cshmanual')
+            ->setDisplayName($this->getShortcutTitle($request))
             ->setArguments([
                 'route' => $request->getQueryParams()['route'],
                 'action' => $action,
@@ -197,6 +194,34 @@ protected function registerDocheaderButtons(ServerRequestInterface $request)
         }
     }
 
+    protected function getManuals(ServerRequestInterface $request): array
+    {
+        $table = $request->getQueryParams()['table'] ?? $request->getParsedBody()['table'];
+        $field = $request->getQueryParams()['field'] ?? $request->getParsedBody()['field'] ?? '*';
+
+        return $field === '*'
+            ? $this->tableManualRepository->getTableManual($table)
+            : [$this->tableManualRepository->getSingleManual($table, $field)];
+    }
+
+    /**
+     * Returns the shortcut title for the current page
+     *
+     * @param ServerRequestInterface $request
+     * @return string
+     */
+    protected function getShortcutTitle(ServerRequestInterface $request): string
+    {
+        $title = $this->getLanguageService()->sL('LLL:EXT:backend/Resources/Private/Language/locallang_mod_help_cshmanual.xlf:mlang_labels_tablabel');
+        if (($manuals = $this->getManuals($request)) !== []) {
+            $manualTitle = array_shift($manuals)['headerLine'] ?? '';
+            if ($manualTitle !== '') {
+                $title .= ': ' . $manualTitle;
+            }
+        }
+        return $title;
+    }
+
     /**
      * Returns the currently logged in BE user
      *

typo3/sysext/backend/Classes/Controller/PageLayoutController.php

@@ -767,6 +767,7 @@ protected function makeButtons(ServerRequestInterface $request): void
         // Shortcut
         $shortcutButton = $this->buttonBar->makeShortcutButton()
             ->setModuleName($this->moduleName)
+            ->setDisplayName($this->getShortcutTitle())
             ->setArguments([
                 'route' => $request->getQueryParams()['route'],
                 'id' => (int)$this->id,
@@ -1107,4 +1108,19 @@ protected function getSearchBox(): string
             </form>
         </div>';
     }
+
+    /**
+     * Returns the shortcut title for the current page
+     *
+     * @return string
+     */
+    protected function getShortcutTitle(): string
+    {
+        return sprintf(
+            '%s: %s [%d]',
+            $this->getLanguageService()->sL('LLL:EXT:backend/Resources/Private/Language/locallang_mod.xlf:mlang_labels_tablabel'),
+            BackendUtility::getRecordTitle('pages', $this->pageinfo),
+            $this->id
+        );
+    }
 }

typo3/sysext/backend/Classes/Controller/SiteConfigurationController.php

@@ -641,6 +641,7 @@ protected function configureOverViewDocHeader(string $route): void
         $buttonBar->addButton($reloadButton, ButtonBar::BUTTON_POSITION_RIGHT);
         $shortcutButton = $buttonBar->makeShortcutButton()
             ->setModuleName('site_configuration')
+            ->setDisplayName($this->getLanguageService()->sL('LLL:EXT:backend/Resources/Private/Language/locallang_siteconfiguration_module.xlf:mlang_labels_tablabel'))
             ->setArguments([
                 'route' => $route
             ]);

typo3/sysext/backend/Classes/Template/Components/Buttons/Action/ShortcutButton.php

@@ -234,6 +234,9 @@ public function __toString()
     public function render()
     {
         if ($this->getBackendUser()->mayMakeShortcut()) {
+            if ($this->displayName === '') {
+                trigger_error('Creating a shortcut button without a display name is deprecated and fallbacks will be removed in v12. Please use ShortcutButton->setDisplayName() to set a display name.', E_USER_DEPRECATED);
+            }
             if (!empty($this->arguments)) {
                 $shortcutMarkup = $this->createShortcutMarkup();
             } else {

typo3/sysext/beuser/Classes/Controller/PermissionController.php

@@ -175,6 +175,7 @@ protected function registerDocHeaderButtons()
 
         $shortcutButton = $buttonBar->makeShortcutButton()
             ->setModuleName($moduleName)
+            ->setDisplayName($this->getShortcutTitle())
             ->setArguments([
                 'route' => GeneralUtility::_GP('route'),
                 'id' => (int)$this->id,
@@ -402,4 +403,19 @@ protected function getLanguageService()
     {
         return $GLOBALS['LANG'];
     }
+
+    /**
+     * Returns the shortcut title for the current page
+     *
+     * @return string
+     */
+    protected function getShortcutTitle(): string
+    {
+        return sprintf(
+            '%s: %s [%d]',
+            $this->getLanguageService()->sL('LLL:EXT:beuser/Resources/Private/Language/locallang_mod.xlf:mlang_tabs_tab'),
+            BackendUtility::getRecordTitle('pages', $this->pageInfo),
+            $this->id
+        );
+    }
 }

typo3/sysext/core/Documentation/Changelog/master/Deprecation-93060-ShortcutTitleMustBeSetByControllers.rst

@@ -0,0 +1,40 @@
+.. include:: ../../Includes.txt
+
+===============================================================
+Deprecation: #93060 - Shortcut title must be set by controllers
+===============================================================
+
+See :issue:`93060`
+
+Description
+===========
+
+Previously the :php:`ShortcutReporsitory` had automatically generated a
+shortcut title based on the given arguments. However, this generation could
+never be reliable, especially for custom extension code, since the repository
+does not know about controller specific logic. Therefore, this functionality
+has now being deprecated. Backend controllers which add a shortcut button to
+their module header are now being required to also set the desired title.
+
+
+Impact
+======
+
+Adding a new shortcut button without defining the :php:`$displayName` raises a
+deprecation level log entry.
+
+
+Affected Installations
+======================
+
+All installations using the shortcut button API without defining the
+:php:`$displayName` property.
+
+
+Migration
+=========
+
+Define the title with
+:php:`TYPO3\CMS\Backend\Template\Components\Buttons\Action\ShortcutButton->setDisplayName()`.
+
+.. index:: Backend, PHP-API, NotScanned, ext:backend

typo3/sysext/filelist/Classes/Controller/FileListController.php

@@ -664,6 +664,7 @@ protected function registerButtons()
         // Shortcut
         $shortCutButton = $buttonBar->makeShortcutButton()
             ->setModuleName('file_FilelistList')
+            ->setDisplayName($this->getShortcutTitle())
             ->setArguments([
                 'route' => GeneralUtility::_GP('route'),
                 'id' => $this->id,
@@ -851,4 +852,18 @@ protected function initClipboard(): void
         $this->view->assign('showClipBoard', (bool)$this->MOD_SETTINGS['clipBoard']);
         $this->view->assign('clipBoardHtml', $this->filelist->clipObj->printClipboard());
     }
+
+    /**
+     * Returns the shortcut title for the current folder object
+     *
+     * @return string
+     */
+    protected function getShortcutTitle(): string
+    {
+        return sprintf(
+            '%s: %s',
+            $this->getLanguageService()->sL('LLL:EXT:filelist/Resources/Private/Language/locallang_mod_file_list.xlf:mlang_tabs_tab'),
+            $this->folderObject->getName() ?: $this->folderObject->getIdentifier()
+        );
+    }
 }