From 040462f5b50d5b168669b747a7eb220300e929af Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Sun, 11 Dec 2022 11:00:45 -0800 Subject: [PATCH] fix: fix pinned status aria-label/blurhash (#2313) Fixes #2294 --- src/routes/_actions/createMakeProps.js | 64 +----------------- src/routes/_actions/pinnedStatuses.js | 14 +++- .../_actions/rehydrateStatusOrNotification.js | 67 +++++++++++++++++++ tests/spec/117-pin-unpin.js | 23 ++++++- 4 files changed, 103 insertions(+), 65 deletions(-) create mode 100644 src/routes/_actions/rehydrateStatusOrNotification.js diff --git a/src/routes/_actions/createMakeProps.js b/src/routes/_actions/createMakeProps.js index e53ae926..d88853d0 100644 --- 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 index 6b8e4096..ba630ab7 100644 --- 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 new file mode 100644 index 00000000..113b27e7 --- /dev/null +++ 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 index 7c5a375d..646042e8 100644 --- 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) +})