fix(player): a step onto an unplayable video carries on the way it went
Previous onto an embed-blocked or removed video skipped forward again (the auto-advance direction), straight back to the item just left. The player now remembers the direction of the step that reached the item and the videos YouTube refused this session: an error carries on that way, Previous / Next pass over known-dead items without loading them, and go dark when only those are left. A step of your own skips with auto-advance off too.
This commit is contained in:
@@ -63,7 +63,11 @@ const started: { videoId: string; start: number | undefined }[] = [];
|
||||
// What the player was asked to switch to in place (a description link, a related video).
|
||||
const loaded: { videoId: string; startSeconds: number }[] = [];
|
||||
// The last player's event handlers, so a test can say "the player is ready" when it wants to.
|
||||
let playerEvents: { onReady?: () => void; onStateChange?: (e: { data: number }) => void } = {};
|
||||
let playerEvents: {
|
||||
onReady?: () => void;
|
||||
onStateChange?: (e: { data: number }) => void;
|
||||
onError?: (e: { data: number }) => void;
|
||||
} = {};
|
||||
const base = {
|
||||
getCurrentTime: () => clock.current,
|
||||
getDuration: () => clock.duration,
|
||||
@@ -1816,3 +1820,81 @@ describe("PlayerModal — clip mode", () => {
|
||||
expect(screen.getByTestId("clip-panel")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
// An embed-blocked or deleted video answers with onError, never 'ended'. Stepping BACK onto one used
|
||||
// to skip forward again (the auto-advance direction), straight to the item you had just left.
|
||||
describe("PlayerModal — stepping over a video that cannot play", () => {
|
||||
const A = vid("A", 0);
|
||||
const B = vid("B", 0);
|
||||
const C = vid("C", 0);
|
||||
const fail = () => act(() => playerEvents.onError?.({ data: 150 }));
|
||||
const opened = (id: string) => started.filter((s) => s.videoId === id).length;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
|
||||
vi.spyOn(api, "saveProgress").mockImplementation(async (id: string) => ({
|
||||
video_id: id,
|
||||
position_seconds: 0,
|
||||
}));
|
||||
});
|
||||
|
||||
const open = async (startIndex: number) => {
|
||||
renderModal(
|
||||
<PlayerModal
|
||||
video={[A, B, C][startIndex]!}
|
||||
queue={[A, B, C]}
|
||||
startIndex={startIndex}
|
||||
onClose={() => {}}
|
||||
onState={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe([A, B, C][startIndex]!.id));
|
||||
};
|
||||
|
||||
it("Previous onto a dead video goes on back, not forward to where you came from", async () => {
|
||||
me = { related_enabled: true, preferences: { playerAutoAdvance: "next" } } as unknown as Me;
|
||||
await open(2);
|
||||
prev();
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
||||
fail();
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
||||
});
|
||||
|
||||
it("a step of your own skips a dead video with auto-advance off too", async () => {
|
||||
await open(2);
|
||||
prev();
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
||||
fail();
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
||||
});
|
||||
|
||||
it("a video found dead is stepped over without loading it again", async () => {
|
||||
await open(2);
|
||||
prev();
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
||||
fail();
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe("A"));
|
||||
next();
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe("C"));
|
||||
expect(opened("B")).toBe(1);
|
||||
});
|
||||
|
||||
it("Previous is off when only dead videos are before you", async () => {
|
||||
await open(0);
|
||||
fail(); // opened on it, auto-advance off: it stays, and it is known dead now
|
||||
await settle();
|
||||
expect(lastStart()?.videoId).toBe("A");
|
||||
next();
|
||||
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
||||
for (const b of screen.getAllByRole("button", { name: "player.previous" }))
|
||||
expect((b as HTMLButtonElement).disabled).toBe(true);
|
||||
});
|
||||
|
||||
it("a video opened dead with auto-advance off stays on its error", async () => {
|
||||
await open(1);
|
||||
fail();
|
||||
await settle();
|
||||
expect(lastStart()?.videoId).toBe("B");
|
||||
expect(started).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -72,7 +72,7 @@ import { linkedPlayer, playerParamUpdates, type PlayerPanelTab } from "../../lib
|
||||
import ChannelNameLink from "../video/ChannelNameLink";
|
||||
import Toaster from "../../components/ui/Toaster";
|
||||
import { useOverlayOrder } from "../../components/ui/overlayOrder";
|
||||
import { afterRemoval } from "../../lib/playerQueue";
|
||||
import { afterRemoval, stepIndex } from "../../lib/playerQueue";
|
||||
import {
|
||||
useNativeControlsYield,
|
||||
useYtBigView,
|
||||
@@ -118,6 +118,11 @@ function canScrollBy(el: HTMLElement | null, dir: -1 | 1): boolean {
|
||||
// a rem width, so it grows with the text size; the video takes what is left.
|
||||
const WIDE_PLAYER_PX = 1280;
|
||||
const SIDE_PANEL = "20rem";
|
||||
// The way a queue step goes: back, on, or to a random other item.
|
||||
type Way = 1 | -1 | "random";
|
||||
const autoWay = (a: Exclude<AutoMode, "off">): Way =>
|
||||
a === "random" ? "random" : a === "next" ? 1 : -1;
|
||||
|
||||
// The side panel shut to a strip: one icon button wide, close / open / previous / next in a column.
|
||||
const PANEL_STRIP = "2.75rem";
|
||||
// The narrowest the stacked video gets to make room for the chrome under it (see the card's maxWidth).
|
||||
@@ -657,15 +662,46 @@ export default function PlayerModal({
|
||||
// Prev/next stepping through the queue (the feed's order, or a playlist). Read the live
|
||||
// queue+index from a ref so the window keydown handler (bound once) always steps from the
|
||||
// current position, not a stale closure.
|
||||
const navRef = useLatest({ queue, index });
|
||||
const goPrev = () => {
|
||||
const { queue: q, index: i } = navRef.current;
|
||||
if (q && i > 0) setPlayingId(q[i - 1]!.id);
|
||||
//
|
||||
// The videos YouTube refused to play this session (embed-blocked, removed): a step passes over
|
||||
// them, and Previous / Next go dark when only those are left that way. State, not a ref, because
|
||||
// the buttons read it.
|
||||
const [dead, setDead] = useState<ReadonlySet<string>>(() => new Set());
|
||||
const navRef = useLatest({ queue, index, dead });
|
||||
// Which way the step that reached the playing item went — what an item that turns out unplayable
|
||||
// carries on along. null: no step yet (the item the player opened on).
|
||||
const stepRef = useRef<{ way: Way } | null>(null);
|
||||
const stepTo = (id: string, way: Way) => {
|
||||
stepRef.current = { way };
|
||||
setPlayingId(id);
|
||||
};
|
||||
const goNext = () => {
|
||||
const { queue: q, index: i } = navRef.current;
|
||||
if (q && i < q.length - 1) setPlayingId(q[i + 1]!.id);
|
||||
// Step from `i` the given way over the videos known dead; whether there was anywhere to go.
|
||||
const moveOn = (
|
||||
q: Video[],
|
||||
i: number,
|
||||
way: Way,
|
||||
d: ReadonlySet<string>,
|
||||
wrap: boolean,
|
||||
): boolean => {
|
||||
if (way === "random") {
|
||||
const rest = q.filter((v, k) => k !== i && !d.has(v.id));
|
||||
if (!rest.length) return false;
|
||||
stepTo(rest[Math.floor(Math.random() * rest.length)]!.id, "random");
|
||||
return true;
|
||||
}
|
||||
const j = stepIndex(q, i, way, d, wrap);
|
||||
if (j < 0) return false;
|
||||
stepTo(q[j]!.id, way);
|
||||
return true;
|
||||
};
|
||||
const step = (dir: 1 | -1) => {
|
||||
const { queue: q, index: i, dead: d } = navRef.current;
|
||||
if (q) moveOn(q, i, dir, d, false);
|
||||
};
|
||||
const goPrev = () => step(-1);
|
||||
const goNext = () => step(1);
|
||||
const canPrev = !!queue && stepIndex(queue, index, -1, dead) >= 0;
|
||||
const canNext = !!queue && stepIndex(queue, index, 1, dead) >= 0;
|
||||
// "Remove from this playlist", offered right where you are watching the thing you want gone. The
|
||||
// parent owns the removal (API + caches + the toast whose Undo puts it back); the player owns what happens NEXT, because it
|
||||
// is the only side that knows which item is playing — its queue is frozen at mount, so the parent
|
||||
@@ -699,7 +735,7 @@ export default function PlayerModal({
|
||||
return;
|
||||
}
|
||||
setQueue(next.queue);
|
||||
setPlayingId(next.playingId);
|
||||
stepTo(next.playingId, 1); // the item that followed: a dead one there carries on forward
|
||||
};
|
||||
|
||||
// Relative seek within the current video (plain Arrow keys), matching YouTube's ±5s.
|
||||
@@ -738,28 +774,28 @@ export default function PlayerModal({
|
||||
if (l === "one") return replayCurrent();
|
||||
// A related video's end is not the queue's: the list beside it is where the next one is picked.
|
||||
if (related) return;
|
||||
const { queue: q, index: i } = navRef.current;
|
||||
if (!q || q.length === 0) return;
|
||||
const { queue: q, index: i, dead: d } = navRef.current;
|
||||
if (!q || q.length === 0 || a === "off") return;
|
||||
const wrap = l === "all";
|
||||
if (a === "next") {
|
||||
if (i < q.length - 1) setPlayingId(q[i + 1]!.id);
|
||||
else if (wrap) {
|
||||
if (q.length === 1) replayCurrent();
|
||||
else setPlayingId(q[0]!.id);
|
||||
}
|
||||
} else if (a === "prev") {
|
||||
if (i > 0) setPlayingId(q[i - 1]!.id);
|
||||
else if (wrap) {
|
||||
if (q.length === 1) replayCurrent();
|
||||
else setPlayingId(q[q.length - 1]!.id);
|
||||
}
|
||||
} else if (a === "random" && q.length > 1) {
|
||||
let r = i;
|
||||
while (r === i) r = Math.floor(Math.random() * q.length);
|
||||
setPlayingId(q[r]!.id);
|
||||
} else if (a === "random" && wrap) {
|
||||
replayCurrent();
|
||||
}
|
||||
// Nowhere else to go (a one-item list, or every other item dead): Loop "All" repeats this one.
|
||||
if (!moveOn(q, i, autoWay(a), d, wrap) && wrap) replayCurrent();
|
||||
};
|
||||
// The playing queue item turned out unplayable (YouTube's onError — it never reaches 'ended').
|
||||
// Remember it, then carry on the way the step that reached it went: Previous onto a dead video
|
||||
// goes on back, not forward to the one you just left. The item the player opened on was reached
|
||||
// by no step, so it moves on only with auto-advance on, in its direction. `go` false: the run of
|
||||
// errors hit the player's cap — remembered, but playback stays on the error.
|
||||
const skipUnplayable = (go: boolean) => {
|
||||
if (related || clipOpenRef.current) return;
|
||||
const { queue: q, index: i, dead: d } = navRef.current;
|
||||
const gone = q?.[i]?.id;
|
||||
if (!q || !gone) return;
|
||||
const now = new Set(d).add(gone);
|
||||
setDead(now);
|
||||
if (!go || q.length <= 1) return;
|
||||
const { autoMode: a, loopMode: l } = modeRef.current;
|
||||
const way = stepRef.current?.way ?? (a === "off" ? null : autoWay(a));
|
||||
if (way != null) moveOn(q, i, way, now, l === "all");
|
||||
};
|
||||
|
||||
// Lazy description (fetched only when it is opened). It opens from an ⓘ beside the title, by click
|
||||
@@ -982,14 +1018,13 @@ export default function PlayerModal({
|
||||
mountRef,
|
||||
playerRef,
|
||||
currentIdRef,
|
||||
navRef,
|
||||
modeRef,
|
||||
setWatched,
|
||||
setPlayerError,
|
||||
setLiveData,
|
||||
reapplyStored,
|
||||
reclaimFocus,
|
||||
advanceOnEnd,
|
||||
skipUnplayable,
|
||||
sessionPosRef,
|
||||
qc,
|
||||
});
|
||||
@@ -1177,6 +1212,8 @@ export default function PlayerModal({
|
||||
variant="docked"
|
||||
index={index}
|
||||
queueLength={queue!.length}
|
||||
canPrev={canPrev}
|
||||
canNext={canNext}
|
||||
autoMode={autoMode}
|
||||
loopMode={loopMode}
|
||||
onPrev={goPrev}
|
||||
@@ -1332,7 +1369,7 @@ export default function PlayerModal({
|
||||
<IconButton
|
||||
variant="ghost"
|
||||
onClick={goPrev}
|
||||
disabled={index === 0}
|
||||
disabled={!canPrev}
|
||||
label={t("player.previous")}
|
||||
title={`${t("player.previous")} · Shift+←`}
|
||||
>
|
||||
@@ -1341,7 +1378,7 @@ export default function PlayerModal({
|
||||
<IconButton
|
||||
variant="ghost"
|
||||
onClick={goNext}
|
||||
disabled={index >= queue!.length - 1}
|
||||
disabled={!canNext}
|
||||
label={t("player.next")}
|
||||
title={`${t("player.next")} · Shift+→`}
|
||||
>
|
||||
@@ -1377,6 +1414,8 @@ export default function PlayerModal({
|
||||
variant="modal"
|
||||
index={index}
|
||||
queueLength={queue!.length}
|
||||
canPrev={canPrev}
|
||||
canNext={canNext}
|
||||
autoMode={autoMode}
|
||||
loopMode={loopMode}
|
||||
onPrev={goPrev}
|
||||
|
||||
@@ -24,6 +24,8 @@ function mount(variant: "modal" | "docked", autoMode: AutoMode, loopMode: LoopMo
|
||||
variant={variant}
|
||||
index={1}
|
||||
queueLength={3}
|
||||
canPrev
|
||||
canNext
|
||||
autoMode={autoMode}
|
||||
loopMode={loopMode}
|
||||
onPrev={() => {}}
|
||||
|
||||
@@ -27,6 +27,8 @@ export function PlayerTransportControls({
|
||||
variant,
|
||||
index,
|
||||
queueLength,
|
||||
canPrev,
|
||||
canNext,
|
||||
autoMode,
|
||||
loopMode,
|
||||
onPrev,
|
||||
@@ -37,6 +39,9 @@ export function PlayerTransportControls({
|
||||
variant: "docked" | "modal";
|
||||
index: number;
|
||||
queueLength: number;
|
||||
/** Whether there is a playable item that way (one YouTube refused is passed over). */
|
||||
canPrev: boolean;
|
||||
canNext: boolean;
|
||||
autoMode: AutoMode;
|
||||
loopMode: LoopMode;
|
||||
onPrev: () => void;
|
||||
@@ -70,7 +75,7 @@ export function PlayerTransportControls({
|
||||
name: t("player.previous"),
|
||||
title: `${t("player.previous")} · Shift+←`,
|
||||
onClick: onPrev,
|
||||
disabled: index === 0,
|
||||
disabled: !canPrev,
|
||||
labelFrom: "sm",
|
||||
},
|
||||
{
|
||||
@@ -79,7 +84,7 @@ export function PlayerTransportControls({
|
||||
name: t("player.next"),
|
||||
title: `${t("player.next")} · Shift+→`,
|
||||
onClick: onNext,
|
||||
disabled: index === queueLength - 1,
|
||||
disabled: !canNext,
|
||||
labelFrom: "sm",
|
||||
trailing: true,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user