[TASK] Deprecate AbstractControl::setOnClick for backend user interface

In favor of allowing content-security-policy headers (some day), inline JavaScript invocation via PHP in `AbstractControl::setOnClick` has been deprecated. Existing instructions can be migrated using existing JavaScript helpers `GlobalEventHandler` or `ActionDispatcher`. Resolves: #91814 Releases: master Change-Id: I6f599b225fc814cd614f246647ba61f5f3aa3ec5 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/71416 Reviewed-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Helmut Hummel <typo3@helhum.io> Tested-by: core-ci <typo3@b13.com> Tested-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Helmut Hummel <typo3@helhum.io>
TYPO3 · Oct 2, 2021 · ba6e58f · ba6e58f
1 parent 8d8e074
commit ba6e58f
Show file tree

Hide file tree

Showing 5 changed files with 194 additions and 3 deletions.
diff --git a/typo3/sysext/backend/Classes/Template/Components/AbstractControl.php b/typo3/sysext/backend/Classes/Template/Components/AbstractControl.php
@@ -47,6 +47,7 @@ class AbstractControl
      * Outdated, use sparingly
      *
      * @var string
+     * @deprecated Use HTML data attrs for GlobalEventHandler or ActionDispatcher instead. Will be removed in TYPO3 v12.0
      */
     protected $onClick = '';
 
@@ -80,13 +81,31 @@ public function getDataAttributes()
         return $this->dataAttributes;
     }
 
+    /**
+     * Helper method to avoid using `getOnClick` in TYPO3 core.
+     *
+     * @return bool
+     * @deprecated Introduced with TYPO3 v11.5, will be removed with TYPO3 v12.0
+     * @internal Basically just to be used by the TYPO3 core, not to be used in extensions.
+     */
+    public function hasOnClick(): bool
+    {
+        // does not trigger deprecation error on purpose
+        return $this->onClick !== '';
+    }
+
     /**
      * Get Onclick Attribute
      *
      * @return string
+     * @deprecated Use HTML data attrs for GlobalEventHandler or ActionDispatcher instead. Will be removed in TYPO3 v12.0
      */
     public function getOnClick()
     {
+        trigger_error(
+            'Use HTML data attrs for GlobalEventHandler or ActionDispatcher instead. Will be removed in TYPO3 v12.0',
+            E_USER_DEPRECATED
+        );
         return $this->onClick;
     }
 
@@ -135,9 +154,14 @@ public function setDataAttributes(array $dataAttributes)
      * @param string $onClick HTML onClick attribute to set
      *
      * @return $this
+     * @deprecated Use HTML data attrs for GlobalEventHandler or ActionDispatcher instead. Will be removed in TYPO3 v12.0
      */
     public function setOnClick($onClick)
     {
+        trigger_error(
+            'Use HTML data attrs for GlobalEventHandler or ActionDispatcher instead. Will be removed in TYPO3 v12.0',
+            E_USER_DEPRECATED
+        );
         $this->onClick = $onClick;
         return $this;
     }

diff --git a/typo3/sysext/backend/Classes/Template/Components/Buttons/LinkButton.php b/typo3/sysext/backend/Classes/Template/Components/Buttons/LinkButton.php
@@ -102,8 +102,8 @@ public function render()
         foreach ($this->dataAttributes as $attributeName => $attributeValue) {
             $attributes['data-' . $attributeName] = $attributeValue;
         }
-        if ($this->onClick !== '') {
-            $attributes['onclick'] = $this->onClick;
+        if ($this->hasOnClick()) {
+            $attributes['onclick'] = $this->getOnClick();
         }
         if ($this->isDisabled()) {
             $attributes['disabled'] = 'disabled';

diff --git a/typo3/sysext/backend/Classes/Template/Components/Buttons/SplitButton.php b/typo3/sysext/backend/Classes/Template/Components/Buttons/SplitButton.php
@@ -178,7 +178,7 @@ public function render()
                 if (!empty($option->getClasses())) {
                     $optionAttributes['class'] = $option->getClasses();
                 }
-                if (!empty($option->getOnClick())) {
+                if ($option->hasOnClick()) {
                     $optionAttributes['onclick'] = $option->getOnClick();
                 }
                 $optionAttributes['class'] = implode(' ', [$optionAttributes['class'] ?? '', 'dropdown-item']);

diff --git a/...ation/Changelog/master/Deprecation-91814-DeprecateAbstractControlsetOnClick.rst b/...ation/Changelog/master/Deprecation-91814-DeprecateAbstractControlsetOnClick.rst
@@ -0,0 +1,153 @@
+.. include:: ../../Includes.txt
+
+===========================================================
+Deprecation: #91814 - Deprecate AbstractControl::setOnClick
+===========================================================
+
+See :issue:`91814`
+
+Description
+===========
+
+In favor of allowing `Content-Security-Policy` HTTP headers, inline JavaScript
+invocation via :php:`\TYPO3\CMS\Backend\Template\Components\AbstractControl::setOnClick`
+has been deprecated. Existing instructions can be migrated using existing JavaScript
+helpers `GlobalEventHandler` or `ActionDispatcher` and their capabilities to provide
+similar functionality using :html:`data-` attributes.
+
+There might be scenarios that require a custom JavaScript module handling
+specific use cases that are not covered by mentioned JavaScript helpers.
+
+
+Impact
+======
+
+Using affected PHP methods (see section below) will trigger PHP deprecation errors.
+
+
+Affected Installations
+======================
+
+All sites using 3rd party extensions that are using following methods directly
+or in inherited class implementations:
+
+* :php:`\TYPO3\CMS\Backend\Template\Components\AbstractControl->setOnClick`
+* :php:`\TYPO3\CMS\Backend\Template\Components\AbstractControl->getOnClick`
+
+
+Migration
+=========
+
+Mentioned JavaScript helpers cover most common use cases by using :html:`data-`
+attributes instead of :html:`onclick` event attributes with corresponding HTML
+elements.
+
+* consider replacing simple :html:`<a ... onclick="window.location.href=[URI]"`
+  with plain HTML links like :html:`<a href="[URI]">`
+* replacing :php:`BackendUtility::viewOnClick`,
+  `see documentation & examples <https://docs.typo3.org/c/typo3/cms-core/master/en-us/Changelog/11.0/Important-91123-AvoidUsingBackendUtilityViewOnClick.html>`__
+* using :html:`data-` attributes for :js:`GlobalEventHandler` and :js:`ActionDispatcher`,
+  `see documentation & examples <https://docs.typo3.org/c/typo3/cms-core/master/en-us/Changelog/10.4.x/Important-91117-UseGlobalEventHandlerAndActionDispatcherInsteadOfInlineJS.html>`__
+
+
+Example #1: open a new window/tab
+---------------------------------
+
+* taken form extension `dce`
+* see `corresponding pull-request <https://bitbucket.org/ArminVieweg/dce/pull-requests/97/task-avoid-using-abstractcontrol>`__
+
+.. code-block:: php
+
+    $button->setOnClick(
+        'window.open(\'' . $this->getDceEditLink($contentUid) . '\', \'editDcePopup\', ' .
+        '\'height=768,width=1024,status=0,menubar=0,scrollbars=1\')'
+    );
+
+Code block above being substituted with :js:`ActionDispatcher` capabilities,
+using :html`data-dispatch-action` and :html:`data-dispatch-args` HTML attributes:
+
+.. code-block:: php
+
+    $button->setDataAttributes([
+        'dispatch-action' => 'TYPO3.WindowManager.localOpen',
+        // JSON encoded representation of JavaScript function arguments
+        // (HTML attributes are encoded in \TYPO3\CMS\Backend\Template\Components\Buttons\LinkButton)
+        'dispatch-args' => GeneralUtility::jsonEncodeForHtmlAttribute([
+            $this->getDceEditLink($contentUid),
+            'editDcePopup',
+            'height=768,width=1024,status=0,menubar=0,scrollbars=1',
+        ], false)
+    ]);
+
+
+Example #2: preview page in frontend
+------------------------------------
+
+* taken from extension `wizard_crpagetree`
+* see `corresponding pull-request <https://github.com/liayn/t3ext-wizard_crpagetree/pull/8>`__
+
+.. code-block:: php
+
+    $viewButton = $buttonBar->makeLinkButton()
+        // @deprecated setOnClick
+        ->setOnClick(BackendUtility::viewOnClick($pageUid, '', BackendUtility::BEgetRootLine($pageUid)))
+        ->setTitle($this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.showPage'))
+        ->setIcon($iconFactory->getIcon('actions-view-page', Icon::SIZE_SMALL))
+        ->setHref('#');
+
+Code block above being substituted with :php:`\TYPO3\CMS\Backend\Routing\PreviewUriBuilder`
+based on :js:`ActionDispatcher` capabilities, using :html`data-dispatch-action` and
+:html:`data-dispatch-args` HTML attributes:
+
+.. code-block:: php
+
+    $previewDataAttributes = PreviewUriBuilder::create($pageUid)
+        ->withRootLine(BackendUtility::BEgetRootLine($pageUid))
+        ->buildDispatcherDataAttributes();
+    $viewButton = $buttonBar->makeLinkButton()
+        // substituted with HTML data attributes
+        ->setDataAttributes($previewDataAttributes ?? [])
+        ->setTitle($this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.showPage'))
+        ->setIcon($iconFactory->getIcon('actions-view-page', Icon::SIZE_SMALL))
+        ->setHref('#');
+
+
+Example #3: confirmation dialog
+-------------------------------
+
+* taken form extension `news`
+* see `corresponding pull-request <https://github.com/georgringer/news/pull/1585>`__
+* side-note: There was a bug in extension `news`, examples below have been adjusted
+  to show how the scenario probably would have been before, using :js:`confirm()`
+
+.. code-block:: php
+
+    $pasteTitle = 'Paste from Clipboard';
+    $confirmMessage = GeneralUtility::quoteJSvalue('Shall we paste the record?');
+    $viewButton = $buttonBar->makeLinkButton()
+        ->setHref($clipBoard->pasteUrl('', $this->pageUid))
+        // @deprecated inline JavaScript requesting user confirmation
+        ->setOnClick('return confirm(' . $confirmMessage . ')')
+        ->setTitle($pasteTitle)
+        ->setIcon($this->iconFactory->getIcon('actions-document-paste-into', Icon::SIZE_SMALL));
+
+Code block above being substituted with capabilities of modal dialog handling
+and functionalities of the Bootstrap framework.
+
+.. code-block:: php
+
+    $pasteTitle = 'Paste from Clipboard';
+    $confirmMessage = 'Shall we paste the record?';
+    $viewButton = $buttonBar->makeLinkButton()
+        ->setHref($clipBoard->pasteUrl('', $this->pageUid))
+        // using CSS class to trigger confirmation in modal box
+        ->setClasses('t3js-modal-trigger')
+        ->setDataAttributes([
+            'title' => $pasteTitle,
+            'bs-content' => $confirmMessage,
+        ])
+        ->setTitle($pasteTitle)
+        ->setIcon($this->iconFactory->getIcon('actions-document-paste-into', Icon::SIZE_SMALL));
+
+
+.. index:: Backend, JavaScript, PHP-API, FullyScanned, ext:backend
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
@@ -5019,4 +5019,18 @@
             'Deprecation-95275-RelationHandler-remapMM.rst',
         ],
     ],
+    'TYPO3\CMS\Backend\Template\Components\AbstractControl->setOnClick' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-91814-DeprecateAbstractControlsetOnClick.rst',
+        ],
+    ],
+    'TYPO3\CMS\Backend\Template\Components\AbstractControl->getOnClick' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Deprecation-91814-DeprecateAbstractControlsetOnClick.rst',
+        ],
+    ],
 ];