From af1d4b63d38ffda1b901ed82d95164113dce0077 Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Sun, 19 Aug 2018 19:31:54 -0700
Subject: [PATCH] better support for de-emojified user display names (#451)

improvements to #450 to fix #449, especially for aria labels
---
 routes/_components/compose/ComposeAuthor.html | 13 ++++++--
 .../profile/AccountDisplayName.html           | 13 +++-----
 .../profile/AccountProfileHeader.html         | 15 ++++++++-
 routes/_components/status/Status.html         | 13 ++++++--
 routes/_utils/emojifyText.js                  | 10 ------
 routes/_utils/removeEmoji.js                  | 17 ++++++++++
 tests/spec/118-display-name-custom-emoji.js   | 32 ++++++++++++++++++-
 7 files changed, 89 insertions(+), 24 deletions(-)
 create mode 100644 routes/_utils/removeEmoji.js

diff --git a/routes/_components/compose/ComposeAuthor.html b/routes/_components/compose/ComposeAuthor.html
index 7e4afccd..a33e9de0 100644
--- a/routes/_components/compose/ComposeAuthor.html
+++ b/routes/_components/compose/ComposeAuthor.html
@@ -1,6 +1,6 @@
 <a href="/accounts/{verifyCredentials.id}"
    class="compose-box-avatar"
-   aria-label="Profile for {verifyCredentials.display_name || verifyCredentials.acct}">
+   aria-label="Profile for {accessibleName}">
   <Avatar account={verifyCredentials} size="small"/>
 </a>
 <a class="compose-box-display-name" href="/accounts/{verifyCredentials.id}">
@@ -52,6 +52,7 @@
   import Avatar from '../Avatar.html'
   import { store } from '../../_store/store'
   import AccountDisplayName from '../profile/AccountDisplayName.html'
+  import { removeEmoji } from '../../_utils/removeEmoji'
 
   export default {
     components: {
@@ -60,7 +61,15 @@
     },
     store: () => store,
     computed: {
-      verifyCredentials: ({ $currentVerifyCredentials }) => $currentVerifyCredentials
+      verifyCredentials: ({ $currentVerifyCredentials }) => $currentVerifyCredentials,
+      emojis: ({ verifyCredentials }) => (verifyCredentials.emojis || []),
+      displayName: ({ verifyCredentials }) => verifyCredentials.display_name || verifyCredentials.username,
+      accessibleName: ({ displayName, emojis, $omitEmojiInDisplayNames }) => {
+        if ($omitEmojiInDisplayNames) {
+          return removeEmoji(displayName, emojis) || displayName
+        }
+        return displayName
+      }
     }
   }
 </script>
\ No newline at end of file
diff --git a/routes/_components/profile/AccountDisplayName.html b/routes/_components/profile/AccountDisplayName.html
index 40f25e6c..96165aa1 100644
--- a/routes/_components/profile/AccountDisplayName.html
+++ b/routes/_components/profile/AccountDisplayName.html
@@ -5,12 +5,10 @@
   }
 </style>
 <script>
-  import { emojifyText, removeEmoji } from '../../_utils/emojifyText'
+  import { emojifyText } from '../../_utils/emojifyText'
   import { store } from '../../_store/store'
   import escapeHtml from 'escape-html'
-  import emojiRegex from 'emoji-regex'
-
-  let theEmojiRegex
+  import { removeEmoji } from '../../_utils/removeEmoji'
 
   export default {
     store: () => store,
@@ -21,10 +19,9 @@
         accountName = escapeHtml(accountName)
 
         if ($omitEmojiInDisplayNames) { // display name emoji are annoying to some screenreader users
-          theEmojiRegex = theEmojiRegex || emojiRegex() // only init when needed
-          let emojiFreeAccountName = removeEmoji(accountName.replace(theEmojiRegex, ''), emojis).trim()
-          if (emojiFreeAccountName) {
-            return emojiFreeAccountName // only remove emoji if the resulting username is non-empty
+          let emojiFreeDisplayName = removeEmoji(accountName, emojis)
+          if (emojiFreeDisplayName) {
+            return emojiFreeDisplayName
           }
         }
 
diff --git a/routes/_components/profile/AccountProfileHeader.html b/routes/_components/profile/AccountProfileHeader.html
index 4927a37e..b13af1fa 100644
--- a/routes/_components/profile/AccountProfileHeader.html
+++ b/routes/_components/profile/AccountProfileHeader.html
@@ -5,7 +5,7 @@
   <ExternalLink href={account.url}
                 showIcon="true"
                 normalIconColor="true"
-                ariaLabel="{account.display_name || account.acct} (opens in new window)"
+                ariaLabel="{accessibleName} (opens in new window)"
   >
     <AccountDisplayName {account} />
   </ExternalLink>
@@ -81,8 +81,21 @@
   import Avatar from '../Avatar.html'
   import ExternalLink from '../ExternalLink.html'
   import AccountDisplayName from '../profile/AccountDisplayName.html'
+  import { removeEmoji } from '../../_utils/removeEmoji'
+  import { store } from '../../_store/store'
 
   export default {
+    store: () => store,
+    computed: {
+      emojis: ({ account }) => (account.emojis || []),
+      displayName: ({ account }) => account.display_name || account.username,
+      accessibleName: ({ displayName, emojis, $omitEmojiInDisplayNames }) => {
+        if ($omitEmojiInDisplayNames) {
+          return removeEmoji(displayName, emojis) || displayName
+        }
+        return displayName
+      }
+    },
     components: {
       Avatar,
       ExternalLink,
diff --git a/routes/_components/status/Status.html b/routes/_components/status/Status.html
index 1e3ff318..42abfb68 100644
--- a/routes/_components/status/Status.html
+++ b/routes/_components/status/Status.html
@@ -110,6 +110,7 @@
   import { classname } from '../../_utils/classname'
   import { checkDomAncestors } from '../../_utils/checkDomAncestors'
   import { scheduleIdleTask } from '../../_utils/scheduleIdleTask'
+  import { removeEmoji } from '../../_utils/removeEmoji'
 
   const INPUT_TAGS = new Set(['a', 'button', 'input', 'textarea'])
   const isUserInputElement = node => INPUT_TAGS.has(node.localName)
@@ -208,9 +209,17 @@
         originalStatus.media_attachments &&
         originalStatus.media_attachments.length
       ),
-      ariaLabel: ({ originalAccount, originalStatus, visibility }) => (
+      originalAccountEmojis: ({ originalAccount }) => (originalAccount.emojis || []),
+      originalAccountDisplayName: ({ originalAccount }) => (originalAccount.display_name || originalAccount.username),
+      originalAccountAccessibleName: ({ originalAccountDisplayName, originalAccountEmojis, $omitEmojiInDisplayNames }) => {
+        if ($omitEmojiInDisplayNames) {
+          return removeEmoji(originalAccountDisplayName, originalAccountEmojis) || originalAccountDisplayName
+        }
+        return originalAccountDisplayName
+      },
+      ariaLabel: ({ originalAccountAccessibleName, originalStatus, visibility }) => (
         (visibility === 'direct' ? 'Direct message' : 'Status') +
-          ` by ${originalAccount.display_name || originalAccount.username}`
+          ` by ${originalAccountAccessibleName}`
       ),
       showHeader: ({ notification, status, timelineType }) => (
         (notification && (notification.type === 'reblog' || notification.type === 'favourite')) ||
diff --git a/routes/_utils/emojifyText.js b/routes/_utils/emojifyText.js
index d203bba8..00028b39 100644
--- a/routes/_utils/emojifyText.js
+++ b/routes/_utils/emojifyText.js
@@ -15,13 +15,3 @@ export function emojifyText (text, emojis, autoplayGifs) {
   }
   return text
 }
-
-export function removeEmoji (text, emojis) {
-  if (emojis) {
-    for (let emoji of emojis) {
-      let shortcodeWithColons = `:${emoji.shortcode}:`
-      text = replaceAll(text, shortcodeWithColons, '')
-    }
-  }
-  return text
-}
diff --git a/routes/_utils/removeEmoji.js b/routes/_utils/removeEmoji.js
new file mode 100644
index 00000000..080ccfdf
--- /dev/null
+++ b/routes/_utils/removeEmoji.js
@@ -0,0 +1,17 @@
+import { replaceAll } from './strings'
+import emojiRegex from 'emoji-regex'
+
+let theEmojiRegex
+
+export function removeEmoji (text, emojis) {
+  // remove custom emoji
+  if (emojis) {
+    for (let emoji of emojis) {
+      let shortcodeWithColons = `:${emoji.shortcode}:`
+      text = replaceAll(text, shortcodeWithColons, '')
+    }
+  }
+  // remove regular emoji
+  theEmojiRegex = theEmojiRegex || emojiRegex() // only init when needed, then cache
+  return text.replace(theEmojiRegex, '').trim()
+}
diff --git a/tests/spec/118-display-name-custom-emoji.js b/tests/spec/118-display-name-custom-emoji.js
index ea10bad9..fa991331 100644
--- a/tests/spec/118-display-name-custom-emoji.js
+++ b/tests/spec/118-display-name-custom-emoji.js
@@ -1,6 +1,7 @@
 import { loginAsFoobar } from '../roles'
 import {
-  displayNameInComposeBox, generalSettingsButton, getNthStatusSelector, getUrl, homeNavButton,
+  avatarInComposeBox,
+  displayNameInComposeBox, generalSettingsButton, getNthStatus, getNthStatusSelector, getUrl, homeNavButton,
   removeEmojiFromDisplayNamesInput,
   settingsNavButton,
   sleep
@@ -38,6 +39,7 @@ test('Can remove emoji from user display names', async t => {
   await t
     .expect(displayNameInComposeBox.innerText).eql('🌈 foo  🌈')
     .expect($('.compose-box-display-name img').exists).ok()
+    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 foo :blobpats: 🌈')
     .click(settingsNavButton)
     .click(generalSettingsButton)
     .click(removeEmojiFromDisplayNamesInput)
@@ -45,6 +47,7 @@ test('Can remove emoji from user display names', async t => {
     .click(homeNavButton)
     .expect(displayNameInComposeBox.innerText).eql('foo')
     .expect($('.compose-box-display-name img').exists).notOk()
+    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for foo')
     .click(settingsNavButton)
     .click(generalSettingsButton)
     .click(removeEmojiFromDisplayNamesInput)
@@ -52,6 +55,7 @@ test('Can remove emoji from user display names', async t => {
     .click(homeNavButton)
     .expect(displayNameInComposeBox.innerText).eql('🌈 foo  🌈')
     .expect($('.compose-box-display-name img').exists).ok()
+    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 foo :blobpats: 🌈')
 })
 
 test('Cannot remove emoji from user display names if result would be empty', async t => {
@@ -61,6 +65,7 @@ test('Cannot remove emoji from user display names if result would be empty', asy
   await t
     .expect(displayNameInComposeBox.innerText).eql('🌈  🌈')
     .expect($('.compose-box-display-name img').exists).ok()
+    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 :blobpats: 🌈')
     .click(settingsNavButton)
     .click(generalSettingsButton)
     .click(removeEmojiFromDisplayNamesInput)
@@ -68,6 +73,7 @@ test('Cannot remove emoji from user display names if result would be empty', asy
     .click(homeNavButton)
     .expect(displayNameInComposeBox.innerText).eql('🌈  🌈')
     .expect($('.compose-box-display-name img').exists).ok()
+    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 :blobpats: 🌈')
     .click(settingsNavButton)
     .click(generalSettingsButton)
     .click(removeEmojiFromDisplayNamesInput)
@@ -75,4 +81,28 @@ test('Cannot remove emoji from user display names if result would be empty', asy
     .click(homeNavButton)
     .expect(displayNameInComposeBox.innerText).eql('🌈  🌈')
     .expect($('.compose-box-display-name img').exists).ok()
+    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 :blobpats: 🌈')
+})
+
+test('Check status aria labels for de-emojified text', async t => {
+  await updateUserDisplayNameAs('foobar', '🌈 foo :blobpats: 🌈')
+  await sleep(1000)
+  await loginAsFoobar(t)
+  await t
+    .click(displayNameInComposeBox)
+    .expect(getNthStatus(0).getAttribute('aria-label')).eql('Status by 🌈 foo :blobpats: 🌈')
+    .click(settingsNavButton)
+    .click(generalSettingsButton)
+    .click(removeEmojiFromDisplayNamesInput)
+    .expect(removeEmojiFromDisplayNamesInput.checked).ok()
+    .click(homeNavButton)
+    .click(displayNameInComposeBox)
+    .expect(getNthStatus(0).getAttribute('aria-label')).eql('Status by foo')
+    .click(settingsNavButton)
+    .click(generalSettingsButton)
+    .click(removeEmojiFromDisplayNamesInput)
+    .expect(removeEmojiFromDisplayNamesInput.checked).notOk()
+    .click(homeNavButton)
+    .click(displayNameInComposeBox)
+    .expect(getNthStatus(0).getAttribute('aria-label')).eql('Status by 🌈 foo :blobpats: 🌈')
 })