From 153e4f4fcde2cbea95641c2e179dbfea26e1324d Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Sat, 1 Dec 2018 11:53:20 -0800
Subject: [PATCH] feat(a11y): add option for short article aria labels (#705)

Actually fixes #694 by providing an option to make the labels like they used to be.
---
 routes/_a11y/getAccessibleLabelForStatus.js | 36 ++++-----
 routes/_components/status/Status.html       |  4 +-
 routes/_pages/settings/general.html         |  5 ++
 routes/_store/store.js                      | 82 ++++++++++-----------
 tests/spec/022-status-aria-label.js         | 31 +++++++-
 5 files changed, 93 insertions(+), 65 deletions(-)

diff --git a/routes/_a11y/getAccessibleLabelForStatus.js b/routes/_a11y/getAccessibleLabelForStatus.js
index 2d42639..280faa5 100644
--- a/routes/_a11y/getAccessibleLabelForStatus.js
+++ b/routes/_a11y/getAccessibleLabelForStatus.js
@@ -2,9 +2,7 @@ import { getAccountAccessibleName } from './getAccountAccessibleName'
 import { POST_PRIVACY_OPTIONS } from '../_static/statuses'
 import { htmlToPlainText } from '../_utils/htmlToPlainText'
 
-const MAX_TEXT_LENGTH = 150
-
-function notificationText (notification, omitEmojiInDisplayNames) {
+function getNotificationText (notification, omitEmojiInDisplayNames) {
   if (!notification) {
     return
   }
@@ -16,7 +14,7 @@ function notificationText (notification, omitEmojiInDisplayNames) {
   }
 }
 
-function privacyText (visibility) {
+function getPrivacyText (visibility) {
   for (let option of POST_PRIVACY_OPTIONS) {
     if (option.key === visibility) {
       return option.label
@@ -24,7 +22,7 @@ function privacyText (visibility) {
   }
 }
 
-function reblogText (reblog, account, omitEmojiInDisplayNames) {
+function getReblogText (reblog, account, omitEmojiInDisplayNames) {
   if (!reblog) {
     return
   }
@@ -32,32 +30,34 @@ function reblogText (reblog, account, omitEmojiInDisplayNames) {
   return `Boosted by ${accountDisplayName}`
 }
 
-// Works around a bug in NVDA where it may crash if the string is too long
-// https://github.com/nolanlawson/pinafore/issues/694
-function truncateTextForSRs (text) {
-  if (text.length > MAX_TEXT_LENGTH) {
-    text = text.substring(0, MAX_TEXT_LENGTH)
-    text = text.replace(/\S+$/, '') + ' (truncated)'
-  }
+function cleanupText (text) {
   return text.replace(/\s+/g, ' ').trim()
 }
 
 export function getAccessibleLabelForStatus (originalAccount, account, content,
   timeagoFormattedDate, spoilerText, showContent,
-  reblog, notification, visibility, omitEmojiInDisplayNames) {
+  reblog, notification, visibility, omitEmojiInDisplayNames,
+  disableLongAriaLabels) {
   let originalAccountDisplayName = getAccountAccessibleName(originalAccount, omitEmojiInDisplayNames)
   let contentTextToShow = (showContent || !spoilerText)
-    ? truncateTextForSRs(htmlToPlainText(content))
-    : `Content warning: ${truncateTextForSRs(spoilerText)}`
+    ? cleanupText(htmlToPlainText(content))
+    : `Content warning: ${cleanupText(spoilerText)}`
+  let privacyText = getPrivacyText(visibility)
+
+  if (disableLongAriaLabels) {
+    // Long text can crash NVDA; allow users to shorten it like we had it before.
+    // https://github.com/nolanlawson/pinafore/issues/694
+    return `${privacyText} status by ${originalAccountDisplayName}`
+  }
 
   let values = [
-    notificationText(notification, omitEmojiInDisplayNames),
+    getNotificationText(notification, omitEmojiInDisplayNames),
     originalAccountDisplayName,
     contentTextToShow,
     timeagoFormattedDate,
     `@${originalAccount.acct}`,
-    privacyText(visibility),
-    reblogText(reblog, account, omitEmojiInDisplayNames)
+    privacyText,
+    getReblogText(reblog, account, omitEmojiInDisplayNames)
   ].filter(Boolean)
 
   return values.join(', ')
diff --git a/routes/_components/status/Status.html b/routes/_components/status/Status.html
index 4403ebd..cd5e5c3 100644
--- a/routes/_components/status/Status.html
+++ b/routes/_components/status/Status.html
@@ -220,10 +220,10 @@
       timeagoFormattedDate: ({ createdAtDate }) => formatTimeagoDate(createdAtDate),
       reblog: ({ status }) => status.reblog,
       ariaLabel: ({ originalAccount, account, content, timeagoFormattedDate, spoilerText,
-        showContent, reblog, notification, visibility, $omitEmojiInDisplayNames }) => (
+        showContent, reblog, notification, visibility, $omitEmojiInDisplayNames, $disableLongAriaLabels }) => (
         getAccessibleLabelForStatus(originalAccount, account, content,
           timeagoFormattedDate, spoilerText, showContent,
-          reblog, notification, visibility, $omitEmojiInDisplayNames)
+          reblog, notification, visibility, $omitEmojiInDisplayNames, $disableLongAriaLabels)
       ),
       showHeader: ({ notification, status, timelineType }) => (
         (notification && (notification.type === 'reblog' || notification.type === 'favourite')) ||
diff --git a/routes/_pages/settings/general.html b/routes/_pages/settings/general.html
index 81e0385..5b7c447 100644
--- a/routes/_pages/settings/general.html
+++ b/routes/_pages/settings/general.html
@@ -28,6 +28,11 @@
              bind:checked="$disableCustomScrollbars" on:change="$save()">
       <label for="choice-disable-custom-scrollbars">Disable custom scrollbars</label>
     </div>
+    <div class="setting-group">
+      <input type="checkbox" id="choice-disable-long-aria-labels"
+             bind:checked="$disableLongAriaLabels" on:change="$save()">
+      <label for="choice-disable-long-aria-labels">Use short article ARIA labels</label>
+    </div>
   </form>
 
   <h2>Themes
diff --git a/routes/_store/store.js b/routes/_store/store.js
index 28db107..f614c54 100644
--- a/routes/_store/store.js
+++ b/routes/_store/store.js
@@ -4,58 +4,52 @@ import { mixins } from './mixins/mixins'
 import { LocalStorageStore } from './LocalStorageStore'
 import { observe } from 'svelte-extras'
 
-const KEYS_TO_STORE_IN_LOCAL_STORAGE = new Set([
-  'currentInstance',
-  'currentRegisteredInstance',
-  'currentRegisteredInstanceName',
-  'instanceNameInSearch',
-  'instanceThemes',
-  'loggedInInstances',
-  'loggedInInstancesInOrder',
-  'autoplayGifs',
-  'markMediaAsSensitive',
-  'reduceMotion',
-  'disableCustomScrollbars',
-  'omitEmojiInDisplayNames',
-  'pinnedPages',
-  'composeData',
-  'pushSubscription'
-])
+const persistedState = {
+  autoplayGifs: false,
+  composeData: {},
+  currentInstance: null,
+  currentRegisteredInstanceName: undefined,
+  currentRegisteredInstance: undefined,
+  disableCustomScrollbars: false,
+  disableLongAriaLabels: false,
+  instanceNameInSearch: '',
+  instanceThemes: {},
+  loggedInInstances: {},
+  loggedInInstancesInOrder: [],
+  markMediaAsSensitive: false,
+  omitEmojiInDisplayNames: undefined,
+  pinnedPages: {},
+  pushSubscription: null,
+  reduceMotion: !process.browser || window.matchMedia('(prefers-reduced-motion: reduce)').matches
+}
+
+const nonPersistedState = {
+  customEmoji: {},
+  instanceInfos: {},
+  instanceLists: {},
+  online: !process.browser || navigator.onLine,
+  pinnedStatuses: {},
+  pushNotificationsSupport: process.browser && ('serviceWorker' in navigator && 'PushManager' in window && 'getKey' in window.PushSubscription.prototype),
+  queryInSearch: '',
+  repliesShown: {},
+  sensitivesShown: {},
+  spoilersShown: {},
+  statusModifications: {},
+  verifyCredentials: {}
+}
+
+const state = Object.assign({}, persistedState, nonPersistedState)
+const keysToStoreInLocalStorage = new Set(Object.keys(persistedState))
 
 class PinaforeStore extends LocalStorageStore {
   constructor (state) {
-    super(state, KEYS_TO_STORE_IN_LOCAL_STORAGE)
+    super(state, keysToStoreInLocalStorage)
   }
 }
 
 PinaforeStore.prototype.observe = observe
 
-export const store = new PinaforeStore({
-  instanceNameInSearch: '',
-  queryInSearch: '',
-  currentInstance: null,
-  loggedInInstances: {},
-  loggedInInstancesInOrder: [],
-  instanceThemes: {},
-  spoilersShown: {},
-  sensitivesShown: {},
-  repliesShown: {},
-  autoplayGifs: false,
-  markMediaAsSensitive: false,
-  reduceMotion: !process.browser || window.matchMedia('(prefers-reduced-motion: reduce)').matches,
-  disableCustomScrollbars: false,
-  pinnedPages: {},
-  instanceLists: {},
-  pinnedStatuses: {},
-  instanceInfos: {},
-  statusModifications: {},
-  customEmoji: {},
-  composeData: {},
-  verifyCredentials: {},
-  online: !process.browser || navigator.onLine,
-  pushNotificationsSupport: process.browser && ('serviceWorker' in navigator && 'PushManager' in window && 'getKey' in window.PushSubscription.prototype),
-  pushSubscription: null
-})
+export const store = new PinaforeStore(state)
 
 mixins(PinaforeStore)
 computations(store)
diff --git a/tests/spec/022-status-aria-label.js b/tests/spec/022-status-aria-label.js
index 496ee67..b6322ef 100644
--- a/tests/spec/022-status-aria-label.js
+++ b/tests/spec/022-status-aria-label.js
@@ -1,5 +1,13 @@
 import { loginAsFoobar } from '../roles'
-import { getNthShowOrHideButton, getNthStatus, notificationsNavButton, scrollToStatus } from '../utils'
+import {
+  generalSettingsButton,
+  getNthShowOrHideButton,
+  getNthStatus, homeNavButton,
+  notificationsNavButton,
+  scrollToStatus,
+  settingsNavButton
+} from '../utils'
+import { Selector as $ } from 'testcafe'
 import { indexWhere } from '../../routes/_utils/arrays'
 import { homeTimeline } from '../fixtures'
 
@@ -67,3 +75,24 @@ test('aria-labels for notifications', async t => {
       /quux followed you, @quux/i
     )
 })
+
+test('can shorten aria-labels', async t => {
+  await loginAsFoobar(t)
+  await t
+    .click(settingsNavButton)
+    .click(generalSettingsButton)
+    .click($('#choice-disable-long-aria-labels'))
+    .click(homeNavButton)
+    .hover(getNthStatus(0))
+    .expect(getNthStatus(0).getAttribute('aria-label')).match(
+      /Unlisted status by quux/
+    )
+    .click(settingsNavButton)
+    .click(generalSettingsButton)
+    .click($('#choice-disable-long-aria-labels'))
+    .click(homeNavButton)
+    .hover(getNthStatus(0))
+    .expect(getNthStatus(0).getAttribute('aria-label')).match(
+      /quux, pinned toot 1, .+ ago, @quux, Unlisted, Boosted by admin/i
+    )
+})