From 1f0d67fcc422a7cf835f67695e2dabbb9ef10fe7 Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Sat, 25 Apr 2020 19:03:39 -0700
Subject: [PATCH] feat: aria-labels and buttons contain more media info (#1743)

* feat: aria-labels and buttons contain more media info

fixes #1733

* fix lint
---
 CONTRIBUTING.md                               |  6 +++---
 .../_a11y/getAccessibleLabelForStatus.js      |  6 +++++-
 src/routes/_components/status/Media.html      | 16 +++++++++-----
 src/routes/_components/status/Status.html     |  7 +++++--
 tests/spec/008-status-media.js                | 21 +++++++++++++++----
 tests/spec/120-status-aria-label.js           | 14 +++++++++++--
 6 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index a892a9e9..c100336a 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -14,7 +14,7 @@ To run a dev server with hot reloading:
 
 Now it's running at `localhost:4002`.
 
-**Linux users:** for file changes to work, 
+**Linux users:** for file changes to work,
 you'll probably want to run `export CHOKIDAR_USEPOLLING=1`
 because of [this issue](https://github.com/paulmillr/chokidar/issues/237).
 
@@ -39,7 +39,7 @@ running on `localhost:3000`.
 
 The integration tests require running Mastodon itself,
 meaning the [Mastodon development guide](https://docs.joinmastodon.org/development/overview/)
-is relevant here. In particular, you'll need a recent 
+is relevant here. In particular, you'll need a recent
 version of Ruby, Redis, and Postgres running. For a full list of deps, see `bin/setup-mastodon-in-travis.sh`.
 
 Run integration tests, using headless Chrome by default:
@@ -96,7 +96,7 @@ There are two parts to the Mastodon data used for testing:
 1. A Postgres dump and a tgz containing the media files, located in `fixtures`
 2. A script that populates the Mastodon backend with test data (`restore-mastodon-data.js`).
 
-The reason we don't use a Postgres dump for everything 
+The reason we don't use a Postgres dump for everything
 is that Mastodon will ignore changes made after a certain period of time, and we
 don't want our tests to randomly start breaking one day. Running the script ensures that statuses,
 favorites, boosts, etc. are all "fresh".
diff --git a/src/routes/_a11y/getAccessibleLabelForStatus.js b/src/routes/_a11y/getAccessibleLabelForStatus.js
index ae8abeb3..05f3ae8b 100644
--- a/src/routes/_a11y/getAccessibleLabelForStatus.js
+++ b/src/routes/_a11y/getAccessibleLabelForStatus.js
@@ -36,11 +36,13 @@ function cleanupText (text) {
 export function getAccessibleLabelForStatus (originalAccount, account, plainTextContent,
   timeagoFormattedDate, spoilerText, showContent,
   reblog, notification, visibility, omitEmojiInDisplayNames,
-  disableLongAriaLabels) {
+  disableLongAriaLabels, showMedia, showPoll) {
   const originalAccountDisplayName = getAccountAccessibleName(originalAccount, omitEmojiInDisplayNames)
   const contentTextToShow = (showContent || !spoilerText)
     ? cleanupText(plainTextContent)
     : `Content warning: ${cleanupText(spoilerText)}`
+  const mediaTextToShow = showMedia && 'has media'
+  const pollTextToShow = showPoll && 'has poll'
   const privacyText = getPrivacyText(visibility)
 
   if (disableLongAriaLabels) {
@@ -53,6 +55,8 @@ export function getAccessibleLabelForStatus (originalAccount, account, plainText
     getNotificationText(notification, omitEmojiInDisplayNames),
     originalAccountDisplayName,
     contentTextToShow,
+    mediaTextToShow,
+    pollTextToShow,
     timeagoFormattedDate,
     `@${originalAccount.acct}`,
     privacyText,
diff --git a/src/routes/_components/status/Media.html b/src/routes/_components/status/Media.html
index af522849..8d8aa013 100644
--- a/src/routes/_components/status/Media.html
+++ b/src/routes/_components/status/Media.html
@@ -2,7 +2,7 @@
     <button id={elementId}
             type="button"
             class="inline-media play-video-button focus-after {$largeInlineMedia ? '' : 'fixed-size'} {type === 'audio' ? 'play-audio-button' : ''}"
-            aria-label="Play {type === 'video' ? 'video' : 'audio'}: {description}"
+            aria-label={videoOrAudioButtonLabel}
             {tabindex}
             aria-hidden={ariaHidden}
             style={inlineMediaStyle}>
@@ -25,7 +25,7 @@
   <button id={elementId}
           type="button"
           class="inline-media show-image-button focus-after {$largeInlineMedia ? '' : 'fixed-size'}"
-          aria-label="Show image: {description}"
+          aria-label={imageButtonAriaLabel}
           title={description}
           on:mouseover="set({mouseover: event})"
           style={inlineMediaStyle}
@@ -34,7 +34,7 @@
   >
   {#if type === 'gifv' && $autoplayGifs && !blurhash}
     <AutoplayVideo
-      ariaLabel="Animated GIF: {description}"
+      ariaLabel="Animated image: {description}"
       poster={previewUrl}
       src={url}
       width={inlineWidth}
@@ -44,7 +44,7 @@
   {:elseif type === 'gifv'}
     <NonAutoplayGifv
       class={noNativeWidthHeight ? 'no-native-width-height' : ''}
-      label="Animated GIF: {description}"
+      label="Animated image: {description}"
       poster={previewUrl}
       {blurhash}
       src={url}
@@ -166,7 +166,13 @@ export default {
         }
       },
       tabindex: ({ showAsSensitive }) => showAsSensitive ? '-1' : '0',
-      ariaHidden: ({ showAsSensitive }) => showAsSensitive
+      ariaHidden: ({ showAsSensitive }) => showAsSensitive,
+      imageButtonAriaLabel: ({ type, description }) => (
+        `Show ${type === 'gifv' ? 'animated image' : 'image'}: ${description}`
+      ),
+      videoOrAudioButtonLabel: ({ type, description }) => (
+        `Play ${type === 'video' ? 'video' : 'audio'}: ${description}`
+      )
     },
     methods: {
       onClick () {
diff --git a/src/routes/_components/status/Status.html b/src/routes/_components/status/Status.html
index bc309a43..438a638c 100644
--- a/src/routes/_components/status/Status.html
+++ b/src/routes/_components/status/Status.html
@@ -280,11 +280,14 @@
       reblog: ({ status }) => status.reblog,
       ariaLabel: ({
         originalAccount, account, plainTextContent, timeagoFormattedDate, spoilerText,
-        showContent, reblog, notification, visibility, $omitEmojiInDisplayNames, $disableLongAriaLabels
+        showContent, reblog, notification, visibility, $omitEmojiInDisplayNames, $disableLongAriaLabels,
+        showMedia, showPoll
       }) => (
         getAccessibleLabelForStatus(originalAccount, account, plainTextContent,
           timeagoFormattedDate, spoilerText, showContent,
-          reblog, notification, visibility, $omitEmojiInDisplayNames, $disableLongAriaLabels)
+          reblog, notification, visibility, $omitEmojiInDisplayNames, $disableLongAriaLabels,
+          showMedia, showPoll
+        )
       ),
       showHeader: ({ notification, status, timelineType }) => (
         (notification && ['reblog', 'favourite', 'poll'].includes(notification.type)) ||
diff --git a/tests/spec/008-status-media.js b/tests/spec/008-status-media.js
index e112a320..eef39811 100644
--- a/tests/spec/008-status-media.js
+++ b/tests/spec/008-status-media.js
@@ -1,4 +1,11 @@
-import { closeDialogButton, getNthStatus, getNthStatusSelector, modalDialogContents, scrollToStatus } from '../utils'
+import {
+  closeDialogButton,
+  getNthStatus,
+  getNthStatusMediaButton,
+  getNthStatusSelector,
+  modalDialogContents,
+  scrollToStatus
+} from '../utils'
 import { loginAsFoobar } from '../roles'
 import { Selector as $ } from 'testcafe'
 import { homeTimeline } from '../fixtures'
@@ -13,8 +20,10 @@ test('shows sensitive images and videos', async t => {
   const videoIdx = homeTimeline.findIndex(_ => _.content === 'secret video')
 
   await scrollToStatus(t, 1 + kittenIdx)
-  await t.expect($(`${getNthStatusSelector(1 + kittenIdx)} .status-media img`).getAttribute('src')).match(/^blob:http:\/\/localhost/)
+  await t.expect($(`${getNthStatusSelector(1 + kittenIdx)} .status-media img`).getAttribute('src'))
+    .match(/^blob:http:\/\/localhost/)
     .click($(`${getNthStatusSelector(1 + kittenIdx)} .status-sensitive-media-button`))
+    .expect($(getNthStatusMediaButton(1 + kittenIdx)).getAttribute('aria-label')).eql('Show image: kitten')
     .expect($(`${getNthStatusSelector(1 + kittenIdx)} .status-media img`).getAttribute('alt')).eql('kitten')
     .expect($(`${getNthStatusSelector(1 + kittenIdx)} .status-media img`).getAttribute('src')).match(/^http:\/\//)
     .hover(getNthStatus(1 + videoIdx))
@@ -31,7 +40,9 @@ test('click and close image and video modals', async t => {
 
   await scrollToStatus(t, 1 + videoIdx)
   await t.expect(modalDialogContents.exists).notOk()
-    .click($(`${getNthStatusSelector(1 + videoIdx)} .play-video-button`))
+    .expect($(getNthStatusMediaButton(1 + videoIdx)).getAttribute('aria-label'))
+    .eql('Play video: kitten')
+    .click($(getNthStatusMediaButton(1 + videoIdx)))
     .expect(modalDialogContents.exists).ok()
     .expect($('.modal-dialog video').getAttribute('src')).contains('mp4')
     .expect($('.modal-dialog video').getAttribute('poster')).contains('png')
@@ -39,7 +50,9 @@ test('click and close image and video modals', async t => {
     .expect(modalDialogContents.exists).notOk()
     .hover(getNthStatus(1 + kittenIdx - 1))
     .hover(getNthStatus(1 + kittenIdx))
-    .click($(`${getNthStatusSelector(1 + kittenIdx)} .show-image-button`))
+    .expect($(getNthStatusMediaButton(1 + kittenIdx)).getAttribute('aria-label'))
+    .eql('Show animated image: kitten')
+    .click($(getNthStatusMediaButton(1 + kittenIdx)))
     .expect(modalDialogContents.exists).ok()
     .expect($('.modal-dialog video').getAttribute('src')).contains('mp4')
     .expect($('.modal-dialog video').getAttribute('poster')).contains('png')
diff --git a/tests/spec/120-status-aria-label.js b/tests/spec/120-status-aria-label.js
index 8b93a8ed..4ba32d17 100644
--- a/tests/spec/120-status-aria-label.js
+++ b/tests/spec/120-status-aria-label.js
@@ -1,6 +1,6 @@
 import { loginAsFoobar } from '../roles'
 import { getNthStatus } from '../utils'
-import { postAs, postEmptyStatusWithMediaAs } from '../serverActions'
+import { createPollAs, postAs, postEmptyStatusWithMediaAs } from '../serverActions'
 
 fixture`120-status-aria-label.js`
   .page`http://localhost:4002`
@@ -11,7 +11,17 @@ test('aria-labels for statuses with no content text', async t => {
   await t
     .hover(getNthStatus(1))
     .expect(getNthStatus(1).getAttribute('aria-label')).match(
-      /foobar, (.+ ago|just now), @foobar, Public/i
+      /foobar, has media, (.+ ago|just now), @foobar, Public/i
+    )
+})
+
+test('aria-labels for statuses with polls', async t => {
+  await createPollAs('foobar', 'here is my poll', ['yolo', 'whatever'])
+  await loginAsFoobar(t)
+  await t
+    .hover(getNthStatus(1))
+    .expect(getNthStatus(1).getAttribute('aria-label')).match(
+      /foobar, here is my poll, has poll, (.+ ago|just now), @foobar, Public/i
     )
 })