From 1e36dc458dcbe516968b70549e12b6f53114ef0c Mon Sep 17 00:00:00 2001
From: Stefan Weil <sw@weilnetz.de>
Date: Tue, 1 Oct 2024 07:45:42 +0200
Subject: [PATCH] Improve accessibility of cover images

---
 .../src/VuFind/View/Helper/Root/Record.php    | 27 ++++++++++---------
 .../DefaultRecord/collection-info.phtml       |  2 +-
 .../RecordDriver/DefaultRecord/core.phtml     |  2 +-
 .../RecordDriver/DefaultRecord/cover.phtml    | 15 ++++++-----
 .../templates/RecordDriver/EDS/cover.phtml    | 16 ++++++-----
 5 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/module/VuFind/src/VuFind/View/Helper/Root/Record.php b/module/VuFind/src/VuFind/View/Helper/Root/Record.php
index 2c71299b1cc..bb992434447 100644
--- a/module/VuFind/src/VuFind/View/Helper/Root/Record.php
+++ b/module/VuFind/src/VuFind/View/Helper/Root/Record.php
@@ -538,15 +538,16 @@ public function getCheckbox($idPrefix = '', $formAttr = false, $number = null)
     /**
      * Render a cover for the current record.
      *
-     * @param string $context Context of code being generated
-     * @param string $default The default size of the cover
-     * @param string $link    The link for the anchor
+     * @param string $context    Context of code being generated
+     * @param string $default    The default size of the cover
+     * @param string $link       The link for the anchor
+     * @param bool   $linkUnique Is there no other link with the same href?
      *
      * @return string
      */
-    public function getCover($context, $default, $link = false)
+    public function getCover($context, $default, $link = false, $linkUnique = false)
     {
-        $details = $this->getCoverDetails($context, $default, $link);
+        $details = $this->getCoverDetails($context, $default, $link, $linkUnique);
         return $details['html'];
     }
 
@@ -575,20 +576,22 @@ protected function getPreviewCoverLinkSetting($context)
     /**
      * Get the rendered cover plus some useful parameters.
      *
-     * @param string             $context Context of code being generated
-     * @param string             $default The default size of the cover
-     * @param string|array|false $link    The href link for the anchor (false
-     * for no link, or a string to use as an href, or an array of attributes
-     * to include in the anchor tag)
+     * @param string             $context    Context of code being generated
+     * @param string             $default    The default size of the cover
+     * @param string|array|false $link       The href link for the anchor
+		 * (false for no link, or a string to use as an href, or an array of
+		 * attributes to include in the anchor tag)
+     * @param bool               $linkUnique Is there no other link with the
+		 * same href?
      *
      * @return array
      */
-    public function getCoverDetails($context, $default, $link = false)
+    public function getCoverDetails($context, $default, $link = false, $linkUnique = false)
     {
         $linkAttributes = is_string($link)
             ? ['href' => $link]
             : (is_array($link) ? $link : []);
-        $details = compact('linkAttributes', 'context') + [
+        $details = compact('linkAttributes', 'linkUnique', 'context') + [
             'driver' => $this->driver, 'cover' => false, 'size' => false,
             'linkPreview' => $this->getPreviewCoverLinkSetting($context),
         ];
diff --git a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-info.phtml b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-info.phtml
index 38e332252eb..d50aebf595d 100644
--- a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-info.phtml
+++ b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/collection-info.phtml
@@ -2,7 +2,7 @@
 <div class="media">
   <?php
     $QRCode = $this->record($this->driver)->getQRCode('core');
-    $coverDetails = $this->record($this->driver)->getCoverDetails('collection-info', 'medium', $this->record($this->driver)->getThumbnail('large'));
+    $coverDetails = $this->record($this->driver)->getCoverDetails('collection-info', 'medium', $this->record($this->driver)->getThumbnail('large'), true);
     $cover = $coverDetails['html'];
     $preview = $this->record($this->driver)->getPreviews();
   ?>
diff --git a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml
index d9c5409ee40..b3867e8d1b8 100644
--- a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml
+++ b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml
@@ -6,7 +6,7 @@
     $QRCode = $this->record($this->driver)->getQRCode('core');
     $largeImage = $this->record($this->driver)->getThumbnail('large');
     $linkAttributes = $largeImage ? ['href' => $largeImage, 'data-lightbox-image' => 'true'] : [];
-    $coverDetails = $this->record($this->driver)->getCoverDetails('core', 'medium', $linkAttributes);
+    $coverDetails = $this->record($this->driver)->getCoverDetails('core', 'medium', $linkAttributes, true);
     $cover = $coverDetails['html'];
     $preview = ($this->previewOverride ?? false)
       ? $this->previewOverride : $this->record($this->driver)->getPreviews();
diff --git a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/cover.phtml b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/cover.phtml
index ae92b633150..7df0e07bda2 100644
--- a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/cover.phtml
+++ b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/cover.phtml
@@ -4,20 +4,23 @@
   if ($linkAttributes && !isset($linkAttributes['class'])) {
     $linkAttributes['class'] = 'record-cover-link';
   }
-  $alt = $linkAttributes ? trim($this->driver->tryMethod('getTitle') ?? '') : '';
 ?>
-<?php if ($linkAttributes && !empty($alt)): ?><a<?=$this->htmlAttributes($linkAttributes)?>><?php endif; ?>
+<?php if ($linkAttributes): ?>
+  <a href="<?=$this->escapeHtmlAttr($linkAttributes)?>" class="record-cover-link" <?php if (!$this->linkUnique): ?>tabindex="-1" aria-hidden="true"<?php endif; ?>>
+<?php else: ?>
+  <div aria-hidden="true">
+<?php endif; ?>
 <?php /* Display thumbnail if appropriate: */ ?>
 <?php if ($cover): ?>
-  <img src="<?=$this->escapeHtmlAttr($cover); ?>" <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>class="recordcover" alt="<?=$this->escapeHtmlAttr($alt); ?>">
+  <img src="<?=$this->escapeHtmlAttr($cover); ?>" <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>class="recordcover" alt="<?=$this->transEscAttr('Cover Image') ?>">
 <?php elseif ($cover === false): ?>
-  <img src="<?=$this->url('cover-unavailable')?>" <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>class="recordcover" alt="">
+  <img src="<?=$this->url('cover-unavailable')?>" <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>class="recordcover" alt="<?=$this->transEscAttr('No Cover Image') ?>">
 <?php else: ?>
   <div class="ajaxcover">
     <div class="spinner"><?=$this->icon('spinner') ?> <?=$this->translate('loading_ellipsis')?></div>
     <div class="cover-container">
-      <img <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>data-context="<?=$this->escapeHtmlAttr($this->context)?>" data-recordsource="<?=$this->escapeHtmlAttr($driver->getSourceIdentifier())?>" data-recordid="<?=$this->escapeHtmlAttr($driver->getUniqueID())?>" data-coversize="<?=$this->escapeHtmlAttr($size)?>" class="recordcover ajax" alt="<?=$this->escapeHtmlAttr($alt); ?>">
+      <img <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>data-context="<?=$this->escapeHtmlAttr($this->context)?>" data-recordsource="<?=$this->escapeHtmlAttr($driver->getSourceIdentifier())?>" data-recordid="<?=$this->escapeHtmlAttr($driver->getUniqueID())?>" data-coversize="<?=$this->escapeHtmlAttr($size)?>" class="recordcover ajax" alt="<?=$this->transEscAttr('Cover Image') ?>">
     </div>
   </div>
 <?php endif; ?>
-<?php if ($linkAttributes && !empty($alt)): ?></a><?php endif; ?>
+<?php if ($linkAttributes): ?></a><?php else: ?></div><?php endif; ?>
diff --git a/themes/bootstrap3/templates/RecordDriver/EDS/cover.phtml b/themes/bootstrap3/templates/RecordDriver/EDS/cover.phtml
index 237e159e952..b632c8fa33e 100644
--- a/themes/bootstrap3/templates/RecordDriver/EDS/cover.phtml
+++ b/themes/bootstrap3/templates/RecordDriver/EDS/cover.phtml
@@ -1,11 +1,15 @@
 <?php
-  // set $alt to empty when we have an icon ($cover === false), because we don't want to link the icons (it looks weird)
-  $alt = $this->link && $cover !== false ? trim($this->driver->tryMethod('getTitle') ?? '') : '';
+  // we don't want to link the icons ($cover === false) (it looks weird)
+  $includeLinkInMarkup = $this->link && $cover !== false;
 ?>
-<?php if ($this->link && !empty($alt)): ?><a href="<?=$this->escapeHtmlAttr($this->link)?>" class="record-cover-link"><?php endif; ?>
+<?php if ($includeLinkInMarkup): ?>
+  <a href="<?=$this->escapeHtmlAttr($this->link)?>" class="record-cover-link" <?php if (!$this->linkUnique): ?>tabindex="-1" aria-hidden="true"<?php endif; ?>>
+<?php else: ?>
+  <div aria-hidden="true">
+<?php endif; ?>
 <?php /* Display thumbnail if appropriate: */ ?>
 <?php if ($cover): ?>
-  <img src="<?=$this->escapeHtmlAttr($cover); ?>" <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>class="recordcover" alt="<?=$this->escapeHtmlAttr($alt); ?>">
+  <img src="<?=$this->escapeHtmlAttr($cover); ?>" <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>class="recordcover" alt="<?=$this->transEscAttr('Cover Image') ?>">
 <?php elseif ($cover === false): ?>
   <span class="recordcover pt-icon pt-<?=$this->driver->getPubTypeId()?>"></span>
   <div><?=$this->driver->getPubType()?></div>
@@ -13,8 +17,8 @@
   <div class="ajaxcover">
     <div class="spinner"><?=$this->icon('spinner') ?> <?=$this->translate('loading_ellipsis')?></div>
     <div class="cover-container">
-      <img <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>data-context="<?=$this->escapeHtmlAttr($this->context)?>" data-recordsource="<?=$this->escapeHtmlAttr($driver->getSourceIdentifier())?>" data-recordid="<?=$this->escapeHtmlAttr($driver->getUniqueID())?>" data-coversize="<?=$this->escapeHtmlAttr($size)?>" class="recordcover ajax" alt="<?=$this->escapeHtmlAttr($alt); ?>">
+      <img <?php if ($linkPreview): ?>data-linkpreview="true" <?php endif; ?>data-context="<?=$this->escapeHtmlAttr($this->context)?>" data-recordsource="<?=$this->escapeHtmlAttr($driver->getSourceIdentifier())?>" data-recordid="<?=$this->escapeHtmlAttr($driver->getUniqueID())?>" data-coversize="<?=$this->escapeHtmlAttr($size)?>" class="recordcover ajax" alt="<?=$this->transEscAttr('Cover Image') ?>">
     </div>
   </div>
 <?php endif; ?>
-<?php if ($this->link && !empty($alt)): ?></a><?php endif; ?>
+<?php if ($includeLinkInMarkup): ?></a><?php else: ?></div><?php endif; ?>