Skip to content

Commit

Permalink
[BUGFIX] Have record icons with human readable title
Browse files Browse the repository at this point in the history
Add record titles to the record icon alt text. To make
the alt text for record icons helpful, the title and
type of the record is now added to the existing id=x.

To implement this also for the pagetree, the method
BackendUtility::titleAttribForPages() has got a new
parameter to return the value unescaped.
The JsonResponse in the TreeController already
escapes the data and additionally the TemplateResult
from lit also does escaping.

Additionally BackendUtility::getRecordIconAltText() received
a new parameter to make it possible to not escape string values.
This is necessary to prevent values to get double encoded when
used in the fluid context.

Resolves: #102472
Releases: main, 12.4
Change-Id: I2476baccc4caf1ffaf27bbb3d5681cd53aea6052
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/81883
Reviewed-by: Jasmina Ließmann <minapokhalo+typo3@gmail.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: Andreas Kienast <a.fernandez@scripting-base.de>
Reviewed-by: Andreas Kienast <a.fernandez@scripting-base.de>
Tested-by: Jasmina Ließmann <minapokhalo+typo3@gmail.com>
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
  • Loading branch information
Christian Rath-Ulrich authored and minapok committed Apr 19, 2024
1 parent 913faf6 commit 162111f
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 51 deletions.
Expand Up @@ -88,7 +88,7 @@ private function getContentVariables(ServerRequestInterface $request): array
$elRow = BackendUtility::getRecordWSOL('tt_content', $this->moveUid);
// Headerline: Icon, record title:
$assigns['record'] = $elRow;
$assigns['recordTooltip'] = BackendUtility::getRecordIconAltText($elRow, 'tt_content');
$assigns['recordTooltip'] = BackendUtility::getRecordIconAltText($elRow, 'tt_content', false);
$assigns['recordTitle'] = BackendUtility::getRecordTitle('tt_content', $elRow, true);
// Make-copy checkbox (clicking this will reload the page with the GET var makeCopy set differently):
$assigns['makeCopyChecked'] = (bool)$this->makeCopy;
Expand All @@ -115,7 +115,7 @@ private function getContentVariables(ServerRequestInterface $request): array
$contentPositionMap->cur_sys_language = $this->sys_language;
$contentPositionMap->R_URI = $this->R_URI;
// Headerline for the parent page: Icon, record title:
$assigns['ttContent']['recordTooltip'] = BackendUtility::getRecordIconAltText($pageInfo, 'pages');
$assigns['ttContent']['recordTooltip'] = BackendUtility::getRecordIconAltText($pageInfo, 'pages', false);
$assigns['ttContent']['recordTitle'] = BackendUtility::getRecordTitle('pages', $pageInfo, true);
// Adding parent page-header and the content element columns from position-map:
$assigns['contentElementColumns'] = $contentPositionMap->printContentElementColumns($this->page_id);
Expand Down
Expand Up @@ -115,13 +115,13 @@ private function getContentVariables(int $pageIdToMove, int $targetPid): array
'targetHasSubpages' => $this->pageHasSubpages($targetPid),
'element' => [
'record' => $elementRow,
'recordTooltip' => BackendUtility::getRecordIconAltText($elementRow, 'pages'),
'recordTooltip' => BackendUtility::getRecordIconAltText($elementRow, 'pages', false),
'recordTitle' => BackendUtility::getRecordTitle('pages', $elementRow),
'recordPath' => BackendUtility::getRecordPath($pageIdToMove, $this->getBackendUser()->getPagePermsClause(Permission::PAGE_SHOW), 0),
],
'target' => [
'record' => $targetRow,
'recordTooltip' => BackendUtility::getRecordIconAltText($targetRow, 'pages'),
'recordTooltip' => BackendUtility::getRecordIconAltText($targetRow, 'pages', false),
'recordTitle' => BackendUtility::getRecordTitle('pages', $targetRow),
'recordPath' => BackendUtility::getRecordPath($targetPid, $this->getBackendUser()->getPagePermsClause(Permission::PAGE_SHOW), 0),
],
Expand Down
Expand Up @@ -219,7 +219,7 @@ protected function renderFileHeader(string $ariaAttributesString): string
. '-' . self::FILE_REFERENCE_TABLE
. '-' . ($databaseRow['uid'] ?? 0);

$altText = BackendUtility::getRecordIconAltText($databaseRow, self::FILE_REFERENCE_TABLE);
$altText = BackendUtility::getRecordIconAltText($databaseRow, self::FILE_REFERENCE_TABLE, false);

// Renders a thumbnail for the header
$thumbnail = '';
Expand Down Expand Up @@ -252,7 +252,7 @@ protected function renderFileHeader(string $ariaAttributesString): string
'width="' . $processedImage->getProperty('width') . '" ' .
'height="' . $processedImage->getProperty('height') . '" ' .
'alt="" ' .
'title="' . $altText . '" ' .
'title="' . htmlspecialchars($altText) . '" ' .
'loading="lazy">';
}
}
Expand Down
Expand Up @@ -321,7 +321,7 @@ protected function renderForeignRecordHeader(array $data, string $ariaAttributes
return '
<button class="form-irre-header-cell form-irre-header-button" ' . $ariaAttributesString . '>
<div class="form-irre-header-icon" id="' . $objectId . '_iconcontainer">
' . $this->iconFactory->getIconForRecord($foreignTable, $record, IconSize::SMALL)->setTitle(BackendUtility::getRecordIconAltText($record, $foreignTable))->render() . '
' . $this->iconFactory->getIconForRecord($foreignTable, $record, IconSize::SMALL)->setTitle(BackendUtility::getRecordIconAltText($record, $foreignTable, false))->render() . '
</div>
<div class="form-irre-header-body"><span id="' . $objectId . '_label">' . $recordTitle . '</span></div>
</button>
Expand Down
Expand Up @@ -1139,7 +1139,7 @@ public function renderListRow($table, array $row, int $indent, array $translatio
} elseif ($fCol === 'icon') {
$icon = $this->iconFactory
->getIconForRecord($table, $row, IconSize::SMALL)
->setTitle(BackendUtility::getRecordIconAltText($row, $table))
->setTitle(BackendUtility::getRecordIconAltText($row, $table, false))
->render();
$theData[$fCol] = ''
. ($indent ? '<span class="indent indent-inline-block" style="--indent-level: ' . $indent . '"></span> ' : '')
Expand Down
Expand Up @@ -233,7 +233,7 @@ protected function getRecordInformations()
// If there IS a real page
$theIcon = $iconFactory
->getIconForRecord('pages', $pageRecord, IconSize::SMALL)
->setTitle(BackendUtility::getRecordIconAltText($pageRecord))
->setTitle(BackendUtility::getRecordIconAltText($pageRecord, 'pages', false))
->render();
// Make Icon:
$theIcon = BackendUtility::wrapClickMenuOnIcon($theIcon, 'pages', $pageRecord['uid']);
Expand Down
2 changes: 1 addition & 1 deletion typo3/sysext/backend/Classes/Tree/View/PagePositionMap.php
Expand Up @@ -181,7 +181,7 @@ public function positionTree($id, $pageinfo, $perms_clause, $R_URI, ServerReques
// The line with the icon and title:
$icon = $this->iconFactory
->getIconForRecord('pages', $dat['row'], IconSize::SMALL)
->setTitle(BackendUtility::getRecordIconAltText($dat['row'], 'pages'))
->setTitle(BackendUtility::getRecordIconAltText($dat['row'], 'pages', false))
->render();

$lines[] = '<span class="text-nowrap">' . $icon . ' ' .
Expand Down
104 changes: 67 additions & 37 deletions typo3/sysext/backend/Classes/Utility/BackendUtility.php
Expand Up @@ -1179,8 +1179,8 @@ public static function titleAttribForPages($row, $perms_clause = '', $includeAtt
$parts[] = $row['title'];
}
if ($row['uid'] === 0) {
$out = htmlspecialchars(implode(' - ', $parts));
return $includeAttrib ? 'title="' . $out . '"' : $out;
$out = implode(' - ', $parts);
return $includeAttrib ? 'title="' . htmlspecialchars($out) . '"' : $out;
}
switch (VersionState::tryFrom($row['t3ver_state'] ?? 0)) {
case VersionState::DELETE_PLACEHOLDER:
Expand Down Expand Up @@ -1253,8 +1253,8 @@ public static function titleAttribForPages($row, $perms_clause = '', $includeAtt
$label = implode(', ', $fe_groups);
$parts[] = $lang->sL($GLOBALS['TCA']['pages']['columns']['fe_group']['label']) . ' ' . $label;
}
$out = htmlspecialchars(implode(' - ', $parts));
return $includeAttrib ? 'title="' . $out . '"' : $out;
$out = implode(' - ', $parts);
return $includeAttrib ? 'title="' . htmlspecialchars($out) . '"' : $out;
}

/**
Expand All @@ -1264,45 +1264,68 @@ public static function titleAttribForPages($row, $perms_clause = '', $includeAtt
*
* @param array $row Table row; $row is a row from the table, $table
* @param string $table Table name
* @return string
* @param bool $escapeResult If $escapeResult is set, then the return value is escaped with htmlspecialchars()
*/
public static function getRecordIconAltText($row, $table = 'pages')
public static function getRecordIconAltText($row, $table = 'pages', bool $escapeResult = true): string
{
if ($table === 'pages') {
$out = self::titleAttribForPages($row, '', false);
} else {
$out = !empty(trim($GLOBALS['TCA'][$table]['ctrl']['descriptionColumn'] ?? ''))
? ($row[$GLOBALS['TCA'][$table]['ctrl']['descriptionColumn']] ?? '') . ' '
: '';
$ctrl = $GLOBALS['TCA'][$table]['ctrl']['enablecolumns'] ?? [];
// Uid is added
$out .= 'id=' . ($row['uid'] ?? 0);
if (static::isTableWorkspaceEnabled($table)) {
switch (VersionState::tryFrom($row['t3ver_state'] ?? 0)) {
case VersionState::DELETE_PLACEHOLDER:
$out .= ' - Deleted element!';
break;
case VersionState::MOVE_POINTER:
$out .= ' - NEW LOCATION (Move-to Pointer) WSID#' . $row['t3ver_wsid'];
break;
case VersionState::NEW_PLACEHOLDER:
$out .= ' - New element!';
break;
}
}
// Hidden
$lang = static::getLanguageService();
if ($ctrl['disabled'] ?? false) {
$out .= ($row[$ctrl['disabled']] ?? false) ? ' - ' . $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.hidden') : '';
$title = self::titleAttribForPages($row, '', false);
return $escapeResult ? htmlspecialchars($title) : $title;
}

$languageService = static::getLanguageService();
$ctrl = $GLOBALS['TCA'][$table]['ctrl'] ?? [];

$parts = ['id=' . ($row['uid'] ?? '0')];
if (!empty(trim($ctrl['descriptionColumn'] ?? '')) && !empty(trim($row[$ctrl['descriptionColumn']] ?? ''))) {
$parts[] = trim($row[$ctrl['descriptionColumn']] ?? '');
}
$recordTitle = self::getRecordTitle($table, $row);
if (!empty($recordTitle)) {
$parts[] = $recordTitle;
}

if (isset($ctrl['type'])) {
// @todo: We need to ensure that only raw rows gets passed here
// It can happen that we recieve already processed data from FormEngine.
// In this case, the row can contain arrays for the types.
$labelKey = $row[$ctrl['type']] ?? '';
if (is_array($labelKey)) {
$labelKey = (string)array_shift($labelKey);
}
if (($ctrl['starttime'] ?? false) && ($row[$ctrl['starttime']] ?? 0) > $GLOBALS['EXEC_TIME']) {
$out .= ' - ' . $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.starttime') . ':' . self::date($row[$ctrl['starttime']]) . ' (' . self::daysUntil($row[$ctrl['starttime']]) . ' ' . $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.days') . ')';
$recordType = self::getLabelFromItemlist($table, $ctrl['type'], $labelKey, $row);
if (!empty($recordType)) {
$parts[] = $languageService->sL($recordType);
}
if (($ctrl['endtime'] ?? false) && ($row[$ctrl['endtime']] ?? false)) {
$out .= ' - ' . $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.endtime') . ': ' . self::date($row[$ctrl['endtime']]) . ' (' . self::daysUntil($row[$ctrl['endtime']]) . ' ' . $lang->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.days') . ')';
}

if (static::isTableWorkspaceEnabled($table)) {
switch (VersionState::tryFrom($row['t3ver_state'] ?? 0)) {
case VersionState::DELETE_PLACEHOLDER:
$parts[] = 'Deleted element!';
break;
case VersionState::MOVE_POINTER:
$parts[] = 'NEW LOCATION (Move-to Pointer) WSID#' . ($row['t3ver_wsid'] ?? 0);
break;
case VersionState::NEW_PLACEHOLDER:
$parts[] = 'New element!';
break;
}
}
return htmlspecialchars($out);
if (($ctrl['enablecolumns']['disabled'] ?? false) && ($row[$ctrl['enablecolumns']['disabled']] ?? false)) {
$parts[] = $languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.hidden');
}
if (($ctrl['enablecolumns']['starttime'] ?? false) && ($row[$ctrl['enablecolumns']['starttime']] ?? 0) > $GLOBALS['EXEC_TIME']) {
$parts[] = $languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.starttime')
. ': ' . self::date($row[$ctrl['enablecolumns']['starttime']])
. ' (' . self::daysUntil($row[$ctrl['enablecolumns']['starttime']]) . ' ' . $languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.days') . ')';
}
if (($ctrl['enablecolumns']['endtime'] ?? false) && ($row[$ctrl['enablecolumns']['endtime']] ?? false)) {
$parts[] = $languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.endtime')
. ': ' . self::date($row[$ctrl['enablecolumns']['endtime']])
. ' (' . self::daysUntil($row[$ctrl['enablecolumns']['endtime']]) . ' ' . $languageService->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:labels.days') . ')';
}
return $escapeResult ? htmlspecialchars(implode(' - ', $parts)) : implode(' - ', $parts);
}

/**
Expand Down Expand Up @@ -1482,10 +1505,17 @@ public static function getRecordTitle($table, $row, $prep = false, $forceResult
} else {
// No userFunc: Build label
$ctrlLabel = $GLOBALS['TCA'][$table]['ctrl']['label'] ?? '';
$ctrlLabelValue = $row[$ctrlLabel] ?? '';
// $row might be a processed row generated by FormResultCompiler
// => bail out if the ctrlLabel field has been processed into an array
// (e.g. in sys_file_reference.uid_local)
if (is_array($ctrlLabelValue)) {
$ctrlLabelValue = '';
}
$recordTitle = self::getProcessedValue(
$table,
$ctrlLabel,
(string)($row[$ctrlLabel] ?? ''),
(string)$ctrlLabelValue,
0,
false,
false,
Expand Down
Expand Up @@ -181,7 +181,7 @@ public function getIcons(): string

$icon = $this->iconFactory
->getIconForRecord($this->table, $row, IconSize::SMALL)
->setTitle(BackendUtility::getRecordIconAltText($row, $this->table))
->setTitle(BackendUtility::getRecordIconAltText($row, $this->table, false))
->render();
if ($this->getBackendUser()->recordEditAccessInternals($this->table, $row)) {
$icon = BackendUtility::wrapClickMenuOnIcon($icon, $this->table, $row['uid']);
Expand Down
Expand Up @@ -10,7 +10,7 @@

<h1><f:translate key="LLL:EXT:core/Resources/Private/Language/locallang_misc.xlf:movingElement" /></h1>
<h3 class="mb-2">
<span title="{recordTooltip -> f:format.raw()}"><core:iconForRecord table="tt_content" row="{record}" /></span>
<span title="{recordTooltip}"><core:iconForRecord table="tt_content" row="{record}" /></span>
{recordTitle -> f:format.raw()}
</h3>
<div class="pt-2">
Expand All @@ -23,7 +23,7 @@ <h3 class="mb-2">
<h2><f:translate key="LLL:EXT:core/Resources/Private/Language/locallang_misc.xlf:selectPositionOfElement" /></h2>
<div>
<f:if condition="{pageInfo}">
<span title="{ttContent.recordTooltip -> f:format.raw()}"><core:iconForRecord table="pages" row="{pageInfo}" /></span>
<span title="{ttContent.recordTooltip}"><core:iconForRecord table="pages" row="{pageInfo}" /></span>
</f:if>
{ttContent.recordTitle}<br />
{contentElementColumns -> f:format.raw()}<br />
Expand Down
Expand Up @@ -36,7 +36,7 @@ <h2>
<f:then>
<div class="page-position-target">
<div class="page-position-target-abstract">
<span title="{target.recordTooltip -> f:format.raw()}"><core:iconForRecord table="pages" row="{target.record}" /></span>
<span title="{target.recordTooltip}"><core:iconForRecord table="pages" row="{target.record}" /></span>
<strong>{target.recordTitle} [{target.record.uid}]</strong>
</div>
<div class="page-position-target-meta">
Expand Down

0 comments on commit 162111f

Please sign in to comment.