Every one of them is the same shape: a value derived correctly, and a WRITE to the thing it derives from left unattended. Deriving hides a stale value; it does not consume it. PlayerModal — the watch toggle inherited the previous video's auto-watch. Moving the queue-advance sync into render was right for `currentVideoId`, but the same block also synced `status`, and that one had an ordering constraint of its own: useYtProgress's cleanup for the OUTGOING item can still call setWatched(true) near the end of a clip, and cleanups run after the commit. As an effect the sync landed last and corrected it; from render it lands first and the stray write wins. Press Next in the final ten seconds and the NEXT video showed as watched — one click on the toggle then sent onState(next, "new") for an item already new. Fixed by keying the mirror to the id it describes, so a write for another item is ignored rather than displayed, and no ordering has to be argued about at all. ChannelsProvider — a focus-channel intent was masked by the page rather than consumed, and nothing ever cleared it. The manager is a page chunk: it unmounts on every navigation, and its seed effect runs again on each mount, so returning to it re-applied a channel name from earlier in the session over whatever the user had typed since. The consumed marker now lives in the provider, keyed on the token — which also closes the case the old clear-on-leave effect never handled, opening a channel page (that does not change `page` at all). TagFilterChips + ChannelTags x2 — `open = openRaw && hidden > 0` hides the overflow popover when the lane stops overflowing, but floating-ui only reports a close for a real event on a mounted surface, so the raw flag stayed true and the menu came back by itself on the next re-fit — a window resize or the text-size slider is enough, with nobody near it. The flag is now reset in render against the same condition that hides it. Avatar + downloadRow — remembering a failed image BY URL fixed the recycled-row frame but never forgot it, so a transient 429 pinned a channel to its initial for the life of the instance, never re-requesting. The memory is now dropped once `src` moves off it, which keeps both properties. And playerModalHooks' last exhaustive-deps suppression claimed "nothing in the body reads a render value" while reading five. The invariant that is actually load-bearing is that each of them is a per-item snapshot — written down instead. All four fixes are mutation-checked: each new test was seen red, against the fixed line only, naming the real defect.
404 lines
16 KiB
TypeScript
404 lines
16 KiB
TypeScript
// @vitest-environment jsdom
|
|
//
|
|
// The YouTube player modal, rendered for real. Its pure helpers (afterRemoval, the progress cache)
|
|
// had tests; what the modal DOES with them had none — a deep-review of the 2026-09 bug hunt found
|
|
// that deleting the lines that apply them left every lane green.
|
|
//
|
|
// `window.YT` is a fake installed before the module's API loader first runs (that loader memoises
|
|
// its promise in a module-level singleton, so this file must not share a module instance with a
|
|
// test that leaves `window.YT` unset). Each item the modal plays constructs a new YT.Player, and the
|
|
// fake records what it was asked for — that is how these tests see where playback starts.
|
|
import { describe, it, expect, vi, afterEach, beforeAll, beforeEach } from "vitest";
|
|
import { render, screen, fireEvent, waitFor, cleanup, act } from "@testing-library/react";
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
import type { ReactNode } from "react";
|
|
|
|
vi.mock("react-i18next", async (importOriginal) => {
|
|
const actual = await importOriginal<typeof import("react-i18next")>();
|
|
return {
|
|
...actual,
|
|
useTranslation: () => ({
|
|
t: (key: string, o?: Record<string, unknown>) => (o?.title ? `${key}:${o.title}` : key),
|
|
i18n: { language: "en" },
|
|
}),
|
|
};
|
|
});
|
|
vi.mock("./navigationContext", () => ({ useNavigationActions: () => ({ openChannel: vi.fn() }) }));
|
|
// Their own queries and popovers are not what is under test here.
|
|
vi.mock("./AddToPlaylist", () => ({ default: () => null }));
|
|
vi.mock("./DownloadButton", () => ({ default: () => null }));
|
|
|
|
import PlayerModal from "./PlayerModal";
|
|
import { ConfirmProvider } from "./ConfirmProvider";
|
|
import { useConfirm } from "./confirmContext";
|
|
import { api, type Video } from "../lib/api";
|
|
import { clearAll, notify } from "../lib/notifications";
|
|
|
|
beforeAll(() => {
|
|
class Stub {
|
|
observe() {}
|
|
unobserve() {}
|
|
disconnect() {}
|
|
takeRecords() {
|
|
return [];
|
|
}
|
|
}
|
|
globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver;
|
|
globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver;
|
|
});
|
|
|
|
// --- the fake IFrame API ------------------------------------------------------------------------
|
|
const clock = { current: 0, duration: 3000 };
|
|
const started: { videoId: string; start: number | undefined }[] = [];
|
|
const base = {
|
|
getCurrentTime: () => clock.current,
|
|
getDuration: () => clock.duration,
|
|
getVideoData: () => null,
|
|
};
|
|
// Anything else the modal calls on the player (volume, rate, destroy…) is a no-op. Not `then`:
|
|
// a thenable would be swallowed by the first `await` that touched it.
|
|
const fakePlayer = new Proxy(base, {
|
|
get: (t, k) => (k in t ? t[k as keyof typeof t] : k === "then" ? undefined : () => undefined),
|
|
});
|
|
window.YT = {
|
|
Player: function (_el: unknown, opts: { videoId: string; playerVars: { start?: number } }) {
|
|
started.push({ videoId: opts.videoId, start: opts.playerVars.start });
|
|
return fakePlayer;
|
|
} as never,
|
|
};
|
|
|
|
const vid = (id: string, position_seconds: number): Video =>
|
|
({
|
|
id,
|
|
title: `Video ${id}`,
|
|
channel_id: "UCx",
|
|
channel_title: "Channel",
|
|
status: "new",
|
|
position_seconds,
|
|
duration_seconds: 3000,
|
|
watch_url: `https://www.youtube.com/watch?v=${id}`,
|
|
}) as unknown as Video;
|
|
|
|
function renderModal(ui: ReactNode) {
|
|
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
|
return render(<QueryClientProvider client={qc}>{ui}</QueryClientProvider>);
|
|
}
|
|
|
|
afterEach(() => {
|
|
cleanup();
|
|
vi.restoreAllMocks();
|
|
started.length = 0;
|
|
});
|
|
|
|
const lastStart = () => started[started.length - 1];
|
|
|
|
describe("PlayerModal — where each item starts", () => {
|
|
it("resumes an item played earlier in the SAME session where this session left it", async () => {
|
|
// The launch queue is frozen at mount, so its rows keep the position they were snapshotted
|
|
// with (748). Stepping next→prev back to an item already watched to 830 used to start it at
|
|
// 748 — and the next checkpoint then wrote 748 over the 830 the server held.
|
|
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
|
|
vi.spyOn(api, "saveProgress").mockImplementation(async (id: string) => ({
|
|
video_id: id,
|
|
position_seconds: id === "A" ? 830 : 0,
|
|
}));
|
|
const A = vid("A", 748);
|
|
const B = vid("B", 0);
|
|
clock.current = 830;
|
|
renderModal(
|
|
<PlayerModal video={A} queue={[A, B]} startIndex={0} onClose={() => {}} onState={() => {}} />,
|
|
);
|
|
await waitFor(() => expect(lastStart()).toEqual({ videoId: "A", start: 748 }));
|
|
fireEvent.click(screen.getAllByRole("button", { name: "player.next" })[0]!);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
|
fireEvent.click(screen.getAllByRole("button", { name: "player.previous" })[0]!);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
expect(lastStart()?.start).toBe(830);
|
|
});
|
|
});
|
|
|
|
// --- "remove from this playlist", from inside the player ----------------------------------------
|
|
const trash = (title: string) =>
|
|
screen.getByRole("button", { name: `playlists.removeItemOf:Video ${title}` });
|
|
const next = () => fireEvent.click(screen.getAllByRole("button", { name: "player.next" })[0]!);
|
|
const prev = () => fireEvent.click(screen.getAllByRole("button", { name: "player.previous" })[0]!);
|
|
const settle = () => act(async () => {});
|
|
|
|
describe("PlayerModal — removing the playing video from its playlist", () => {
|
|
const [A, B, C, D] = ["A", "B", "C", "D"].map((id) => vid(id, 0));
|
|
beforeEach(() => {
|
|
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
|
|
vi.spyOn(api, "saveProgress").mockImplementation(async (id: string) => ({
|
|
video_id: id,
|
|
position_seconds: 0,
|
|
}));
|
|
clock.current = 0;
|
|
});
|
|
|
|
it("steps to the item that followed, and the removed one is gone from the queue", async () => {
|
|
renderModal(
|
|
<PlayerModal
|
|
video={B!}
|
|
queue={[A!, B!, C!]}
|
|
startIndex={1}
|
|
onClose={() => {}}
|
|
onState={() => {}}
|
|
onRemoveFromList={async () => true}
|
|
/>,
|
|
);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
|
fireEvent.click(trash("B"));
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("C"));
|
|
prev(); // …straight back to A: B is no longer there to step onto
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
});
|
|
|
|
it("closes the player when the last item goes", async () => {
|
|
const onClose = vi.fn();
|
|
renderModal(
|
|
<PlayerModal
|
|
video={A!}
|
|
queue={[A!]}
|
|
startIndex={0}
|
|
onClose={onClose}
|
|
onState={() => {}}
|
|
onRemoveFromList={async () => true}
|
|
/>,
|
|
);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
fireEvent.click(trash("A"));
|
|
await waitFor(() => expect(onClose).toHaveBeenCalled());
|
|
});
|
|
|
|
it("leaves playback and the queue alone when the removal did not happen", async () => {
|
|
const onClose = vi.fn();
|
|
renderModal(
|
|
<PlayerModal
|
|
video={A!}
|
|
queue={[A!, B!]}
|
|
startIndex={0}
|
|
onClose={onClose}
|
|
onState={() => {}}
|
|
onRemoveFromList={async () => false}
|
|
/>,
|
|
);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
fireEvent.click(trash("A"));
|
|
await settle();
|
|
expect(onClose).not.toHaveBeenCalled();
|
|
expect(started.map((s) => s.videoId)).toEqual(["A"]);
|
|
next();
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
|
prev(); // A is still in the queue
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
});
|
|
|
|
it("keeps playback where the viewer went while the confirm was open", async () => {
|
|
// prev/next keep working under the confirm dialog; deciding from the cursor as it was when the
|
|
// button was pressed threw the viewer back onto a video they had already moved past.
|
|
let answer!: (ok: boolean) => void;
|
|
renderModal(
|
|
<PlayerModal
|
|
video={A!}
|
|
queue={[A!, B!, C!, D!]}
|
|
startIndex={0}
|
|
onClose={() => {}}
|
|
onState={() => {}}
|
|
onRemoveFromList={() => new Promise<boolean>((r) => (answer = r))}
|
|
/>,
|
|
);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
fireEvent.click(trash("A"));
|
|
next();
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
|
next();
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("C"));
|
|
await act(async () => answer(true));
|
|
expect(lastStart()?.videoId).toBe("C");
|
|
prev();
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
|
// …and A is gone from the queue, not just skipped over: B is its head now, so stepping back
|
|
// again goes nowhere. (One step back lands on B either way — it took the second to tell.)
|
|
prev();
|
|
await settle();
|
|
expect(lastStart()?.videoId).toBe("B");
|
|
expect(started.filter((s) => s.videoId === "A")).toHaveLength(1); // never played again
|
|
});
|
|
|
|
it("gives focus back to the Remove button when the confirm is cancelled", async () => {
|
|
// The button used to be disabled for the confirm's whole lifetime, so the dialog's focus
|
|
// restore landed on a disabled element — a no-op — and a keyboard user was left on <body>,
|
|
// outside the aria-modal player.
|
|
function WithConfirm() {
|
|
const confirm = useConfirm();
|
|
return (
|
|
<PlayerModal
|
|
video={A!}
|
|
queue={[A!, B!]}
|
|
startIndex={0}
|
|
onClose={() => {}}
|
|
onState={() => {}}
|
|
onRemoveFromList={(v) =>
|
|
confirm({ title: "t", message: v.title, confirmLabel: "remove", danger: true })
|
|
}
|
|
/>
|
|
);
|
|
}
|
|
renderModal(
|
|
<ConfirmProvider>
|
|
<WithConfirm />
|
|
</ConfirmProvider>,
|
|
);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
trash("A").focus();
|
|
fireEvent.click(trash("A"));
|
|
fireEvent.click(await screen.findByRole("button", { name: "common.cancel" }));
|
|
await waitFor(() => expect(document.activeElement).toBe(trash("A")));
|
|
});
|
|
});
|
|
|
|
describe("PlayerModal — toasts raised while it is open", () => {
|
|
afterEach(() => act(() => clearAll()));
|
|
|
|
it("shows them inside the player, not under its scrim", async () => {
|
|
// The app's Toaster is in a stacking context below this body-level portal: a failed Remove
|
|
// reported by a toast was drawn under the 80% black scrim, i.e. still silent.
|
|
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
|
|
const A = vid("A", 0);
|
|
renderModal(
|
|
<PlayerModal video={A} queue={[A]} startIndex={0} onClose={() => {}} onState={() => {}} />,
|
|
);
|
|
act(() => void notify({ level: "error", message: "Couldn't remove the video." }));
|
|
expect(screen.getByTestId("player-modal").textContent).toContain("Couldn't remove the video.");
|
|
});
|
|
|
|
it("shows them inside the stage in true fullscreen — the element holding the lock", async () => {
|
|
// Anything outside the fullscreen element is painted under it. The player's host sat beside the
|
|
// card, so in fullscreen a lost connection was reported to nobody.
|
|
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
|
|
const A = vid("A", 0);
|
|
renderModal(
|
|
<PlayerModal video={A} queue={[A]} startIndex={0} onClose={() => {}} onState={() => {}} />,
|
|
);
|
|
const stage = document.querySelector(".player-stage")!;
|
|
Object.defineProperty(document, "fullscreenElement", { configurable: true, get: () => stage });
|
|
try {
|
|
act(() => void document.dispatchEvent(new Event("fullscreenchange")));
|
|
act(() => void notify({ level: "error", message: "Connection lost" }));
|
|
await waitFor(() => expect(stage.textContent).toContain("Connection lost"));
|
|
} finally {
|
|
delete (document as { fullscreenElement?: unknown }).fullscreenElement;
|
|
}
|
|
});
|
|
});
|
|
|
|
// --- the dialog contract it now shares (ui/dialogFocus) -----------------------------------------
|
|
// Until E11 this was the one `aria-modal` shell with no accessible name, no Tab trap and no focus
|
|
// restore: Tab walked out of an open player into the page behind the scrim, and closing it with
|
|
// Escape left a keyboard user on <body>, their place in a long feed lost (deep-review round 7).
|
|
describe("PlayerModal — the dialog contract", () => {
|
|
const open = (onClose = () => {}) => {
|
|
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
|
|
const A = vid("A", 0);
|
|
return renderModal(
|
|
<PlayerModal video={A} queue={[A]} startIndex={0} onClose={onClose} onState={() => {}} />,
|
|
);
|
|
};
|
|
|
|
it("is named by the video's title", () => {
|
|
open();
|
|
expect(screen.getByRole("dialog", { name: "Video A" })).toBeTruthy();
|
|
});
|
|
|
|
it("opens focused on its card, not on a control, so its shortcuts keep working", () => {
|
|
open();
|
|
const active = document.activeElement as HTMLElement;
|
|
expect(active.tagName).not.toBe("BUTTON");
|
|
expect(screen.getByTestId("player-modal").contains(active)).toBe(true);
|
|
});
|
|
|
|
it("gives the focus back to what opened it when it closes", () => {
|
|
const opener = document.createElement("button");
|
|
document.body.appendChild(opener);
|
|
opener.focus();
|
|
try {
|
|
const view = open();
|
|
expect(document.activeElement).not.toBe(opener);
|
|
view.unmount();
|
|
expect(document.activeElement).toBe(opener);
|
|
} finally {
|
|
opener.remove();
|
|
}
|
|
});
|
|
|
|
it("keeps Tab inside the player", () => {
|
|
open();
|
|
const dialog = screen.getByTestId("player-modal");
|
|
const tabbables = [...dialog.querySelectorAll<HTMLElement>("button:not([disabled]), a[href]")];
|
|
const last = tabbables[tabbables.length - 1]!;
|
|
last.focus();
|
|
// jsdom has no layout, so the trap's visibility filter would see nothing tabbable; give every
|
|
// element an offsetParent for the duration of this check.
|
|
const spy = vi
|
|
.spyOn(HTMLElement.prototype, "offsetParent", "get")
|
|
.mockReturnValue(document.body);
|
|
try {
|
|
fireEvent.keyDown(window, { key: "Tab" });
|
|
expect(dialog.contains(document.activeElement)).toBe(true);
|
|
expect(document.activeElement).not.toBe(last);
|
|
} finally {
|
|
spy.mockRestore();
|
|
}
|
|
});
|
|
});
|
|
|
|
// --- the watch toggle belongs to the item it describes -------------------------------------------
|
|
// A late writer exists by construction: useYtProgress's cleanup for the OUTGOING item runs after the
|
|
// commit that already advanced `active`, and within the finish margin it auto-marks that item
|
|
// watched. While the mirror was a bare boolean that write landed last and painted the INCOMING item
|
|
// as watched. Keying the mirror by id makes the stray write not this item's, so it is ignored.
|
|
describe("PlayerModal — the watch toggle after a queue advance", () => {
|
|
const A = vid("A", 0);
|
|
const B = vid("B", 0);
|
|
|
|
beforeEach(() => {
|
|
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
|
|
vi.spyOn(api, "saveProgress").mockImplementation(async (id: string) => ({
|
|
video_id: id,
|
|
position_seconds: 0,
|
|
}));
|
|
});
|
|
|
|
it("does not inherit the outgoing item's auto-watch when stepping away near the end", async () => {
|
|
const onState = vi.fn();
|
|
clock.current = 2995; // inside min(FINISH_MARGIN, duration/2) = 10s of 3000
|
|
renderModal(
|
|
<PlayerModal video={A} queue={[A, B]} startIndex={0} onClose={() => {}} onState={onState} />,
|
|
);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
|
|
next();
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
|
await settle();
|
|
|
|
// A really is marked watched — that part is correct and must keep working.
|
|
expect(onState.mock.calls).toContainEqual(["A", "watched"]);
|
|
// B is not, and its toggle says so. Before the fix both toggles read "watchedUnmark" here.
|
|
expect(screen.queryAllByTitle("player.watchedUnmark")).toHaveLength(0);
|
|
expect(screen.queryAllByTitle("player.markWatched").length).toBeGreaterThan(0);
|
|
});
|
|
|
|
it("still reflects a watch toggled on the item you are actually on", async () => {
|
|
const onState = vi.fn();
|
|
clock.current = 0;
|
|
renderModal(
|
|
<PlayerModal video={A} queue={[A, B]} startIndex={0} onClose={() => {}} onState={onState} />,
|
|
);
|
|
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
|
|
|
fireEvent.click(screen.getAllByTitle("player.markWatched")[0]!);
|
|
expect(onState.mock.calls).toContainEqual(["A", "watched"]);
|
|
expect(screen.queryAllByTitle("player.watchedUnmark").length).toBeGreaterThan(0);
|
|
});
|
|
});
|