From ea4c1ad819f538e92f06977376c3cecd131321b9 Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Tue, 24 Sep 2019 22:31:56 -0700
Subject: [PATCH] fix: return focus to sensitive media button (#1535)

* fix: return focus to sensitive media button

fixes #1517

* additional fix for media sensitive focus

* fix audio/video name in aria-label of button

* fix hotkeys
---
 src/routes/_components/status/Media.html      | 13 +++-
 .../status/StatusMediaAttachments.html        | 74 +++++++++++--------
 tests/spec/010-focus.js                       | 30 +++++++-
 3 files changed, 82 insertions(+), 35 deletions(-)

diff --git a/src/routes/_components/status/Media.html b/src/routes/_components/status/Media.html
index 6902cc30..6351b24a 100644
--- a/src/routes/_components/status/Media.html
+++ b/src/routes/_components/status/Media.html
@@ -2,7 +2,9 @@
     <button id={elementId}
             type="button"
             class="inline-media play-video-button focus-after {$largeInlineMedia ? '' : 'fixed-size'} {type === 'audio' ? 'play-audio-button' : ''}"
-            aria-label="Play video: {description}"
+            aria-label="Play {type === 'video' ? 'video' : 'audio'}: {description}"
+            {tabindex}
+            aria-hidden={ariaHidden}
             style={inlineMediaStyle}>
       <PlayVideoIcon />
       {#if type === 'video'}
@@ -26,7 +28,10 @@
           aria-label="Show image: {description}"
           title={description}
           on:mouseover="set({mouseover: event})"
-          style={inlineMediaStyle}>
+          style={inlineMediaStyle}
+          {tabindex}
+          aria-hidden={ariaHidden}
+  >
   {#if type === 'gifv' && $autoplayGifs && !blurhash}
     <AutoplayVideo
       ariaLabel="Animated GIF: {description}"
@@ -159,7 +164,9 @@ export default {
         } else {
           return `width: ${inlineWidth}px; height: ${inlineHeight}px;`
         }
-      }
+      },
+      tabindex: ({ showAsSensitive }) => showAsSensitive ? '-1' : '0',
+      ariaHidden: ({ showAsSensitive }) => showAsSensitive
     },
     methods: {
       onClick () {
diff --git a/src/routes/_components/status/StatusMediaAttachments.html b/src/routes/_components/status/StatusMediaAttachments.html
index 8494902f..af9e5cb2 100644
--- a/src/routes/_components/status/StatusMediaAttachments.html
+++ b/src/routes/_components/status/StatusMediaAttachments.html
@@ -1,37 +1,41 @@
 {#if sensitive }
 <div class={computedClass} style={customSize}>
   <div class="status-sensitive-inner-div">
-  {#if sensitiveShown}
-  <button id={elementId}
-          type="button"
-          class="status-sensitive-media-button"
-          aria-label="Hide sensitive media" >
-    <div class="svg-wrapper">
-      <SvgIcon className="status-sensitive-media-svg"
-               href="#fa-eye-slash" />
-    </div>
-  </button>
-  {:else}
-    <button id={elementId}
-            type="button"
-            class="status-sensitive-media-button"
-            aria-label="Show sensitive media" >
-
-      <div class="status-sensitive-media-warning">
-        <div class="status-sensitive-media-warning-text">
-          Sensitive content. Click to show.
+    {#if sensitiveShown}
+      <button id={elementId}
+              type="button"
+              class="status-sensitive-media-button"
+              aria-label="Hide sensitive media"
+              ref:hideSensitiveMedia
+      >
+        <div class="svg-wrapper">
+          <SvgIcon className="status-sensitive-media-svg"
+                   href="#fa-eye-slash" />
         </div>
-      </div>
-      <div class="svg-wrapper">
-        <SvgIcon className="status-sensitive-media-svg" href="#fa-eye" />
-      </div>
-    </button>
-  {/if}
-  <MediaAttachments {mediaAttachments} {sensitive} {sensitiveShown} {uuid} />
+      </button>
+    {:else}
+      <button id={elementId}
+              type="button"
+              class="status-sensitive-media-button"
+              aria-label="Show sensitive media"
+              ref:showSensitiveMedia
+      >
+
+        <div class="status-sensitive-media-warning">
+          <div class="status-sensitive-media-warning-text">
+            Sensitive content. Click to show.
+          </div>
+        </div>
+        <div class="svg-wrapper">
+          <SvgIcon className="status-sensitive-media-svg" href="#fa-eye" />
+        </div>
+      </button>
+    {/if}
+    <MediaAttachments {mediaAttachments} {sensitive} {sensitiveShown} {uuid} />
   </div>
 </div>
 {#if enableShortcuts}
-  <Shortcut scope={shortcutScope} key="y" on:pressed="toggleSensitiveMedia()"/>
+  <Shortcut scope={shortcutScope} key="y" on:pressed="toggleSensitiveMedia(false)"/>
 {/if}
 {:else}
   <MediaAttachments {mediaAttachments} {sensitive} {sensitiveShown} {uuid} />
@@ -167,7 +171,7 @@
   export default {
     oncreate () {
       const { elementId } = this.get()
-      registerClickDelegate(this, elementId, () => this.toggleSensitiveMedia())
+      registerClickDelegate(this, elementId, () => this.toggleSensitiveMedia(true))
     },
     components: {
       MediaAttachments,
@@ -206,12 +210,24 @@
       }
     },
     methods: {
-      toggleSensitiveMedia () {
+      toggleSensitiveMedia (changeFocus) {
         const { uuid } = this.get()
         const { sensitivesShown } = this.store.get()
         sensitivesShown[uuid] = !sensitivesShown[uuid]
         this.store.set({ sensitivesShown })
         this.fire('recalculateHeight')
+        // Only change focus for clicks, not for hotkeys. It's weird if, when the entire toot
+        // is focused and you press "y", that the focus changes to the sensitive media button.
+        if (changeFocus) {
+          requestAnimationFrame(() => {
+            const element = this.refs.hideSensitiveMedia || this.refs.showSensitiveMedia
+            try {
+              element.focus({ preventScroll: true })
+            } catch (e) {
+              console.error('ignored focus error', e)
+            }
+          })
+        }
         return true
       }
     }
diff --git a/tests/spec/010-focus.js b/tests/spec/010-focus.js
index a8b25379..1ef042c6 100644
--- a/tests/spec/010-focus.js
+++ b/tests/spec/010-focus.js
@@ -1,7 +1,20 @@
 import {
-  getNthStatus, scrollToStatus, closeDialogButton, modalDialogContents, goBack, getUrl,
-  goBackButton, getActiveElementInnerText, getNthReplyButton, getActiveElementInsideNthStatus, focus,
-  getNthStatusSelector, getActiveElementTagName, getActiveElementClassList
+  getNthStatus,
+  scrollToStatus,
+  closeDialogButton,
+  modalDialogContents,
+  goBack,
+  getUrl,
+  goBackButton,
+  getActiveElementInnerText,
+  getNthReplyButton,
+  getActiveElementInsideNthStatus,
+  focus,
+  getNthStatusSelector,
+  getActiveElementTagName,
+  getActiveElementClassList,
+  getNthStatusSensitiveMediaButton,
+  getActiveElementAriaLabel
 } from '../utils'
 import { loginAsFoobar } from '../roles'
 import { Selector as $ } from 'testcafe'
@@ -113,3 +126,14 @@ test('reply preserves focus and moves focus to the text input', async t => {
 test('focus main content element on index page load', async t => {
   await t.expect(getActiveElementTagName()).match(/body/i)
 })
+
+test('clicking sensitive button returns focus to sensitive button', async t => {
+  await loginAsFoobar(t)
+  const sensitiveKittenIdx = homeTimeline.findIndex(_ => _.spoiler === 'kitten CW')
+  await scrollToStatus(t, sensitiveKittenIdx + 1)
+  await t
+    .click(getNthStatusSensitiveMediaButton(sensitiveKittenIdx + 1))
+    .expect(getActiveElementAriaLabel()).eql('Hide sensitive media')
+    .click(getNthStatusSensitiveMediaButton(sensitiveKittenIdx + 1))
+    .expect(getActiveElementAriaLabel()).eql('Show sensitive media')
+})