From a4e78d949f752f1ce565ba4bf0eace04504e7003 Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Sun, 2 Aug 2026 14:21:35 -0400
Subject: [PATCH] fix(ui): keep the placeholder for a cover whose fetch failed

useImageUrl caches a failed fetch as an entry with no blob, so treating every
cached entry as instantly painted suppressed the placeholder on remount: the
cover became an empty box instead of falling back to its thumbhash. Only a
cached blob counts as instant.

Caught by Codex on the previous commit, which introduced fromCache with the
looser predicate.
---
 ui/src/common/Artwork.test.jsx    | 12 ++++++++++++
 ui/src/common/useImageUrl.js      |  8 ++++----
 ui/src/common/useImageUrl.test.js |  3 +++
 3 files changed, 19 insertions(+), 4 deletions(-)

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(<Artwork record={withArt} />)
+    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)
   })
 })