diff --git a/ui/src/common/Artwork.test.jsx b/ui/src/common/Artwork.test.jsx index ed40823c8..9240e8a9e 100644 --- a/ui/src/common/Artwork.test.jsx +++ b/ui/src/common/Artwork.test.jsx @@ -192,6 +192,18 @@ describe('Artwork', () => { expect(onClick).toHaveBeenCalledTimes(1) }) + it('shows the placeholder for a cached fetch failure, which has no blob', () => { + useImageUrl.mockReturnValue({ + imgUrl: null, + loading: false, + error: true, + fromCache: false, + }) + const { container } = render() + expect(container.querySelector('img')).toBeNull() + expect(container.querySelector('canvas')).not.toBeNull() + }) + it('falls back to the placeholder when a refresh swaps in an uncached hash', () => { useImageUrl.mockReturnValue({ imgUrl: 'blob:old', diff --git a/ui/src/common/useImageUrl.js b/ui/src/common/useImageUrl.js index 736c599d6..8a7f2bcc7 100644 --- a/ui/src/common/useImageUrl.js +++ b/ui/src/common/useImageUrl.js @@ -43,16 +43,16 @@ export const useImageUrl = (url) => { const [error, setError] = useState(cached?.error || false) const abortedRef = useRef(false) - // Resync during render, not in the effect below: for one render otherwise imgUrl still holds the - // previous url's blob, so only fromCache can answer "was this url already cached when it started". - const [fromCache, setFromCache] = useState(!!cached) + // Resync during render: otherwise imgUrl holds the previous url's blob for one render, so only + // fromCache can answer "already cached" - and a remembered failure has no blob to paint. + const [fromCache, setFromCache] = useState(!!cached?.blobUrl) const [trackedUrl, setTrackedUrl] = useState(url) if (trackedUrl !== url) { setTrackedUrl(url) setImgUrl(cached?.blobUrl || null) setLoading(!!url && !cached) setError(cached?.error || false) - setFromCache(!!cached) + setFromCache(!!cached?.blobUrl) } useEffect(() => { diff --git a/ui/src/common/useImageUrl.test.js b/ui/src/common/useImageUrl.test.js index 976317105..818732d06 100644 --- a/ui/src/common/useImageUrl.test.js +++ b/ui/src/common/useImageUrl.test.js @@ -200,6 +200,7 @@ describe('useImageUrl', () => { expect(global.fetch).toHaveBeenCalledTimes(1) expect(result2.current.imgUrl).toBe('blob:mock-url') expect(result2.current.loading).toBe(false) + expect(result2.current.fromCache).toBe(true) }) it('should cache errors and not re-fetch broken URLs', async () => { @@ -230,5 +231,7 @@ describe('useImageUrl', () => { expect(result2.current.error).toBe(true) expect(result2.current.imgUrl).toBeNull() expect(result2.current.loading).toBe(false) + // A remembered failure has no blob, so callers must not treat it as instantly painted. + expect(result2.current.fromCache).toBe(false) }) })