From 2d80409fb1fe0425cfd6126665f96f138d201465 Mon Sep 17 00:00:00 2001
From: Tobias Bengfort <bengfort@mpib-berlin.mpg.de>
Date: Mon, 25 Mar 2024 14:11:00 +0100
Subject: [PATCH] Improve accessibility of cover images

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

diff --git a/module/VuFind/src/VuFind/View/Helper/Root/Record.php b/module/VuFind/src/VuFind/View/Helper/Root/Record.php
index ca257e87a9c..bc21f20b0fe 100644
--- a/module/VuFind/src/VuFind/View/Helper/Root/Record.php
+++ b/module/VuFind/src/VuFind/View/Helper/Root/Record.php
@@ -435,15 +435,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'];
     }
 
@@ -472,15 +473,16 @@ 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 $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 array
      */
-    public function getCoverDetails($context, $default, $link = false)
+    public function getCoverDetails($context, $default, $link = false, $linkUnique = false)
     {
-        $details = compact('link', 'context') + [
+        $details = compact('link', '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 2531f054611..e53863a0821 100644
--- a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml
+++ b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/core.phtml
@@ -4,7 +4,7 @@
 <div class="media" vocab="http://schema.org/" resource="#record" typeof="<?=$this->driver->getSchemaOrgFormats()?> Product">
   <?php
     $QRCode = $this->record($this->driver)->getQRCode('core');
-    $coverDetails = $this->record($this->driver)->getCoverDetails('core', 'medium', $this->record($this->driver)->getThumbnail('large'));
+    $coverDetails = $this->record($this->driver)->getCoverDetails('core', 'medium', $this->record($this->driver)->getThumbnail('large'), 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 14171149c6d..90ea49d4d5d 100644
--- a/themes/bootstrap3/templates/RecordDriver/DefaultRecord/cover.phtml
+++ b/themes/bootstrap3/templates/RecordDriver/DefaultRecord/cover.phtml
@@ -1,16 +1,19 @@
-<?php $alt = $this->link ? trim($this->driver->tryMethod('getTitle') ?? '') : ''; ?>
-<?php if ($this->link && !empty($alt)): ?><a href="<?=$this->escapeHtmlAttr($this->link)?>" class="record-cover-link"><?php endif; ?>
+<?php if ($this->link): ?>
+  <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): ?>
-  <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 ($this->link && !empty($alt)): ?></a><?php endif; ?>
+<?php if ($this->link): ?></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; ?>