fix: fix pinned status aria-label/blurhash (nolanlawson#2313)

Fixes nolanlawson#2294
EldritchCafe · Dec 11, 2022 · 040462f · 040462f
1 parent f5f3395
commit 040462f
Showing 4 changed files with 103 additions and 65 deletions.
diff --git a/src/routes/_actions/createMakeProps.js b/src/routes/_actions/createMakeProps.js
@@ -1,9 +1,6 @@
 import { database } from '../_database/database.js'
-import { decode as decodeBlurhash, init as initBlurhash } from '../_utils/blurhash.js'
 import { mark, stop } from '../_utils/marks.js'
-import { get } from '../_utils/lodash-lite.js'
-import { statusHtmlToPlainText } from '../_utils/statusHtmlToPlainText.js'
-import { scheduleIdleTask } from '../_utils/scheduleIdleTask.js'
+import { prepareToRehydrate, rehydrateStatusOrNotification } from './rehydrateStatusOrNotification.js'
 
 async function getNotification (instanceName, timelineType, timelineValue, itemId) {
   return {
@@ -21,62 +18,10 @@ async function getStatus (instanceName, timelineType, timelineValue, itemId) {
   }
 }
 
-function tryInitBlurhash () {
-  try {
-    initBlurhash()
-  } catch (err) {
-    console.error('could not start blurhash worker', err)
-  }
-}
-
-function getActualStatus (statusOrNotification) {
-  return get(statusOrNotification, ['status']) ||
-    get(statusOrNotification, ['notification', 'status'])
-}
-
-async function decodeAllBlurhashes (statusOrNotification) {
-  const status = getActualStatus(statusOrNotification)
-  if (!status) {
-    return
-  }
-  const mediaWithBlurhashes = get(status, ['media_attachments'], [])
-    .concat(get(status, ['reblog', 'media_attachments'], []))
-    .filter(_ => _.blurhash)
-  if (mediaWithBlurhashes.length) {
-    mark(`decodeBlurhash-${status.id}`)
-    await Promise.all(mediaWithBlurhashes.map(async media => {
-      try {
-        media.decodedBlurhash = await decodeBlurhash(media.blurhash)
-      } catch (err) {
-        console.warn('Could not decode blurhash, ignoring', err)
-      }
-    }))
-    stop(`decodeBlurhash-${status.id}`)
-  }
-}
-
-async function calculatePlainTextContent (statusOrNotification) {
-  const status = getActualStatus(statusOrNotification)
-  if (!status) {
-    return
-  }
-  const originalStatus = status.reblog ? status.reblog : status
-  const content = originalStatus.content || ''
-  const mentions = originalStatus.mentions || []
-  // Calculating the plaintext from the HTML is a non-trivial operation, so we might
-  // as well do it in advance, while blurhash is being decoded on the worker thread.
-  await new Promise(resolve => {
-    scheduleIdleTask(() => {
-      originalStatus.plainTextContent = statusHtmlToPlainText(content, mentions)
-      resolve()
-    })
-  })
-}
-
 export function createMakeProps (instanceName, timelineType, timelineValue) {
   let promiseChain = Promise.resolve()
 
-  tryInitBlurhash() // start the blurhash worker a bit early to save time
+  prepareToRehydrate() // start blurhash early to save time
 
   async function fetchFromIndexedDB (itemId) {
     mark(`fetchFromIndexedDB-${itemId}`)
@@ -92,10 +37,7 @@ export function createMakeProps (instanceName, timelineType, timelineValue) {
 
   async function getStatusOrNotification (itemId) {
     const statusOrNotification = await fetchFromIndexedDB(itemId)
-    await Promise.all([
-      decodeAllBlurhashes(statusOrNotification),
-      calculatePlainTextContent(statusOrNotification)
-    ])
+    await rehydrateStatusOrNotification(statusOrNotification)
     return statusOrNotification
   }
 

diff --git a/src/routes/_actions/pinnedStatuses.js b/src/routes/_actions/pinnedStatuses.js
@@ -4,18 +4,28 @@ import { database } from '../_database/database.js'
 import {
   getPinnedStatuses
 } from '../_api/pinnedStatuses.js'
+import { prepareToRehydrate, rehydrateStatusOrNotification } from './rehydrateStatusOrNotification.js'
+
+// Pinned statuses aren't a "normal" timeline, so their blurhashes/plaintext need to be calculated specially
+async function rehydratePinnedStatuses (statuses) {
+  await Promise.all(statuses.map(status => rehydrateStatusOrNotification({ status })))
+  return statuses
+}
 
 export async function updatePinnedStatusesForAccount (accountId) {
   const { currentInstance, accessToken } = store.get()
 
   await cacheFirstUpdateAfter(
-    () => getPinnedStatuses(currentInstance, accessToken, accountId),
     async () => {
+      return rehydratePinnedStatuses(await getPinnedStatuses(currentInstance, accessToken, accountId))
+    },
+    async () => {
+      prepareToRehydrate() // start blurhash early to save time
       const pinnedStatuses = await database.getPinnedStatuses(currentInstance, accountId)
       if (!pinnedStatuses || !pinnedStatuses.every(Boolean)) {
         throw new Error('missing pinned statuses in idb')
       }
-      return pinnedStatuses
+      return rehydratePinnedStatuses(pinnedStatuses)
     },
     statuses => database.insertPinnedStatuses(currentInstance, accountId, statuses),
     statuses => {

diff --git a/src/routes/_actions/rehydrateStatusOrNotification.js b/src/routes/_actions/rehydrateStatusOrNotification.js
@@ -0,0 +1,67 @@
+import { get } from '../_utils/lodash-lite.js'
+import { mark, stop } from '../_utils/marks.js'
+import { decode as decodeBlurhash, init as initBlurhash } from '../_utils/blurhash.js'
+import { scheduleIdleTask } from '../_utils/scheduleIdleTask.js'
+import { statusHtmlToPlainText } from '../_utils/statusHtmlToPlainText.js'
+
+function getActualStatus (statusOrNotification) {
+  return get(statusOrNotification, ['status']) ||
+    get(statusOrNotification, ['notification', 'status'])
+}
+
+export function prepareToRehydrate () {
+  // start the blurhash worker a bit early to save time
+  try {
+    initBlurhash()
+  } catch (err) {
+    console.error('could not start blurhash worker', err)
+  }
+}
+
+async function decodeAllBlurhashes (statusOrNotification) {
+  const status = getActualStatus(statusOrNotification)
+  if (!status) {
+    return
+  }
+  const mediaWithBlurhashes = get(status, ['media_attachments'], [])
+    .concat(get(status, ['reblog', 'media_attachments'], []))
+    .filter(_ => _.blurhash)
+  if (mediaWithBlurhashes.length) {
+    mark(`decodeBlurhash-${status.id}`)
+    await Promise.all(mediaWithBlurhashes.map(async media => {
+      try {
+        media.decodedBlurhash = await decodeBlurhash(media.blurhash)
+      } catch (err) {
+        console.warn('Could not decode blurhash, ignoring', err)
+      }
+    }))
+    stop(`decodeBlurhash-${status.id}`)
+  }
+}
+
+async function calculatePlainTextContent (statusOrNotification) {
+  const status = getActualStatus(statusOrNotification)
+  if (!status) {
+    return
+  }
+  const originalStatus = status.reblog ? status.reblog : status
+  const content = originalStatus.content || ''
+  const mentions = originalStatus.mentions || []
+  // Calculating the plaintext from the HTML is a non-trivial operation, so we might
+  // as well do it in advance, while blurhash is being decoded on the worker thread.
+  await new Promise(resolve => {
+    scheduleIdleTask(() => {
+      originalStatus.plainTextContent = statusHtmlToPlainText(content, mentions)
+      resolve()
+    })
+  })
+}
+
+// Do stuff that we need to do when the status or notification is fetched from the database,
+// like calculating the blurhash or calculating the plain text content
+export async function rehydrateStatusOrNotification (statusOrNotification) {
+  await Promise.all([
+    decodeAllBlurhashes(statusOrNotification),
+    calculatePlainTextContent(statusOrNotification)
+  ])
+}
diff --git a/tests/spec/117-pin-unpin.js b/tests/spec/117-pin-unpin.js
@@ -4,10 +4,10 @@ import {
   getNthPinnedStatusFavoriteButton,
   getNthStatus, getNthStatusContent,
   getNthStatusOptionsButton, getUrl, homeNavButton, postStatusButton, scrollToTop, scrollToBottom,
-  settingsNavButton, sleep
+  settingsNavButton, sleep, getNthStatusAccountLink
 } from '../utils'
 import { users } from '../users'
-import { postAs } from '../serverActions'
+import { postAs, postStatusWithMediaAs } from '../serverActions'
 
 fixture`117-pin-unpin.js`
   .page`http://localhost:4002`
@@ -84,3 +84,22 @@ test('Saved pinned/unpinned state of status', async t => {
     .click(getNthStatusOptionsButton(1))
     .expect(getNthDialogOptionsOption(2).innerText).contains('Unpin from profile', { timeout })
 })
+
+test('pinned posts and aria-labels', async t => {
+  const timeout = 20000
+  await postStatusWithMediaAs('foobar', 'here is a sensitive kitty', 'kitten2.jpg', 'kitten', true)
+  await loginAsFoobar(t)
+  await t
+    .expect(getNthStatusContent(1).innerText).contains('here is a sensitive kitty', { timeout })
+    .click(getNthStatusOptionsButton(1))
+    .expect(getNthDialogOptionsOption(2).innerText).contains('Pin to profile')
+    .click(getNthDialogOptionsOption(2))
+    .click(getNthStatusAccountLink(1))
+    .expect(getNthPinnedStatus(1).getAttribute('aria-label')).match(
+      /foobar, here is a sensitive kitty, has media, (.+ ago|just now), @foobar, Public/i
+    )
+    .expect(getNthStatusContent(1).innerText).contains('here is a sensitive kitty')
+    .click(getNthStatusOptionsButton(1))
+    .expect(getNthDialogOptionsOption(2).innerText).contains('Unpin from profile')
+  await sleep(2000)
+})