Skip to content

Commit

Permalink
[BUGFIX] Prevent unecessary usage of inline javascript in href attrib…
Browse files Browse the repository at this point in the history
…utes

Replace all `<a> ` tags having `href=“javascript:;“` as target
with `<button>` tags of type `button` and appropriate classes.

Resolves: #99917
Releases: main
Change-Id: Ic46df405db3f52cb94b06632b638652ab98d50c6
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/78663
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: core-ci <typo3@b13.com>
  • Loading branch information
NeoBlack authored and georgringer committed Apr 17, 2023
1 parent bcdc124 commit 4d78130
Show file tree
Hide file tree
Showing 14 changed files with 63 additions and 63 deletions.
24 changes: 11 additions & 13 deletions Build/Sources/Sass/component/_resources.scss
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,17 @@
border: 1px solid var(--resource-tile-border-color);
border-radius: var(--resource-tile-border-radius);

a {
a,
button {
position: absolute;
top: 0;
left: 0;
right: 0;
bottom: 0;
display: flex;
flex-direction: column;
text-decoration: none;
outline: none;
color: inherit;
}
}
Expand Down Expand Up @@ -127,18 +137,6 @@
}
}

.resource-tile a {
position: absolute;
top: 0;
left: 0;
right: 0;
bottom: 0;
display: flex;
flex-direction: column;
text-decoration: none;
outline: none;
}

.resource-tile-label {
position: absolute;
width: 1px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2969,9 +2969,9 @@ protected function renderCheckboxActions(): string

return '
<div class="btn-group dropdown">
<a href="javascript:;" class="dropdown-toggle t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<button type="button" class="dropdown-toggle btn btn-link p-0 t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
' . $this->iconFactory->getIcon('actions-selection', Icon::SIZE_SMALL) . '
</a>
</button>
<ul class="dropdown-menu t3js-multi-record-selection-check-actions">
' . implode(PHP_EOL, $dropdownItems) . '
</ul>
Expand Down
3 changes: 1 addition & 2 deletions typo3/sysext/backend/Resources/Public/Css/backend.css
Original file line number Diff line number Diff line change
Expand Up @@ -3765,7 +3765,7 @@ typo3-backend-live-search-result-item-action>* .livesearch-result-item-title .sm
}
}
.resource-tile{position:relative;padding-top:98%;color:var(--resource-tile-color);background-color:var(--resource-tile-bg);border:1px solid var(--resource-tile-border-color);border-radius:var(--resource-tile-border-radius)}
.resource-tile a{color:inherit}
.resource-tile a,.resource-tile button{position:absolute;top:0;left:0;right:0;bottom:0;display:flex;flex-direction:column;text-decoration:none;outline:0;color:inherit}
.resource-tile:hover{--resource-tile-color:var(--resource-tile-hover-color);--resource-tile-bg:var(--resource-tile-hover-bg);--resource-tile-border-color:var(--resource-tile-hover-border-color);text-decoration:none}
.resource-tile:focus-within{--resource-tile-color:var(--resource-tile-focus-color);--resource-tile-bg:var(--resource-tile-focus-bg);--resource-tile-border-color:var(--resource-tile-focus-border-color)}
.resource-tile.active{--resource-tile-color:var(--resource-tile-active-color);--resource-tile-bg:var(--resource-tile-active-bg);--resource-tile-border-color:var(--resource-tile-active-border-color)}
Expand All @@ -3774,7 +3774,6 @@ typo3-backend-live-search-result-item-action>* .livesearch-result-item-title .sm
.resource-tile.danger{--resource-tile-color:var(--resource-tile-danger-color);--resource-tile-bg:var(--resource-tile-danger-bg);--resource-tile-border-color:var(--resource-tile-danger-border-color)}
.resource-tile.warning{--resource-tile-color:var(--resource-tile-warning-color);--resource-tile-bg:var(--resource-tile-warning-bg);--resource-tile-border-color:var(--resource-tile-warning-border-color)}
.resource-tile.active .resource-tile-checkbox,.resource-tile.selected .resource-tile-checkbox,.resource-tile:hover .resource-tile-checkbox{display:block}
.resource-tile a{position:absolute;top:0;left:0;right:0;bottom:0;display:flex;flex-direction:column;text-decoration:none;outline:0}
.resource-tile-label{position:absolute;width:1px;height:1px;padding:0;margin:-1px;overflow:hidden;clip:rect(0,0,0,0);border:0}
.resource-tile-preview{flex:1;position:relative;margin:var(--resource-tile-spacing);margin-bottom:0}
.resource-tile-preview-content{position:absolute;top:0;left:0;height:100%;width:100%}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private function openActionDropdown(ApplicationTester $I, string $title): Remote
static function (RemoteWebDriver $webDriver) use ($title) {
return $webDriver->findElement(
\Facebook\WebDriver\WebDriverBy::xpath(
'//a[contains(text(),"' . $title . '")]/parent::node()/parent::node()//a[contains(@href, "#actions_") and contains(@data-bs-toggle, "dropdown")]'
'//button[contains(text(),"' . $title . '")]/parent::node()/parent::node()//a[contains(@href, "#actions_") and contains(@data-bs-toggle, "dropdown")]'
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public function fileCrud(ApplicationTester $I, ModalDialog $modalDialog): void

// Delete file
$I->amGoingTo('delete the file');
$I->clickWithRightButton('[data-filelist-identifier="1:/' . $fileName . '"] a[data-filelist-action="primary"]');
$I->clickWithRightButton('[data-filelist-identifier="1:/' . $fileName . '"] [data-filelist-action="primary"]');
$I->click('[data-title="Delete"]');
$modalDialog->canSeeDialog();
$modalDialog->clickButtonInDialog('Yes, delete this file');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ public function addingResourceToDefaultLangPageAddResourceToLocalizedPage(Applic
});
// Add an image, closes modal again
$context->findElement(WebDriverBy::cssSelector('text.node-name'))->click();
$I->waitForElementVisible('[data-filelist-name="telephone_box.jpg"] a[data-filelist-action="primary"]');
$I->click('[data-filelist-name="telephone_box.jpg"] a[data-filelist-action="primary"]');
$I->waitForElementVisible('[data-filelist-name="telephone_box.jpg"] [data-filelist-action="primary"]');
$I->click('[data-filelist-name="telephone_box.jpg"] [data-filelist-action="primary"]');
// Save, go back to page
$I->switchToWindow('typo3-backend');
$I->switchToContentFrame();
Expand Down Expand Up @@ -99,8 +99,8 @@ public function addingResourceToDefaultLangPageAddResourceToLocalizedPage(Applic
});
// Add an image, closes modal again
$context->findElement(WebDriverBy::cssSelector('text.node-name'))->click();
$I->waitForElementVisible('[data-filelist-name="underground.jpg"] a[data-filelist-action="primary"]');
$I->click('[data-filelist-name="underground.jpg"] a[data-filelist-action="primary"]');
$I->waitForElementVisible('[data-filelist-name="underground.jpg"] [data-filelist-action="primary"]');
$I->click('[data-filelist-name="underground.jpg"] [data-filelist-action="primary"]');
// Save, go back to page
$I->switchToWindow('typo3-backend');
$I->switchToContentFrame();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ public function addingResourceToDefaultLangPageAddResourceToLocalizedPage(Applic
});
// Add an image, closes modal again
$context->findElement(WebDriverBy::cssSelector('text.node-name'))->click();
$I->waitForElementVisible('[data-filelist-name="telephone_box.jpg"] a[data-filelist-action="primary"]');
$I->click('[data-filelist-name="telephone_box.jpg"] a[data-filelist-action="primary"]');
$I->waitForElementVisible('[data-filelist-name="telephone_box.jpg"] [data-filelist-action="primary"]');
$I->click('[data-filelist-name="telephone_box.jpg"] [data-filelist-action="primary"]');
// Save, go back to list
$I->switchToWindow('typo3-backend');
$I->switchToContentFrame();
Expand Down
53 changes: 28 additions & 25 deletions typo3/sysext/filelist/Classes/FileList.php
Original file line number Diff line number Diff line change
Expand Up @@ -615,10 +615,11 @@ protected function renderName(ResourceView $resourceView): string

$attributes = [];
$attributes['title'] = $resourceView->getName();
$attributes['href'] = 'javascript:;';
$attributes['type'] = 'button';
$attributes['class'] = 'btn btn-link p-0';
$attributes['data-filelist-action'] = 'primary';

$output = '<a ' . GeneralUtility::implodeAttributes($attributes, true) . '>' . $resourceName . '</a>';
$output = '<button ' . GeneralUtility::implodeAttributes($attributes, true) . '>' . $resourceName . '</button>';
if ($resourceView->isMissing()) {
$label = htmlspecialchars($this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:warning.file_missing'));
$output = '<span class="badge badge-danger">' . $label . '</span> ' . $output;
Expand Down Expand Up @@ -1014,10 +1015,9 @@ protected function createControlRename(ResourceView $resourceView): ?ButtonInter
return null;
}

$button = GeneralUtility::makeInstance(LinkButton::class);
$button = GeneralUtility::makeInstance(GenericButton::class);
$button->setTitle($this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:cm.rename'));
$button->setHref('javascript:;');
$button->setDataAttributes(['filelist-action' => 'rename']);
$button->setAttributes(['type' => 'button', 'data-filelist-action' => 'rename']);
$button->setIcon($this->iconFactory->getIcon('actions-edit-rename', Icon::SIZE_SMALL));

return $button;
Expand All @@ -1033,12 +1033,12 @@ protected function createControlDownload(ResourceView $resourceView): ?ButtonInt
return null;
}

$button = GeneralUtility::makeInstance(LinkButton::class);
$button = GeneralUtility::makeInstance(GenericButton::class);
$button->setTitle($this->getLanguageService()->sL('LLL:EXT:filelist/Resources/Private/Language/locallang.xlf:download'));
$button->setHref('javascript:;');
$button->setDataAttributes([
'filelist-action' => 'download',
'filelist-action-url' => $this->uriBuilder->buildUriFromRoute('file_download'),
$button->setAttributes([
'type' => 'button',
'data-filelist-action' => 'download',
'data-filelist-action-url' => $this->uriBuilder->buildUriFromRoute('file_download'),
]);
$button->setIcon($this->iconFactory->getIcon('actions-download', Icon::SIZE_SMALL));

Expand Down Expand Up @@ -1067,10 +1067,12 @@ protected function createControlInfo(ResourceView $resourceView): ?ButtonInterfa
return null;
}

$button = GeneralUtility::makeInstance(LinkButton::class);
$button = GeneralUtility::makeInstance(GenericButton::class);
$button->setTitle($this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:cm.info'));
$button->setHref('javascript:;');
$button->setDataAttributes(['filelist-action' => 'show']);
$button->setAttributes([
'type' => 'button',
'data-filelist-action' => 'show',
]);
$button->setIcon($this->iconFactory->getIcon('actions-document-info', Icon::SIZE_SMALL));

return $button;
Expand Down Expand Up @@ -1100,18 +1102,19 @@ protected function createControlDelete(ResourceView $resourceView): ?ButtonInter
}

$title = $this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:cm.delete');
$button = GeneralUtility::makeInstance(LinkButton::class);
$button = GeneralUtility::makeInstance(GenericButton::class);
$button->setTitle($title);
$button->setHref('javascript:;');
$button->setIcon($this->iconFactory->getIcon('actions-edit-delete', Icon::SIZE_SMALL));
$button->setDataAttributes([
'title' => $title,
'bs-content' => sprintf($this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:mess.delete'), trim($recordInfo)) . $referenceCountText,
'filelist-delete' => 'true',
'filelist-delete-identifier' => $resourceView->getIdentifier(),
'filelist-delete-url' => $this->uriBuilder->buildUriFromRoute('tce_file'),
'filelist-delete-type' => $deleteType,
'filelist-delete-check' => $this->getBackendUser()->jsConfirmation(JsConfirmation::DELETE) ? '1' : '0',
$button->setAttributes([
'type' => 'button',
'data-title' => $title,
'data-bs-content' => sprintf($this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:mess.delete'), trim($recordInfo)) . $referenceCountText,
'data-filelist-action' => 'delete',
'data-filelist-delete' => 'true',
'data-filelist-delete-identifier' => $resourceView->getIdentifier(),
'data-filelist-delete-url' => $this->uriBuilder->buildUriFromRoute('tce_file'),
'data-filelist-delete-type' => $deleteType,
'data-filelist-delete-check' => $this->getBackendUser()->jsConfirmation(JsConfirmation::DELETE) ? '1' : '0',
]);

return $button;
Expand Down Expand Up @@ -1361,9 +1364,9 @@ protected function renderCheckboxActions(): string

return '
<div class="btn-group dropdown">
<a href="javascript:;" class="dropdown-toggle t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<button type="button" class="dropdown-toggle btn btn-link p-0 t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
' . $this->iconFactory->getIcon('actions-selection', Icon::SIZE_SMALL) . '
</a>
</button>
<ul class="dropdown-menu t3js-multi-record-selection-check-actions">
' . implode(PHP_EOL, $dropdownItems) . '
</ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
data-multi-record-selection-element="true"
draggable="{resource.canMove ? 'true' : 'false'}"
>
<a title="{resource.name}" href="javascript:;" data-filelist-action="primary">
<button type="button" title="{resource.name}" class="btn p-0" data-filelist-action="primary">
<span class="resource-tile-label" id="resource-tile-label-{resource.uid}">{resource.name}</span>
<span class="resource-tile-preview" role="presentation">
<span class="resource-tile-preview-content">
Expand Down Expand Up @@ -86,7 +86,7 @@
<span class="resource-tile-nameplate-label">{resource.name}</span>
<span class="resource-tile-nameplate-activity">{resource.updatedAt -> f:format.date()}</span>
</span>
</a>
</button>
<f:if condition="{resource.isSelectable} && {resource.checkboxConfig} && {displayCheckbox}">
<span class="resource-tile-checkbox" role="checkbox" aria-label="{resource.name}">
<div class="form-check">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ <h1><f:translate key="LLL:EXT:reactions/Resources/Private/Language/locallang_mod
<tr>
<th>
<div class="btn-group dropdown">
<a href="javascript:;" class="dropdown-toggle t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<button type="button" class="dropdown-toggle btn btn-link p-0 t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<core:icon identifier="actions-selection" size="small" />
</a>
</button>
<ul class="dropdown-menu t3js-multi-record-selection-check-actions">
<li>
<button type="button" class="dropdown-item disabled" data-multi-record-selection-check-action="check-all" title="{f:translate(key: 'LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.checkAll')}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ <h1><f:translate key="LLL:EXT:recycler/Resources/Private/Language/locallang.xlf:

<f:section name="multiRecordSelectionCheckboxActions">
<div class="btn-group dropdown">
<a href="javascript:;" class="dropdown-toggle t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<button type="button" class="dropdown-toggle btn btn-link p-0 t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<core:icon identifier="actions-selection" size="small" />
</a>
</button>
<ul class="dropdown-menu t3js-multi-record-selection-check-actions">
<li>
<button type="button" class="dropdown-item disabled" data-multi-record-selection-check-action="check-all" title="{f:translate(key: 'LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.checkAll')}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,14 @@
<tr class="taskGroup_{taskGroup.groupUid}">
<th class="col-selector nowrap" data-sort-method="none">
<div class="btn-group dropdown">
<a href="javascript:;"
class="dropdown-toggle t3js-multi-record-selection-check-actions-toggle"
<button type="button"
class="dropdown-toggle btn btn-link p-0 t3js-multi-record-selection-check-actions-toggle"
data-bs-toggle="dropdown"
data-bs-boundary="window"
aria-expanded="false"
>
<core:icon identifier="actions-selection" size="small" />
</a>
</button>
<ul class="dropdown-menu t3js-multi-record-selection-check-actions">
<li>
<button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,9 @@ <h1><f:translate key="LLL:EXT:webhooks/Resources/Private/Language/locallang_modu

<f:section name="multiRecordSelectionCheckboxActions">
<div class="btn-group dropdown">
<a href="javascript:;" class="dropdown-toggle t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<button type="button" class="dropdown-toggle btn btn-link p-0 t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<core:icon identifier="actions-selection" size="small" />
</a>
</button>
<ul class="dropdown-menu t3js-multi-record-selection-check-actions">
<li>
<button type="button" class="dropdown-item disabled" data-multi-record-selection-check-action="check-all" title="{f:translate(key: 'LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.checkAll')}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@
<tr>
<th>
<div class="btn-group dropdown">
<a href="javascript:;" class="dropdown-toggle t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<button type="button" class="dropdown-toggle btn btn-link p-0 t3js-multi-record-selection-check-actions-toggle" data-bs-toggle="dropdown" data-bs-boundary="window" aria-expanded="false">
<core:icon identifier="actions-selection" size="small" />
</a>
</button>
<ul class="dropdown-menu t3js-multi-record-selection-check-actions">
<li>
<button type="button" class="dropdown-item disabled" data-multi-record-selection-check-action="check-all" title="{f:translate(key: 'LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.checkAll')}">
Expand Down

0 comments on commit 4d78130

Please sign in to comment.