fix(media): drop the stale loopback url when the media source changes (#1855)

<!-- Please read
https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before
submitting your pull request -->

### Description

<!-- Please include a summary of the change. Please also include
relevant motivation and context. List any dependencies that are required
for this change. -->

Fixes #

#### Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

### Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings

### AI disclosure:

- [ ] Partially AI assisted (clarify which code was AI assisted and
briefly explain what it does).
- [ ] Fully AI generated (explain what all the generated code does in
moderate detail).
<!-- Write any explanation required here, but do not generate the
explanation using AI!! You must prove you understand what the code in
this PR does. -->

Author: eleboucher

src/app/components/room-avatar/AvatarImage.tsx

@@ -4,7 +4,6 @@ import { useState } from 'react';
 import bgColorImg from '$utils/bgColorImg';
 import { settingsAtom } from '$state/settings';
 import { useSetting } from '$state/hooks/settings';
-import { useRenderableMediaUrl } from '$hooks/useRenderableMediaUrl';
 import * as css from './RoomAvatar.css';
 
 type AvatarImageProps = {
@@ -17,8 +16,6 @@ type AvatarImageProps = {
 export function AvatarImage({ src, alt, uniformIcons, onError }: AvatarImageProps) {
   const [uniformIconsSetting] = useSetting(settingsAtom, 'uniformIcons');
   const [image, setImage] = useState<HTMLImageElement | undefined>(undefined);
-  const resolvedSrc = useRenderableMediaUrl(src);
-  const mediaSrc = resolvedSrc ?? src;
 
   const useUniformIcons = uniformIconsSetting && uniformIcons === true;
   const normalizedBg = useUniformIcons && image ? bgColorImg(image) : undefined;
@@ -28,13 +25,13 @@ export function AvatarImage({ src, alt, uniformIcons, onError }: AvatarImageProp
     setImage(evt.currentTarget);
   };
 
-  const isBlobUrl = mediaSrc.startsWith('blob:');
+  const isBlobUrl = src.startsWith('blob:');
 
   return (
     <FoldsAvatarImage
       className={css.RoomAvatar}
       style={{ backgroundColor: useUniformIcons ? normalizedBg : undefined }}
-      src={mediaSrc}
+      src={src}
       crossOrigin={isBlobUrl ? undefined : 'anonymous'}
       alt={alt}
       loading="lazy"

src/app/components/room-avatar/RoomAvatar.test.tsx

@@ -0,0 +1,34 @@
+import { fireEvent, render, screen } from '@testing-library/react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const media = vi.hoisted(() => ({
+  useRenderableMediaUrl: vi.fn<(url: string | undefined) => string | undefined>(),
+}));
+
+vi.mock('$hooks/useRenderableMediaUrl', () => media);
+
+const RAW_SRC = 'https://example.org/_matrix/client/v1/media/thumbnail/example.org/avatar';
+
+describe('RoomAvatar', () => {
+  beforeEach(() => {
+    vi.resetModules();
+    media.useRenderableMediaUrl.mockReset();
+  });
+
+  it('shows the image once the resolved url arrives after a failed raw request', async () => {
+    media.useRenderableMediaUrl.mockReturnValue(undefined);
+    const { RoomAvatar } = await import('./RoomAvatar');
+
+    const { rerender } = render(
+      <RoomAvatar roomId="!room:example.org" src={RAW_SRC} renderFallback={() => 'RM'} />
+    );
+
+    fireEvent.error(screen.getByRole('img'));
+    expect(screen.queryByRole('img')).not.toBeInTheDocument();
+
+    media.useRenderableMediaUrl.mockReturnValue('blob:resolved-avatar');
+    rerender(<RoomAvatar roomId="!room:example.org" src={RAW_SRC} renderFallback={() => 'RM'} />);
+
+    expect(screen.getByRole('img')).toHaveAttribute('src', 'blob:resolved-avatar');
+  });
+});

src/app/components/room-avatar/RoomAvatar.tsx

@@ -12,6 +12,7 @@ import {
   getRoomStandaloneIconComponent,
 } from '$components/icons/roomIcons';
 import colorMXID from '$utils/colorMXID';
+import { useRenderableMediaUrl } from '$hooks/useRenderableMediaUrl';
 import * as css from './RoomAvatar.css';
 import { AvatarImage } from './AvatarImage';
 
@@ -25,12 +26,14 @@ type RoomAvatarProps = {
 
 export function RoomAvatar({ roomId, src, alt, renderFallback, uniformIcons }: RoomAvatarProps) {
   const [error, setError] = useState(false);
+  const resolvedSrc = useRenderableMediaUrl(src);
+  const mediaSrc = resolvedSrc ?? src;
 
   useEffect(() => {
     setError(false);
-  }, [src]);
+  }, [mediaSrc]);
 
-  if (!src || error) {
+  if (!mediaSrc || error) {
     return (
       <AvatarFallback
         style={{ backgroundColor: colorMXID(roomId ?? ''), color: color.Surface.Container }}
@@ -42,7 +45,12 @@ export function RoomAvatar({ roomId, src, alt, renderFallback, uniformIcons }: R
   }
 
   return (
-    <AvatarImage src={src} alt={alt} uniformIcons={uniformIcons} onError={() => setError(true)} />
+    <AvatarImage
+      src={mediaSrc}
+      alt={alt}
+      uniformIcons={uniformIcons}
+      onError={() => setError(true)}
+    />
   );
 }
 

src/app/components/user-avatar/UserAvatar.test.tsx

@@ -0,0 +1,34 @@
+import { fireEvent, render, screen } from '@testing-library/react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const media = vi.hoisted(() => ({
+  useRenderableMediaUrl: vi.fn<(url: string | undefined) => string | undefined>(),
+}));
+
+vi.mock('$hooks/useRenderableMediaUrl', () => media);
+
+const RAW_SRC = 'https://example.org/_matrix/client/v1/media/thumbnail/example.org/avatar';
+
+describe('UserAvatar', () => {
+  beforeEach(() => {
+    vi.resetModules();
+    media.useRenderableMediaUrl.mockReset();
+  });
+
+  it('shows the image once the resolved url arrives after a failed raw request', async () => {
+    media.useRenderableMediaUrl.mockReturnValue(undefined);
+    const { UserAvatar } = await import('./UserAvatar');
+
+    const { rerender } = render(
+      <UserAvatar userId="@user:example.org" src={RAW_SRC} renderFallback={() => 'US'} />
+    );
+
+    fireEvent.error(screen.getByRole('img'));
+    expect(screen.queryByRole('img')).not.toBeInTheDocument();
+
+    media.useRenderableMediaUrl.mockReturnValue('blob:resolved-avatar');
+    rerender(<UserAvatar userId="@user:example.org" src={RAW_SRC} renderFallback={() => 'US'} />);
+
+    expect(screen.getByRole('img')).toHaveAttribute('src', 'blob:resolved-avatar');
+  });
+});

src/app/components/user-avatar/UserAvatar.tsx

@@ -29,12 +29,13 @@ export function UserAvatar({
 }: UserAvatarProps) {
   const [error, setError] = useState(false);
   const resolvedSrc = useRenderableMediaUrl(src);
+  const mediaSrc = resolvedSrc ?? src;
 
   useEffect(() => {
     setError(false);
-  }, [src]);
+  }, [mediaSrc]);
 
-  if (!src || error) {
+  if (!mediaSrc || error) {
     return (
       <AvatarFallback
         style={{
@@ -51,7 +52,7 @@ export function UserAvatar({
   return (
     <AvatarImage
       className={classNames(css.UserAvatar, className)}
-      src={resolvedSrc ?? src}
+      src={mediaSrc}
       alt={alt}
       loading="lazy"
       decoding="async"

src/app/hooks/useRenderableMediaUrl.test.tsx

@@ -236,6 +236,22 @@ describe('useRenderableMediaUrl', () => {
     expect(tauriApi.convertFileSrc).not.toHaveBeenCalled();
   });
 
+  it('drops the previous loopback url when the media source goes away under Tauri', async () => {
+    tauriApi.isTauri.mockReturnValue(true);
+    const { useRenderableMediaUrl } = await import('./useRenderableMediaUrl');
+
+    const { result, rerender } = renderHook(
+      ({ url }: { url: string | undefined }) => useRenderableMediaUrl(url),
+      { initialProps: { url: 'https://example.org/banner.png' as string | undefined } }
+    );
+
+    await waitFor(() => expect(result.current).toBe(LOOPBACK_URL));
+
+    rerender({ url: undefined });
+
+    expect(result.current).toBeUndefined();
+  });
+
   it('passes through non-authenticated URLs unchanged under Tauri', async () => {
     tauriApi.isTauri.mockReturnValue(true);
     const { useRenderableMediaUrl } = await import('./useRenderableMediaUrl');

src/app/hooks/useRenderableMediaUrl.ts

@@ -183,20 +183,21 @@ export function useRenderableMediaUrl(url: string | undefined): string | undefin
   );
   const protocolUrl = tauri ? (rewriteAuthenticatedMediaUrl(url ?? null) ?? undefined) : undefined;
   // A settled entry resolves synchronously, so a repeated avatar never flashes a fallback.
-  const [loopbackUrl, setLoopbackUrl] = useState<string | undefined>(() =>
-    tauri && protocolUrl ? loopbackCache.get(protocolUrl)?.url : undefined
-  );
+  const [loopbackState, setLoopbackState] = useState<{ source?: string; url?: string }>(() => ({
+    source: protocolUrl,
+    url: tauri && protocolUrl ? loopbackCache.get(protocolUrl)?.url : undefined,
+  }));
 
   useEffect(() => {
     if (!tauri || !protocolUrl) return undefined;
     const entry = resolveLoopbackUrl(protocolUrl);
     if (entry.url) {
-      setLoopbackUrl(entry.url);
+      setLoopbackState({ source: protocolUrl, url: entry.url });
       return undefined;
     }
     let cancelled = false;
     void entry.promise.then((resolved) => {
-      if (!cancelled) setLoopbackUrl(resolved);
+      if (!cancelled) setLoopbackState({ source: protocolUrl, url: resolved });
     });
     return () => {
       cancelled = true;
@@ -270,7 +271,9 @@ export function useRenderableMediaUrl(url: string | undefined): string | undefin
   if (tauri) {
     // No protocolUrl fallback while resolving: resolveLoopbackUrl already degrades to it,
     // and handing out the custom-scheme URL first would fail a media element.
-    return loopbackUrl;
+    if (!protocolUrl) return undefined;
+    if (loopbackState.source === protocolUrl) return loopbackState.url;
+    return loopbackCache.get(protocolUrl)?.url;
   }
 
   if (!needsBlob || usesExistingObjectUrl) {