fix(ui): a picture that fails to load shows the card's own frame, then retries
A bare <img> sent a Referer that Google's image CDNs refuse and had no retry and no fallback, so a Home shelf card showed the browser's broken-image mark. ui/RemoteImage gathers what Avatar, the channel banner and the download rows had each hand-rolled: no Referer, up to three retries past the browser's negative cache, the caller's fallback (by default the card's empty frame) instead of the broken mark, the failure remembered by URL. Every picture now goes through it; a lint rule refuses a bare <img>.
This commit is contained in:
@@ -1,15 +1,9 @@
|
||||
import { useState } from "react";
|
||||
import RemoteImage from "./RemoteImage";
|
||||
|
||||
// Avatar image with a graceful fallback.
|
||||
//
|
||||
// Uses referrerPolicy="no-referrer" because Google's avatar CDNs
|
||||
// (yt3.ggpht.com / lh3.googleusercontent.com) refuse referrer-bearing requests
|
||||
// from our origin — yt3 refuses even a single load, not only a burst (measured
|
||||
// 2026-09-26) — and the browser shows its broken-image icon. Dropping the
|
||||
// referrer makes those CDNs serve the images. The channel banner, from the same
|
||||
// CDN, does the same (ChannelPage).
|
||||
// If a load still fails we render a neutral placeholder (optionally an initial)
|
||||
// instead of the broken-image icon.
|
||||
// Avatar image with a graceful fallback: a neutral disc (optionally the name's initial) while there is
|
||||
// no picture. The loading itself — no Referer for Google's avatar CDNs, the retry past the browser's
|
||||
// negative cache, the failure remembered by URL so a recycled feed slot never paints one channel's
|
||||
// initial over the next one's picture — is ui/RemoteImage's.
|
||||
export default function Avatar({
|
||||
src,
|
||||
alt = "",
|
||||
@@ -21,43 +15,21 @@ export default function Avatar({
|
||||
fallback?: string; // a name/title; its first letter is shown when there's no image
|
||||
className?: string;
|
||||
}) {
|
||||
// The failure is remembered BY URL, not as a boolean that an effect resets when `src` changes.
|
||||
// That distinction is visible: this component is recycled across items in the virtualized feed, so
|
||||
// one avatar's 429 used to paint the fallback initial over the NEXT item's perfectly good image
|
||||
// for a frame — React committed the render with the new `src` and the stale `failed`, and only the
|
||||
// post-paint effect cleared it. Derived state cannot be stale, so there is nothing to reset.
|
||||
//
|
||||
// Forgetting the URL once `src` has moved off it is the other half, and it is what keeps the 429
|
||||
// TRANSIENT: without it the memory outlives the reason for it, and a mounted instance whose `src`
|
||||
// swaps — the player's channel avatar stepping through a queue — would
|
||||
// show the initial for a throttled channel forever, never re-requesting even after the CDN
|
||||
// relented. Written in render because it is convergent (the write falsifies its own condition) and
|
||||
// because an effect would leave one committed frame in which the memory is stale, which is the
|
||||
// frame this whole component is about.
|
||||
const [failedSrc, setFailedSrc] = useState<string | null>(null);
|
||||
if (failedSrc !== null && failedSrc !== src) setFailedSrc(null);
|
||||
const failed = !!src && failedSrc === src;
|
||||
|
||||
if (src && !failed) {
|
||||
return (
|
||||
<img
|
||||
src={src}
|
||||
alt={alt}
|
||||
loading="lazy"
|
||||
referrerPolicy="no-referrer"
|
||||
onError={() => setFailedSrc(src)}
|
||||
className={className}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
const initial = fallback?.trim()?.[0]?.toUpperCase() ?? "";
|
||||
return (
|
||||
<div
|
||||
className={`bg-card grid place-items-center font-semibold text-muted select-none ${className}`}
|
||||
aria-hidden
|
||||
>
|
||||
{initial}
|
||||
</div>
|
||||
<RemoteImage
|
||||
src={src}
|
||||
alt={alt}
|
||||
loading="lazy"
|
||||
className={className}
|
||||
fallback={
|
||||
<div
|
||||
className={`bg-card grid place-items-center font-semibold text-muted select-none ${className}`}
|
||||
aria-hidden
|
||||
>
|
||||
{initial}
|
||||
</div>
|
||||
}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -71,6 +71,7 @@ function Page({ withMenu = true }: { withMenu?: boolean }) {
|
||||
>
|
||||
<span data-testid="row-inner" />
|
||||
</div>
|
||||
{/* eslint-disable-next-line no-restricted-syntax -- the native element whose browser menu is kept */}
|
||||
<img data-testid="img" alt="" />
|
||||
</div>
|
||||
</PageShell>
|
||||
|
||||
@@ -39,7 +39,7 @@ Counter-examples, deliberately NOT here: `PosterTile` (knows about Plex montage
|
||||
| ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| overlays & anchoring | `AnchoredPopover` + `anchoredPopoverHooks` (the popover engine: arrows, typeahead, Escape, anchor-flip), `ActionMenu`, `MembershipMenu` (the body of a "which sets is it in" menu: checkbox rows + a field naming a new set), `ContextMenu` + `useContextMenu` (the SAME menu, opened by right-click / long-press / Shift+F10 / a row's `MoreButton` "…" via `buttonProps`; `anchor` for a panel a row opens; `PageMenu` for a page's background menu), `Modal` (+ `ModalFooter`; `initialFocus` by purpose, C4), `Overlay`, `SidePanel`, `Tooltip`, `Toaster` (+ `toastLevels`, `overlayOrder`; `lib/toastDock` docks one control under the stack, in every host), `CommandPalette`, `KeyboardHelp` |
|
||||
| actions & inputs | `Button`, `IconButton`, `Pressable` (`selectable` when its text is a title or a name), `Chip` + `chipStyles`, `DragHandle` + `OrderButtons` / `useOrderItems` (the non-drag reorder, WCAG 2.5.7) (one per button family, docs/DESIGN.md; a raw `<button>` or a bare `FocusSafeButton` outside the control primitives — eslint `BUTTON_PRIMITIVES` — is a lint error), `Input` + `fieldStyles`, `RenameField` (a name typed in place of the name: Enter or blur saves, Escape cancels; the caller hands the keyboard back), `Meter` (how full something is against a limit — one danger line for every place showing the same measure), `TimecodeField` + `useTimecodeField` / `TimecodeError` (a cut's time typed in: the draft, held by the caller, until Enter or blur takes it, then the value as it landed; unreadable stays once offered, marked invalid, and the caller places the line saying how to type one in its flow, outside a label; a draft goes when the value moves under it), `NativeControls`, `MenuCheck` (the drawn check of a menu row or a whole-row checkbox button; its tick is the `chosen` colour, which `media-marks` turns amber on the player chrome), `mediaChrome` (`MEDIA_CHOSEN`, a chosen tab or toggle on the chrome), `SelectTrigger` (the field family; a raw `<input>` is a lint error), `useReturnFocus`, `surfaces` (the B5 glass roles; a bare glass class is a lint error), `Badge` (a static label), `LinkButton` / `IconLink` (a link wearing the button face) over `buttonStyles`, `Skeleton` (B8 first loads, through `LoadingState shape`), `NavRail` (B7; `NavRailGroup` folds a set of rare pages into one row with a panel), `form`, `DraftSaveBar`, `Combobox`, `FilterSelect`, `SegmentedToggle`, `SortControl`, `ColumnSortControl`, `PriorityStepper`, `UndoToolbar` |
|
||||
| data display | `DataTable` (+ `progressiveRows`: a long page mounts in steps), `tableHeader` + `tableAlign`, `HiddenFilterCue` (names a filter whose column is off screen), `Pager` + `pagerContent`, `VirtualGrid` + `gridColumns` (the column/width maths, incl. the ultrawide `maxCols` cap), `readAhead`, `Carousel`, `MediaCard` + `mediaCardStyles` (the one media card, docs/DESIGN.md "Media card") + `MediaPreview` / `useMediaPreview` (its mouse-rest preview), `NavGrid` (a plain grid, list or table body the keyboard walks — `DataTable`, `VirtualGrid` and `Carousel` take `keyboardNav`), `Avatar`, `CountBadge`, `ItemCount`, `FilterChipGroup`, `RelativeTime` ("3d ago" with the exact minute on hover, docs/DESIGN.md "Copy"), `ClampedText` (a long text clamped to a few lines; More / Less only when it is cut), `StatTiles` + `StatTile` / `StatBars` (number tiles, a breakdown as horizontal bars), `RangeTrack` + `RulerTicks` (a time track over a window of a media item: regions, slider ends the arrows step, a playhead, a background slot — the editor's cuts, the players' clip panel), `CropFrame` (a crop frame over a picture, on the picture and not its bars: drag, edges, corners, the arrows; the editor's crop, the players' clip crop) |
|
||||
| data display | `DataTable` (+ `progressiveRows`: a long page mounts in steps), `tableHeader` + `tableAlign`, `HiddenFilterCue` (names a filter whose column is off screen), `Pager` + `pagerContent`, `VirtualGrid` + `gridColumns` (the column/width maths, incl. the ultrawide `maxCols` cap), `readAhead`, `Carousel`, `MediaCard` + `mediaCardStyles` (the one media card, docs/DESIGN.md "Media card") + `MediaPreview` / `useMediaPreview` (its mouse-rest preview), `NavGrid` (a plain grid, list or table body the keyboard walks — `DataTable`, `VirtualGrid` and `Carousel` take `keyboardNav`), `RemoteImage` (every network picture: no Referer, retries past the negative cache, a fallback instead of the broken-image mark — a bare `<img>` is linted out), `Avatar`, `CountBadge`, `ItemCount`, `FilterChipGroup`, `RelativeTime` ("3d ago" with the exact minute on hover, docs/DESIGN.md "Copy"), `ClampedText` (a long text clamped to a few lines; More / Less only when it is cut), `StatTiles` + `StatTile` / `StatBars` (number tiles, a breakdown as horizontal bars), `RangeTrack` + `RulerTicks` (a time track over a window of a media item: regions, slider ends the arrows step, a playhead, a background slot — the editor's cuts, the players' clip panel), `CropFrame` (a crop frame over a picture, on the picture and not its bars: drag, edges, corners, the arrows; the editor's crop, the players' clip crop) |
|
||||
| page & panel structure | `PageShell` + `pageToolbarSlot` + `pageHeading`, `PageScroller`, `Headings` (`PageHeading`, `SectionLabel`), `SkipLink` + `landmarks`, `FilterFold` (B4), `scrollFold` (`useScrollFold`: a fixed header that folds once its content scrolls), `PanelGroup`, `PanelGroups`, `Tabs`, `BackToTop` |
|
||||
| state & messaging | `QueryState`, `Banner`, `InlineNotice`, `CrashBoundary` |
|
||||
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
// @vitest-environment jsdom
|
||||
import { describe, it, expect, afterEach, beforeEach, vi } from "vitest";
|
||||
import { render, screen, fireEvent, cleanup, act } from "@testing-library/react";
|
||||
|
||||
import RemoteImage from "./RemoteImage";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
const img = () => screen.queryByRole("img", { name: "pic" });
|
||||
const wait = (ms: number) =>
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(ms);
|
||||
});
|
||||
const fallback = <span>no picture</span>;
|
||||
|
||||
describe("RemoteImage", () => {
|
||||
// Google's image CDNs refuse a request carrying our origin's Referer — and the card then showed
|
||||
// the browser's broken-image mark (a Home shelf card, 2026-10-14).
|
||||
it("asks without a Referer", () => {
|
||||
render(<RemoteImage src="https://cdn.test/a.jpg" alt="pic" />);
|
||||
expect(img()?.getAttribute("referrerpolicy")).toBe("no-referrer");
|
||||
});
|
||||
|
||||
it("shows the fallback, never the broken image, while a retry waits — then asks again past the cache", async () => {
|
||||
render(<RemoteImage src="https://cdn.test/a.jpg?s=1" alt="pic" fallback={fallback} />);
|
||||
fireEvent.error(img()!);
|
||||
expect(img()).toBeNull();
|
||||
expect(screen.getByText("no picture")).toBeTruthy();
|
||||
|
||||
await wait(999);
|
||||
expect(img()).toBeNull();
|
||||
await wait(1);
|
||||
// A fresh request: the browser negative-caches the failed URL, so the same one would only replay it.
|
||||
expect(img()?.getAttribute("src")).toBe("https://cdn.test/a.jpg?s=1&r=1");
|
||||
});
|
||||
|
||||
it("waits longer each time, and keeps the fallback once every retry is spent", async () => {
|
||||
render(<RemoteImage src="https://cdn.test/a.jpg" alt="pic" fallback={fallback} />);
|
||||
for (const [after, n] of [
|
||||
[1000, 1],
|
||||
[2000, 2],
|
||||
[4000, 3],
|
||||
] as const) {
|
||||
fireEvent.error(img()!);
|
||||
await wait(after - 1);
|
||||
expect(img()).toBeNull();
|
||||
await wait(1);
|
||||
expect(img()?.getAttribute("src")).toBe(`https://cdn.test/a.jpg?r=${n}`);
|
||||
}
|
||||
fireEvent.error(img()!);
|
||||
await wait(60_000);
|
||||
expect(img()).toBeNull();
|
||||
expect(screen.getByText("no picture")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("a loaded retry stays", async () => {
|
||||
render(<RemoteImage src="https://cdn.test/a.jpg" alt="pic" />);
|
||||
fireEvent.error(img()!);
|
||||
await wait(1000);
|
||||
fireEvent.load(img()!);
|
||||
await wait(60_000);
|
||||
expect(img()?.getAttribute("src")).toBe("https://cdn.test/a.jpg?r=1");
|
||||
});
|
||||
|
||||
it("busts every candidate of a srcset", async () => {
|
||||
render(
|
||||
<RemoteImage
|
||||
src="/api/plex/image/7"
|
||||
srcSet="/api/plex/image/7?variant=thumb_sm 200w, /api/plex/image/7 400w"
|
||||
alt="pic"
|
||||
/>,
|
||||
);
|
||||
fireEvent.error(img()!);
|
||||
await wait(1000);
|
||||
expect(img()?.getAttribute("srcset")).toBe(
|
||||
"/api/plex/image/7?variant=thumb_sm&r=1 200w, /api/plex/image/7?r=1 400w",
|
||||
);
|
||||
});
|
||||
|
||||
it("with no retries, a failure is the fallback at once", () => {
|
||||
render(<RemoteImage src="/api/files/1/preview" alt="pic" retries={0} fallback={fallback} />);
|
||||
fireEvent.error(img()!);
|
||||
expect(screen.getByText("no picture")).toBeTruthy();
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(60_000);
|
||||
});
|
||||
expect(img()).toBeNull();
|
||||
});
|
||||
|
||||
it("starts over for a new src, and gives no src the fallback", () => {
|
||||
const { rerender } = render(
|
||||
<RemoteImage src="https://cdn.test/a.jpg" alt="pic" retries={0} fallback={fallback} />,
|
||||
);
|
||||
fireEvent.error(img()!);
|
||||
rerender(
|
||||
<RemoteImage src="https://cdn.test/b.jpg" alt="pic" retries={0} fallback={fallback} />,
|
||||
);
|
||||
expect(img()?.getAttribute("src")).toBe("https://cdn.test/b.jpg");
|
||||
rerender(<RemoteImage src={null} alt="pic" fallback={fallback} />);
|
||||
expect(img()).toBeNull();
|
||||
expect(screen.getByText("no picture")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("does not bust an inline picture", async () => {
|
||||
render(<RemoteImage src="data:image/png;base64,AAAA" alt="pic" />);
|
||||
fireEvent.error(img()!);
|
||||
await wait(1000);
|
||||
expect(img()?.getAttribute("src")).toBe("data:image/png;base64,AAAA");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,91 @@
|
||||
import { useEffect, useState, type ImgHTMLAttributes, type ReactNode } from "react";
|
||||
|
||||
// Every picture the app loads over the network goes through here (lint-enforced: a bare <img> is
|
||||
// refused everywhere else), because a picture that fails left the browser's broken-image mark on a
|
||||
// card, and each component that noticed had hand-rolled its own cure. What they learned, in one place:
|
||||
//
|
||||
// - No Referer. Google's image CDNs (yt3.ggpht.com, lh3.googleusercontent.com) refuse a request that
|
||||
// carries one from our origin — not only in a burst (measured 2026-09-26). Our own endpoints never
|
||||
// read it, so it is dropped for every picture rather than guessed per host.
|
||||
// - A failed load is retried, with a cache-buster: the browser NEGATIVE-CACHES the miss, so asking for
|
||||
// the same URL again would only replay the failure. Both Google CDNs and our API ignore the extra
|
||||
// `r` parameter (measured 2026-10-14). A 429 from a burst of cards heals this way.
|
||||
// - While there is no picture — none given, a retry still waiting, every retry spent — the caller's
|
||||
// `fallback` stands in (nothing by default: the card's own empty frame, which is what a card with no
|
||||
// thumbnail has always shown). Never the broken-image mark.
|
||||
// - The failure is remembered BY URL, in render: these sit in virtualized grids whose slots are recycled,
|
||||
// and a failure stored as a flag an effect resets painted one item's fallback over the next one's
|
||||
// picture for a frame (ui/Avatar had that defect).
|
||||
|
||||
/** How many times a failed picture is asked for again, and the wait before the first retry (it
|
||||
* doubles each time): 1 s, 2 s, 4 s, then the fallback for good. */
|
||||
const IMAGE_RETRIES = 3;
|
||||
const FIRST_RETRY_MS = 1000;
|
||||
|
||||
/** `url` with a cache-buster for retry `attempt` (0 = the URL as given). An inline `data:`/`blob:` URL
|
||||
* is not fetched, so it has no cache to bust. */
|
||||
function bustImageUrl(url: string, attempt: number): string {
|
||||
if (attempt === 0 || /^(data|blob):/.test(url)) return url;
|
||||
return `${url}${url.includes("?") ? "&" : "?"}r=${attempt}`;
|
||||
}
|
||||
|
||||
/** The same for every candidate of a `srcset` ("url 200w, url2 400w"). */
|
||||
function bustSrcSet(srcSet: string, attempt: number): string {
|
||||
if (attempt === 0) return srcSet;
|
||||
return srcSet
|
||||
.split(",")
|
||||
.map((part) => {
|
||||
const [url = "", ...descriptor] = part.trim().split(/\s+/);
|
||||
return [bustImageUrl(url, attempt), ...descriptor].join(" ");
|
||||
})
|
||||
.join(", ");
|
||||
}
|
||||
|
||||
type Miss = { src: string; attempt: number; waiting: boolean };
|
||||
|
||||
export default function RemoteImage({
|
||||
src,
|
||||
srcSet,
|
||||
fallback = null,
|
||||
retries = IMAGE_RETRIES,
|
||||
...img
|
||||
}: Omit<ImgHTMLAttributes<HTMLImageElement>, "src" | "srcSet" | "onError" | "referrerPolicy"> & {
|
||||
src: string | null | undefined;
|
||||
srcSet?: string;
|
||||
/** What stands in while there is no picture. */
|
||||
fallback?: ReactNode;
|
||||
/** Fewer retries for a picture whose failure is an answer, not a hiccup — a file that is not there
|
||||
* yet is better shown as its fallback now than after seven seconds of retries. */
|
||||
retries?: number;
|
||||
}) {
|
||||
const [miss, setMiss] = useState<Miss | null>(null);
|
||||
// Forgotten once `src` moves off it — convergent, so it belongs in render (see above).
|
||||
if (miss !== null && miss.src !== src) setMiss(null);
|
||||
const current = miss !== null && miss.src === src ? miss : null;
|
||||
const attempt = current?.attempt ?? 0;
|
||||
|
||||
const waiting = current?.waiting === true;
|
||||
useEffect(() => {
|
||||
if (!waiting) return;
|
||||
const timer = window.setTimeout(
|
||||
() => setMiss((m) => (m !== null && m.src === src ? { ...m, waiting: false } : m)),
|
||||
FIRST_RETRY_MS * 2 ** (attempt - 1),
|
||||
);
|
||||
return () => window.clearTimeout(timer);
|
||||
}, [waiting, attempt, src]);
|
||||
|
||||
const spent = current !== null && !current.waiting && current.attempt > retries;
|
||||
if (!src || waiting || spent) return <>{fallback}</>;
|
||||
return (
|
||||
// eslint-disable-next-line no-restricted-syntax -- the one <img> the rest of the app goes through
|
||||
<img
|
||||
{...img}
|
||||
// A fresh element per attempt: the failed one is done, and a new src on it would not refire.
|
||||
key={attempt}
|
||||
src={bustImageUrl(src, attempt)}
|
||||
srcSet={srcSet === undefined ? undefined : bustSrcSet(srcSet, attempt)}
|
||||
referrerPolicy="no-referrer"
|
||||
onError={() => setMiss({ src, attempt: attempt + 1, waiting: attempt < retries })}
|
||||
/>
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user