fix(nav): Back looks for a moved item only in the page's lists, once its own list has had its chance, and its reveal stops the scroll restore

The cross-list match took a same-named row of the app-wide download tray, and took it at the first look while a virtualized list had yet to render the item. The reveal of a moved item ran before the scroll restore's first frame, which put the old offset back over it. The pending restore's name is readable, for a page that folds rows away.

frontend/src/components/ui/PageScroller.dom.test.tsx

@@ -17,6 +17,7 @@ vi.mock("react-i18next", async (importOriginal) => {
 import PageScroller from "./PageScroller";
 import BackToTop from "./BackToTop";
 import { notifyNav } from "../../lib/history";
+import { revealedNavItem } from "../../lib/listNav";
 
 beforeAll(() => {
   // useScrollFade wires a ResizeObserver (watch-the-children); jsdom ships none.
@@ -125,6 +126,38 @@ describe("PageScroller scroll memory", () => {
     expect(main.scrollTop).toBe(450);
   });
 
+  // Back focuses an item the restore did not expect — one that moved to another list while the page
+  // was away — and scrolls it into view: the restore's next frame put the old offset back over it.
+  it("gives the page up to an item brought into view while it restores", () => {
+    let frames: FrameRequestCallback[] = [];
+    vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => frames.push(cb));
+    const flush = (n: number) => {
+      for (let i = 0; i < n; i++) {
+        const run = frames;
+        frames = [];
+        for (const cb of run) cb(0);
+      }
+    };
+    setEntry(1);
+    const { container } = render(
+      <PageScroller scrollKey="e:k1">
+        <div id="moved" />
+      </PageScroller>,
+    );
+    const main = sizedMain(container);
+    main.scrollTop = 450;
+    fireEvent.scroll(main);
+    setEntry(2);
+    notifyNav();
+    flush(1);
+    setEntry(1);
+    notifyNav(); // Back: the restore to 450 waits for its first frame
+    main.scrollTop = 120; // the item scrolled into view
+    revealedNavItem(container.querySelector("#moved")!);
+    flush(5);
+    expect(main.scrollTop).toBe(120);
+  });
+
   // Landing at the end shows Back to top, whose spacer then brings the spot within reach.
   it("finishes the way to a spot the spacer brings within reach after it lands", () => {
     let frames: FrameRequestCallback[] = [];

frontend/src/components/ui/PageScroller.tsx

@@ -12,6 +12,7 @@ import { StickyTopProvider } from "./stickyTop";
 import { useBackToTopShown, useCornerLift } from "./backToTopState";
 import { CONTENT_ID } from "./landmarks";
 import { ensureNavId, peekNavId, subscribeNav } from "../../lib/history";
+import { NAV_REVEAL_EVENT } from "../../lib/listNav";
 
 // Saved scroll offsets, keyed by `${historyEntryId}:${scrollKey}`. The entry id ties an offset to the
 // exact history entry, so Back restores where you left it and a forward navigation (a new entry) opens
@@ -63,11 +64,15 @@ function restoreScroll(el: HTMLElement, top: number): () => void {
     el.removeEventListener("wheel", stop);
     el.removeEventListener("touchstart", stop);
     el.removeEventListener("pointerdown", stop);
+    el.removeEventListener(NAV_REVEAL_EVENT, stop);
     document.removeEventListener("keydown", stop, true);
   };
   el.addEventListener("wheel", stop, { passive: true });
   el.addEventListener("touchstart", stop, { passive: true });
   el.addEventListener("pointerdown", stop);
+  // So does an item the keyboard brought into view (`lib/listNav`): Back focusing one that is not
+  // where the restored offset shows.
+  el.addEventListener(NAV_REVEAL_EVENT, stop);
   document.addEventListener("keydown", stop, true);
   const tick = () => {
     if (cancelled) return;

frontend/src/lib/listNav.restoreMoved.dom.test.tsx

@@ -9,13 +9,20 @@ import { afterEach, describe, expect, it, vi } from "vitest";
 import { act, cleanup, render } from "@testing-library/react";
 import { useRef } from "react";
 
-import { navItemAttrs, navPosition, restoreNavPosition, useListNav } from "./listNav";
+import {
+  NAV_REVEAL_EVENT,
+  navItemAttrs,
+  navPosition,
+  restoreNavPosition,
+  useListNav,
+} from "./listNav";
 
 afterEach(cleanup);
 
-function List({ items }: { items: string[] }) {
+// `count`: a virtualized list's, which counts rows it has yet to render.
+function List({ items, count }: { items: string[]; count?: number }) {
   const ref = useRef<HTMLDivElement>(null);
-  useListNav(ref, { enabled: true });
+  useListNav(ref, { enabled: true, count });
   return (
     <div ref={ref}>
       {items.map((t, i) => (
@@ -27,12 +34,25 @@ function List({ items }: { items: string[] }) {
   );
 }
 
-const Page = ({ lists }: { lists: string[][] }) => (
-  <main>
-    {lists.map((items, i) => (
-      <List key={i} items={items} />
-    ))}
-  </main>
+// `tray`: a list of the app's own, outside the page (the running downloads), whose rows can share a
+// name with the page's.
+const Page = ({
+  lists,
+  tray,
+  counts = [],
+}: {
+  lists: string[][];
+  tray?: string[];
+  counts?: number[];
+}) => (
+  <>
+    <main>
+      {lists.map((items, i) => (
+        <List key={i} items={items} count={counts[i]} />
+      ))}
+    </main>
+    {tray && <List items={tray} />}
+  </>
 );
 
 const button = (name: string) =>
@@ -40,12 +60,17 @@ const button = (name: string) =>
 const focused = () => document.activeElement?.closest("[data-nav-index]")?.textContent;
 const wait = (ms: number) => act(() => new Promise((r) => setTimeout(r, ms)));
 
-async function leaveAndReturn(before: string[][], from: string, after: string[][]) {
-  const { unmount } = render(<Page lists={before} />);
+async function leaveAndReturn(
+  before: string[][],
+  from: string,
+  after: string[][],
+  tray?: string[],
+) {
+  const { unmount } = render(<Page lists={before} tray={tray} />);
   button(from).focus();
   const pos = navPosition()!;
   unmount(); // the page switch
-  render(<Page lists={after} />); // Back
+  render(<Page lists={after} tray={tray} />); // Back
   const cancel = restoreNavPosition(pos);
   await wait(120);
   cancel();
@@ -78,15 +103,51 @@ describe("Back to an item that moved to another list", () => {
     // off screen.
     const reveal = vi.fn();
     Element.prototype.scrollIntoView = reveal;
+    // ...and tells the page's scroll restore, which would otherwise put the old offset back over it.
+    const told = vi.fn();
+    document.addEventListener(NAV_REVEAL_EVENT, told);
     await leaveAndReturn([["subscribed", "download"], ["older"]], "subscribed", [
       ["download"],
       ["subscribed", "older"],
     ]);
     expect(reveal).toHaveBeenCalledTimes(1);
+    expect(told).toHaveBeenCalledTimes(1);
     cleanup();
     reveal.mockClear();
+    told.mockClear();
     await leaveAndReturn([["subscribed", "download"]], "download", [["download", "subscribed"]]);
     expect(focused()).toBe("download");
     expect(reveal).not.toHaveBeenCalled();
+    expect(told).not.toHaveBeenCalled();
+    document.removeEventListener(NAV_REVEAL_EVENT, told);
+  });
+
+  it("never takes it from a list outside the page: the download tray's row of that name", async () => {
+    // A feed card and its download in the tray are both named by the video's title.
+    await leaveAndReturn(
+      [["subscribed", "download"]],
+      "subscribed",
+      [["download"]],
+      ["subscribed"],
+    );
+    expect(focused()).toBe("download");
+  });
+
+  it("waits for its own list to render the place before looking in another one", async () => {
+    // A virtualized list puts its rows back a frame after the page: the item is still to come at
+    // the first look, and the one of that name in the rail below is not it.
+    const { unmount } = render(<Page lists={[["older", "subscribed"], ["subscribed-2"]]} />);
+    button("subscribed").focus();
+    const pos = navPosition()!;
+    unmount();
+    const { rerender } = render(<Page lists={[["older"], ["subscribed"]]} counts={[2]} />);
+    const cancel = restoreNavPosition(pos);
+    await wait(60);
+    rerender(<Page lists={[["older", "subscribed"], ["subscribed"]]} counts={[2]} />);
+    await wait(120);
+    cancel();
+    expect(document.activeElement?.closest("main > div")).toBe(
+      document.querySelector("main")!.firstElementChild,
+    );
   });
 });

frontend/src/lib/listNav.ts

@@ -226,6 +226,14 @@ function itemName(el: HTMLElement): string {
 /** What a screen reader hears while the item holds the focus: its `label` when it gives one. */
 const spokenName = (el: HTMLElement): string => el.getAttribute("data-nav-label") || itemName(el);
 
+/** The event an item brought into view sends up its page: a scroll restore still under way gives the
+ *  page up to it (`ui/PageScroller`) — on Back, an item found in another list than the one it was
+ *  left in is not where the restored offset shows, and the restore's next frame scrolled it away. */
+export const NAV_REVEAL_EVENT = "navreveal";
+export const revealedNavItem = (el: HTMLElement): void => {
+  el.dispatchEvent(new Event(NAV_REVEAL_EVENT, { bubbles: true }));
+};
+
 function focusItem(el: HTMLElement, reveal = true): void {
   if (roved !== el) {
     if (named) roved?.removeAttribute("role");
@@ -259,7 +267,10 @@ function focusItem(el: HTMLElement, reveal = true): void {
   el.focus({ preventScroll: true });
   // `nearest` + the item's scroll margins: a visible item does not move, an item under the floating
   // header or past the bottom edge scrolls just far enough to clear it.
-  if (reveal) el.scrollIntoView?.({ block: "nearest" });
+  if (reveal) {
+    el.scrollIntoView?.({ block: "nearest" });
+    revealedNavItem(el);
+  }
 }
 
 function focusIndex(
@@ -483,17 +494,24 @@ export function navPosition(): NavPosition | null {
 // look — on a timer, not animation frames: a hidden tab runs none.
 const RESTORE_MS = 2000;
 const RESTORE_POLL_MS = 50;
+// The restore under way, while it looks for its item.
+let pending: NavPosition | null = null;
 
 /** The item to hand the focus back to: in `list`, the saved place while it still holds the saved
  *  name, else the nearest item of that name (names repeat — a log console's identical request
  *  lines). Then the one item of that name in another of the page's lists: an item can change lists
  *  while the page was away (a notification asked for the reader on the visit it was opened from and
  *  sits in the stream on the next), and the place left behind is now its neighbour's. Only when the
- *  name is the page's alone — of two, either could be the one. Else the saved place. */
+ *  name is the page's alone — of two, either could be the one — and only the PAGE's lists: the
+ *  running-downloads tray is the app's, on every page, and its row of a download is named like the
+ *  feed card it came from. Not while `list` has yet to render a saved place it still counts (a
+ *  virtualized list puts its rows back a frame after the page), until the `last` look: the item is
+ *  likely still to come there. Else the saved place. */
 function restoreTarget(
   all: readonly NavList[],
   list: NavList | undefined,
   pos: NavPosition,
+  last: boolean,
 ): HTMLElement | null {
   const at = list ? itemAt(list, pos.index) : null;
   if (list && (!pos.name || (at && itemName(at) === pos.name))) return at;
@@ -504,7 +522,8 @@ function restoreTarget(
     const here = named(list).sort((a, b) => distance(a) - distance(b));
     if (here[0]) return here[0];
   }
-  const [only, ...more] = all.flatMap(named);
+  if (list && !at && pos.index < list.count() && !last) return null;
+  const [only, ...more] = all.filter((l) => l.el.closest("main")).flatMap(named);
   return only && more.length === 0 ? only : at;
 }
 
@@ -522,8 +541,12 @@ function restoreTarget(
 export function restoreNavPosition(pos: NavPosition): () => void {
   const deadline = Date.now() + RESTORE_MS;
   let timer: ReturnType<typeof setTimeout> | undefined;
+  pending = pos;
+  const done = () => {
+    if (pending === pos) pending = null;
+  };
   const attempt = () => {
-    if (!focusIsUnclaimed()) return;
+    if (!focusIsUnclaimed()) return done();
     const all = ordered();
     const list = pos.label
       ? all.find((l) => labelOf(l) === pos.label)
@@ -536,20 +559,33 @@ export function restoreNavPosition(pos: NavPosition): () => void {
     // The saved place still holding the saved name wins, and among several items of that name the
     // nearest one: names repeat (a log console's `http.request GET /api/feed 200` lines, two
     // downloads of one video), and the first of them is rarely the one left behind.
-    const el = restoreTarget(all, list, pos);
+    const last = Date.now() + RESTORE_POLL_MS >= deadline;
+    const el = restoreTarget(all, list, pos, last);
     // Where the scroll restore put it — unless it is found in another list than the one it was left
     // in: the restored scroll is the old place's, and the item would hold the focus off screen.
-    if (el) return focusItem(el, !list?.el.contains(el));
-    if (Date.now() < deadline) timer = setTimeout(attempt, RESTORE_POLL_MS);
+    if (el) {
+      done();
+      return focusItem(el, !list?.el.contains(el));
+    }
+    if (last) done();
+    else timer = setTimeout(attempt, RESTORE_POLL_MS);
   };
   // Not inside the popstate dispatch that landed: a sub-view's own listener (`useHistorySubview`)
   // may run after ours and has not switched the view yet — focus still sits in the view being left,
   // and "focus is already somewhere" would give up for good (measured in Chrome:
   // Plex show → Back left focus on <body>). One task later the view has rendered.
   timer = setTimeout(attempt, 0);
-  return () => clearTimeout(timer);
+  return () => {
+    clearTimeout(timer);
+    done();
+  };
 }
 
+/** The name of the item a Back is still looking for, while it looks: a page that keeps some of its
+ *  rows folded away (the notification center's Earlier) opens the fold holding it — a list it does
+ *  not render is one the restore cannot find the item in. Read it as the page mounts. */
+export const pendingNavName = (): string | null => pending?.name ?? null;
+
 /** Register `ref` as a keyboard-navigable list while `enabled`. A virtualized list must say how many
  *  items it has (`count` — the loaded ones, for an infinite list), how many per row (`cols`) and how
  *  to scroll an unmounted item into view (`reveal`). A list that mounts everything omits all three: