Round 4 was targeted at the surfaces rounds 2 and 3 changed. 29 findings raised, 26 survived deduplication, 12 survived three adversarial readings each. `set_delivery` renewed the claim but never checked it was still ours, while its sibling `release_send` always did. `expire_on_commit` is off, so the token it re-stamped came from the caller's own memory, not the row: a send that outlived its 600 s lease came back, overwrote the take-over send's committed links with a full-column write, and put its own dead token back. It now takes the token, reads the row's claim with a targeted scalar (not a refresh, which would discard the `issue_url` assigned one line earlier) and raises `SendClaimLost` rather than writing. An issue created by a send that had already lost its claim is logged with its number — that record is the only one that will exist. The tracker's settings are read above the pre-call checkpoint instead of inside `_client`. Reading them there was a SELECT one line after the commit that was meant to end the transaction, and held it — idle, snapshot pinned — across the repository check and the create. The comment and the module docstring said otherwise; measuring it in pg_stat_activity is what settled it. Two more comments were saying things the code had stopped doing: the `**state` spread does not carry the claim (`set_delivery` strips and re-stamps it), and react-query does not debounce the preview — the editor being unmounted is what paces it. The composer stopped stranding the focus on `<body>`: a send that creates the issue and then fails on an attachment unmounts the button that was just pressed, and only the success path handed the focus on. Export no longer saves into a refusal it does not need — the zip is the way out of a stuck send, and its pre-save was the one step that could not survive one. The delivery banner announces through a role instead of an `aria-live` created with its own message. `historyState` returns a union, so `chipTone` matching on the same keys is checked by tsc rather than by hope. PlexPlayer was the only full-screen overlay in the tree with no focus containment at all — Tab walked out of an opaque player into a page nobody can see. It now carries the shared trap. Closing that hole exposed the one underneath it: `useDismiss` registered as a layer that traps the Tab while installing no trap, so the player stood its own down for its own menus. Measured in a real browser: 30 Tabs with the settings panel open, 9 escapes into the nav rail, and 0 after. Escape ordering is unchanged — the panel first, then the player. Three tests that did not hold their line: the advisory lock's call site (deleting it left the suite green), the trail assertion whose `all()` was vacuous over an empty filter, and a `layerStack` assertion the stand-in dialog already forced. All eleven executable fixes mutation-checked red for the right reason; the two comment corrections have nothing to pin.
186 lines
6.9 KiB
TypeScript
186 lines
6.9 KiB
TypeScript
// @vitest-environment jsdom
|
|
//
|
|
// The Plex player paints an opaque, full-viewport layer over the whole app — and until deep-review
|
|
// round 4 it was the only overlay in the tree with no Tab trap at all. Tab from its transport bar,
|
|
// from its menus, or from the debug recorder's pill (which docks in THIS player's toast host) walked
|
|
// straight out into the page behind it, where the focus ring is invisible and the next Enter fires
|
|
// something the user cannot see. `ui/Modal`, `PlayerModal`, `OnboardingWizard` and `Welcome` all trap;
|
|
// the round-3 comment that handed the step prompt's trap to "the dialog below" assumed this one did
|
|
// too. Its own file because `layerStack` is module state and these tests push into it.
|
|
import { describe, it, expect, vi, beforeAll, afterEach } from "vitest";
|
|
import { render, cleanup, waitFor, act } from "@testing-library/react";
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
|
|
vi.mock("react-i18next", async (importOriginal) => ({
|
|
...(await importOriginal<typeof import("react-i18next")>()),
|
|
useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
|
|
}));
|
|
|
|
import PlexPlayer from "./PlexPlayer";
|
|
import { api, type PlexItemDetail } from "../lib/api";
|
|
import { pushLayer, removeLayer } from "../lib/layerStack";
|
|
|
|
beforeAll(() => {
|
|
class Stub {
|
|
observe() {}
|
|
unobserve() {}
|
|
disconnect() {}
|
|
takeRecords() {
|
|
return [];
|
|
}
|
|
}
|
|
globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver;
|
|
globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver;
|
|
for (const [name, value] of Object.entries({
|
|
play: () => Promise.resolve(),
|
|
pause: () => {},
|
|
load: () => {},
|
|
}))
|
|
Object.defineProperty(HTMLMediaElement.prototype, name, { configurable: true, value });
|
|
Object.defineProperty(HTMLMediaElement.prototype, "seekable", {
|
|
configurable: true,
|
|
get: () => ({ length: 0, end: () => 0 }),
|
|
});
|
|
Object.defineProperty(HTMLMediaElement.prototype, "textTracks", {
|
|
configurable: true,
|
|
get: () => Object.assign([], { addEventListener() {}, removeEventListener() {} }),
|
|
});
|
|
});
|
|
|
|
const behind: HTMLButtonElement[] = [];
|
|
afterEach(() => {
|
|
cleanup();
|
|
while (behind.length) behind.pop()!.remove();
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
const detail = {
|
|
id: "1",
|
|
title: "Title 1",
|
|
type: "movie",
|
|
duration_seconds: 3000,
|
|
status: "new",
|
|
position_seconds: 0,
|
|
audio_streams: [],
|
|
subtitle_streams: [],
|
|
markers: [],
|
|
} as unknown as PlexItemDetail;
|
|
|
|
async function mount() {
|
|
vi.spyOn(api, "plexItem").mockResolvedValue(detail);
|
|
vi.spyOn(api, "plexSession").mockResolvedValue({
|
|
mode: "direct",
|
|
url: "/media/1",
|
|
start_s: 0,
|
|
} as Awaited<ReturnType<typeof api.plexSession>>);
|
|
vi.spyOn(api, "plexProgress").mockResolvedValue(undefined as never);
|
|
render(
|
|
<QueryClientProvider
|
|
client={new QueryClient({ defaultOptions: { queries: { retry: false } } })}
|
|
>
|
|
<PlexPlayer itemId="1" onClose={() => {}} />
|
|
</QueryClientProvider>,
|
|
);
|
|
await waitFor(() =>
|
|
expect(document.querySelector("video")?.getAttribute("src")).toBe("/media/1"),
|
|
);
|
|
}
|
|
|
|
/** A control on the page the player is painted over — the thing focus used to escape onto. */
|
|
function pageBehind() {
|
|
const b = document.createElement("button");
|
|
b.textContent = "on the page behind";
|
|
document.body.insertBefore(b, document.body.firstChild);
|
|
behind.push(b);
|
|
return b;
|
|
}
|
|
|
|
const player = () => document.querySelector<HTMLElement>(".fixed.z-overlay")!;
|
|
|
|
/** jsdom has no layout, so the trap's visibility filter (`offsetParent`) would see nothing tabbable
|
|
* anywhere. Give every element one for the duration of a check — the house pattern (PlayerModal,
|
|
* OnboardingWizard, ui/dialogFocus all do this). */
|
|
function withLayout<T>(fn: () => T): T {
|
|
const spy = vi.spyOn(HTMLElement.prototype, "offsetParent", "get").mockReturnValue(document.body);
|
|
try {
|
|
return fn();
|
|
} finally {
|
|
spy.mockRestore();
|
|
}
|
|
}
|
|
|
|
const press = (key: string, shiftKey = false) =>
|
|
act(() => {
|
|
window.dispatchEvent(
|
|
new KeyboardEvent("keydown", { key, shiftKey, bubbles: true, cancelable: true }),
|
|
);
|
|
});
|
|
|
|
describe("PlexPlayer · Tab stays inside the player", () => {
|
|
it("pulls the focus back in when it is on the page behind", async () => {
|
|
await mount();
|
|
const outside = pageBehind();
|
|
outside.focus();
|
|
expect(document.activeElement).toBe(outside);
|
|
withLayout(() => press("Tab"));
|
|
// Not left on a control under an opaque overlay: the trap takes it into the player.
|
|
expect(document.activeElement).not.toBe(outside);
|
|
expect(player().contains(document.activeElement)).toBe(true);
|
|
});
|
|
|
|
it("wraps from the last of its own controls back to the first", async () => {
|
|
await mount();
|
|
const items = [
|
|
...player().querySelectorAll<HTMLElement>(
|
|
'button:not([disabled]), a[href], [tabindex]:not([tabindex="-1"])',
|
|
),
|
|
];
|
|
expect(items.length).toBeGreaterThan(1);
|
|
const first = items[0]!;
|
|
const last = items[items.length - 1]!;
|
|
last.focus();
|
|
withLayout(() => press("Tab"));
|
|
expect(document.activeElement).toBe(first);
|
|
// ...and backwards, which is the direction that used to walk straight out behind the player.
|
|
first.focus();
|
|
withLayout(() => press("Tab", true));
|
|
expect(document.activeElement).toBe(last);
|
|
});
|
|
|
|
it("keeps trapping while one of its OWN menus is open", async () => {
|
|
await mount();
|
|
// The settings and track menus are `useDismiss` layers. That hook owns Escape and the
|
|
// outside-press and installs no Tab trap at all — but it used to register as a layer that
|
|
// DOES trap, so the player stood its own trap down for it and Tab walked out past the last
|
|
// control into the nav rail. Measured in a real browser: 30 Tabs, 9 escapes. The menus render
|
|
// inside the player, so the player's trap is the one that must stay on.
|
|
const gear = [...player().querySelectorAll<HTMLElement>("button")].find(
|
|
(b) => b.getAttribute("aria-label") === "plex.player.settings",
|
|
);
|
|
expect(gear).toBeDefined();
|
|
act(() => gear!.click());
|
|
const items = [...player().querySelectorAll<HTMLElement>("button:not([disabled])")];
|
|
// The menu really opened — the trap now has to span its controls too.
|
|
expect(items.length).toBeGreaterThan(12);
|
|
const last = items[items.length - 1]!;
|
|
last.focus();
|
|
withLayout(() => press("Tab"));
|
|
expect(player().contains(document.activeElement)).toBe(true);
|
|
expect(document.activeElement).toBe(items[0]);
|
|
});
|
|
|
|
it("leaves the Tab to a dialog opened above it, which traps for itself", async () => {
|
|
await mount();
|
|
const outside = pageBehind();
|
|
outside.focus();
|
|
const above = pushLayer(); // a Modal over the player — the download dialog, say
|
|
try {
|
|
withLayout(() => press("Tab"));
|
|
// The player does not reach past it: the dialog above owns the keyboard and its own trap.
|
|
expect(document.activeElement).toBe(outside);
|
|
} finally {
|
|
removeLayer(above);
|
|
}
|
|
});
|
|
});
|