fix(player): make the video seek bar work by touch (DR-099)
🏗️ Build and Test JellyTau / Run Tests (push) Failing after 6m30s
🏗️ Build and Test JellyTau / Android Compile Check (push) Has been skipped
Publish Documentation / Build & publish docs to gitea-pages (push) Successful in 5m25s
Traceability Validation / Check Requirement Traces (push) Successful in 18s
Build & Release / Run Tests (push) Failing after 6m4s
Build & Release / Build Linux (push) Has been skipped
Build & Release / Build Windows (push) Has been skipped
Build & Release / Build Android (push) Has been skipped
Build & Release / Create Release (push) Has been skipped
🏗️ Build and Test JellyTau / Run Tests (push) Failing after 6m30s
🏗️ Build and Test JellyTau / Android Compile Check (push) Has been skipped
Publish Documentation / Build & publish docs to gitea-pages (push) Successful in 5m25s
Traceability Validation / Check Requirement Traces (push) Successful in 18s
Build & Release / Run Tests (push) Failing after 6m4s
Build & Release / Build Linux (push) Has been skipped
Build & Release / Build Windows (push) Has been skipped
Build & Release / Build Android (push) Has been skipped
Build & Release / Create Release (push) Has been skipped
On Android, dragging or tapping the progress bar moved the thumb but playback stayed where it was. Two separate defects, both touch-only, which is why the mouse-driven scrub tests never caught either. 1. Gesture hijack. DR-098 taught handleTouchStart to ignore touches that land on a control, but handleTouchMove kept running. It measures against touchStartX/Y, which that early return leaves at the PREVIOUS gesture's values, so a seek-bar drag produced a huge bogus vertical delta: read as a brightness swipe, it dimmed the screen to the 0.3 floor and fired a spurious play/pause "correction" mid-drag. A gesture is now latched at touchstart (playerGestureActive) and touchmove ignores anything unlatched — re-checking the move target cannot recover a start point that was never recorded. 2. Commit signal. The seek was committed only from `change`, which Android's WebView does not reliably fire for a touch interaction on a range input, so the thumb moved to the tapped position and no seek ever ran. touchend/mouseup now commit too; `input` arms a one-shot latch so whichever release signal arrives first commits and the other is a no-op. seekRelative shares the same commitSeek entry point instead of fabricating a synthetic change event. Tests drive the slider with real touch events (UT-089, UT-090) and fail against the pre-fix component.
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
<!-- TRACES: UR-003, UR-005, UR-020, UR-021, UR-026, UR-040, UR-061 | DR-010, DR-023, DR-024, DR-051, DR-052, DR-092 -->
|
||||
<!-- TRACES: UR-003, UR-005, UR-020, UR-021, UR-026, UR-040, UR-061 | DR-010, DR-023, DR-024, DR-051, DR-052, DR-092, DR-098, DR-099 -->
|
||||
<script lang="ts">
|
||||
import { onMount, onDestroy, untrack } from "svelte";
|
||||
import { goto } from "$app/navigation";
|
||||
@@ -124,6 +124,14 @@
|
||||
// so back-to-back double taps chain instead of stacking on a stale position.
|
||||
let pendingSeekTarget: number | null = null;
|
||||
let swipeGestureActive = $state(false);
|
||||
// Whether the in-flight touch belongs to the player surface (and so may be
|
||||
// read as a tap/swipe gesture) rather than to a control. Set on touchstart,
|
||||
// cleared on touchend — see handleTouchMove for why a per-gesture flag and not
|
||||
// just a per-event target check.
|
||||
let playerGestureActive = false;
|
||||
// Raised when the user changes the seek bar's value, cleared by whichever
|
||||
// release signal commits the seek. See handleSeekBarRelease.
|
||||
let seekCommitArmed = false;
|
||||
|
||||
// Backend info from Rust (Rust decides which backend to use based on platform)
|
||||
let useHtml5Element = $state(true); // Default to HTML5, Rust will override if using native backend
|
||||
@@ -1162,14 +1170,33 @@
|
||||
const targetTime = parseFloat(input.value);
|
||||
// Update the displayed time immediately for smooth visual feedback
|
||||
currentTime = targetTime;
|
||||
// The user has moved the value; the next release must commit it.
|
||||
seekCommitArmed = true;
|
||||
}
|
||||
|
||||
async function handleSeekBarChange(e: Event) {
|
||||
const input = e.target as HTMLInputElement;
|
||||
/**
|
||||
* Seek-bar released — commit the value the user landed on, at most once.
|
||||
*
|
||||
* Wired to `touchend`/`mouseup` AND `change`, because `change` alone is not
|
||||
* dependable: Android's WebView does not reliably fire it for a touch
|
||||
* interaction on a range input, so the thumb moved to the tapped position but
|
||||
* the seek never ran ("the bar moves, playback doesn't"). Engines that DO fire
|
||||
* `change` deliver both signals, hence the arm/disarm — whichever arrives
|
||||
* first commits and the other is a no-op.
|
||||
*/
|
||||
function handleSeekBarRelease(e: Event) {
|
||||
isDraggingSeekBar = false;
|
||||
if (!seekCommitArmed) return;
|
||||
seekCommitArmed = false;
|
||||
const input = (e.currentTarget ?? e.target) as HTMLInputElement;
|
||||
void commitSeek(parseFloat(input.value));
|
||||
}
|
||||
|
||||
async function commitSeek(rawTarget: number) {
|
||||
// Clamp strictly inside the media: the range input's max IS the duration, so
|
||||
// dragging fully right would otherwise request a segment past the media end,
|
||||
// which the server never produces (see END_SEEK_MARGIN_SECONDS).
|
||||
const targetTime = clampSeekTarget(parseFloat(input.value), duration);
|
||||
const targetTime = clampSeekTarget(rawTarget, duration);
|
||||
|
||||
// Set isSeeking immediately to prevent timeupdate from interfering
|
||||
isSeeking = true;
|
||||
@@ -1406,16 +1433,9 @@
|
||||
to: newTime.toFixed(2),
|
||||
});
|
||||
|
||||
// Call the unified handleSeekBarChange logic with the new time
|
||||
// Create a synthetic event to reuse the existing logic
|
||||
const syntheticEvent = {
|
||||
target: {
|
||||
value: newTime.toString()
|
||||
}
|
||||
} as unknown as Event;
|
||||
|
||||
// Same commit path as the seek bar — one place decides how a seek is issued.
|
||||
try {
|
||||
await handleSeekBarChange(syntheticEvent);
|
||||
await commitSeek(newTime);
|
||||
} finally {
|
||||
// The player is authoritative again from here on.
|
||||
if (pendingSeekTarget === newTime) pendingSeekTarget = null;
|
||||
@@ -1473,7 +1493,16 @@
|
||||
// container and touch events bubble, so without this a tap on the bottom
|
||||
// play button would toggle here AND again via the button's own click — the
|
||||
// two cancelling out and leaving the control apparently dead (DR-098).
|
||||
if (isControlSurfaceTouch(ancestorChain(e.target))) return;
|
||||
if (isControlSurfaceTouch(ancestorChain(e.target))) {
|
||||
// The move handler must stay out of it too. It reads touchStartX/Y, which
|
||||
// this early return leaves at the PREVIOUS gesture's values, so a seek-bar
|
||||
// drag came out as a huge vertical delta: it was mis-read as a brightness
|
||||
// swipe, which dimmed the screen and fired a spurious play/pause
|
||||
// "correction" mid-drag (DR-098).
|
||||
playerGestureActive = false;
|
||||
return;
|
||||
}
|
||||
playerGestureActive = true;
|
||||
|
||||
const touch = e.touches[0];
|
||||
touchStartX = touch.clientX;
|
||||
@@ -1505,6 +1534,10 @@
|
||||
}
|
||||
|
||||
function handleTouchMove(e: TouchEvent) {
|
||||
// Only a gesture that began on the bare video surface is ours. Re-checking
|
||||
// the target here would not be enough: the touch that started on a control
|
||||
// never recorded a start point, so any delta computed here is meaningless.
|
||||
if (!playerGestureActive) return;
|
||||
if (!e.touches[0]) return;
|
||||
|
||||
const touch = e.touches[0];
|
||||
@@ -1537,6 +1570,7 @@
|
||||
}
|
||||
|
||||
function handleTouchEnd(e: TouchEvent) {
|
||||
playerGestureActive = false;
|
||||
swipeGestureActive = false;
|
||||
swipeType = null;
|
||||
}
|
||||
@@ -1900,11 +1934,11 @@
|
||||
max={duration || 100}
|
||||
value={currentTime}
|
||||
oninput={handleSeekBarInput}
|
||||
onchange={handleSeekBarChange}
|
||||
onchange={handleSeekBarRelease}
|
||||
onmousedown={() => isDraggingSeekBar = true}
|
||||
onmouseup={() => isDraggingSeekBar = false}
|
||||
onmouseup={handleSeekBarRelease}
|
||||
ontouchstart={() => isDraggingSeekBar = true}
|
||||
ontouchend={() => isDraggingSeekBar = false}
|
||||
ontouchend={handleSeekBarRelease}
|
||||
class="flex-1 h-1 bg-white/30 rounded-full appearance-none cursor-pointer
|
||||
[&::-webkit-slider-thumb]:appearance-none [&::-webkit-slider-thumb]:w-3 [&::-webkit-slider-thumb]:h-3
|
||||
[&::-webkit-slider-thumb]:bg-white [&::-webkit-slider-thumb]:rounded-full"
|
||||
|
||||
@@ -0,0 +1,218 @@
|
||||
/**
|
||||
* VideoPlayer seek-bar TOUCH scrub regression tests (Android).
|
||||
*
|
||||
* Reported bug: on Android, dragging the progress bar does not change the
|
||||
* playback location.
|
||||
*
|
||||
* The gesture listener lives on the outer container and touch events bubble.
|
||||
* `handleTouchStart` ignores touches that land on a control (the seek bar is an
|
||||
* <input>, inside `data-player-controls`) — but `handleTouchMove` does not, so a
|
||||
* seek-bar drag is still interpreted as a container swipe. That mis-read swipe
|
||||
* fires `togglePlayPause()` (undoing a first-tap toggle that never happened) and
|
||||
* hijacks the drag into brightness control.
|
||||
*
|
||||
* The existing scrub regression tests only drive the slider with MOUSE events,
|
||||
* which never reach the touch handlers — which is why this survived.
|
||||
*
|
||||
* The seek was also committed only from `change`, which Android's WebView does
|
||||
* not reliably fire for a touch interaction on a range input — so a tap moved
|
||||
* the thumb and no seek ever ran. Release now commits from touchend/mouseup too.
|
||||
*
|
||||
* TRACES: UR-005, UR-061 | DR-098, DR-099 | UT-089, UT-090
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
// ---- Mocks (must precede component import) --------------------------------
|
||||
|
||||
const channelHandlers: Record<string, (event: any) => void> = {};
|
||||
vi.mock("@tauri-apps/api/event", () => ({
|
||||
listen: vi.fn(async (channel: string, handler: any) => {
|
||||
channelHandlers[channel] = handler;
|
||||
return () => {
|
||||
delete channelHandlers[channel];
|
||||
};
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("@tauri-apps/api/core", () => ({
|
||||
invoke: vi.fn(),
|
||||
}));
|
||||
|
||||
const playerPlayItem = vi.fn(async () => ({
|
||||
useHtml5Element: false,
|
||||
backend: "exoplayer",
|
||||
state: { kind: "playing" },
|
||||
}));
|
||||
const playerSeekVideo = vi.fn(async (_h: string, position: number) => ({
|
||||
strategy: "native",
|
||||
position,
|
||||
}));
|
||||
const playerStop = vi.fn(async () => ({}));
|
||||
const playerToggle = vi.fn(async () => ({ state: "playing" }));
|
||||
|
||||
vi.mock("$lib/api/bindings", () => ({
|
||||
commands: {
|
||||
playerPlayItem: (...a: any[]) => playerPlayItem(...(a as [])),
|
||||
playerSeekVideo: (...a: any[]) => playerSeekVideo(...(a as [string, number])),
|
||||
playerStop: (...a: any[]) => playerStop(...(a as [])),
|
||||
playerToggle: (...a: any[]) => playerToggle(...(a as [])),
|
||||
playerPlay: vi.fn(async () => ({})),
|
||||
playerPause: vi.fn(async () => ({})),
|
||||
playerSetSleepTimer: vi.fn(async () => ({})),
|
||||
playerCancelSleepTimer: vi.fn(async () => ({})),
|
||||
playerSetSubtitleTrack: vi.fn(async () => ({})),
|
||||
playerSwitchAudioTrack: vi.fn(async () => ({})),
|
||||
storageGetSeriesAudioPreference: vi.fn(async () => null),
|
||||
storageSaveSeriesAudioPreference: vi.fn(async () => ({})),
|
||||
},
|
||||
events: {
|
||||
playerStatusEvent: { listen: vi.fn(async () => () => {}) },
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("$lib/stores/auth", () => ({
|
||||
auth: {
|
||||
getUserId: () => "user-1",
|
||||
getRepository: () => ({
|
||||
getHandle: () => "repo-1",
|
||||
getSubtitleUrl: async () => "",
|
||||
jrayActorsAt: async () => [],
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("$app/navigation", () => ({
|
||||
goto: vi.fn(),
|
||||
}));
|
||||
|
||||
import { render, fireEvent, waitFor } from "@testing-library/svelte";
|
||||
import { tick } from "svelte";
|
||||
import VideoPlayer from "./VideoPlayer.svelte";
|
||||
import type { MediaItem } from "$lib/api/types";
|
||||
|
||||
function makeEpisode(): MediaItem {
|
||||
return {
|
||||
id: "ep1",
|
||||
name: "Episode 1",
|
||||
kind: "episode",
|
||||
durationMs: 24 * 60 * 1000, // 24 min
|
||||
} as MediaItem;
|
||||
}
|
||||
|
||||
async function mountAndroidPlayer() {
|
||||
const utils = render(VideoPlayer, {
|
||||
props: {
|
||||
media: makeEpisode(),
|
||||
streamUrl: "http://server/videos/ep1/master.m3u8",
|
||||
mediaSourceId: "src-1",
|
||||
needsTranscoding: false,
|
||||
onClose: vi.fn(),
|
||||
},
|
||||
});
|
||||
|
||||
await waitFor(() => expect(playerPlayItem).toHaveBeenCalled());
|
||||
await waitFor(() => expect(playerStop).toHaveBeenCalled());
|
||||
|
||||
const slider = utils.container.querySelector(
|
||||
'input[type="range"]'
|
||||
) as HTMLInputElement;
|
||||
const video = utils.container.querySelector("video") as HTMLVideoElement;
|
||||
expect(slider).not.toBeNull();
|
||||
return { ...utils, slider, video };
|
||||
}
|
||||
|
||||
function touch(x: number, y: number) {
|
||||
return { clientX: x, clientY: y } as Touch;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drag the seek bar with TOUCH events, the way a finger does on Android.
|
||||
*
|
||||
* A real drag along the bar moves the finger far enough that the container's
|
||||
* swipe detector (50px) would trigger if it were still listening.
|
||||
*/
|
||||
async function touchScrubTo(
|
||||
slider: HTMLInputElement,
|
||||
video: HTMLVideoElement,
|
||||
target: number
|
||||
) {
|
||||
await fireEvent.touchStart(slider, { touches: [touch(100, 700)] });
|
||||
// Finger travels across the bar. Small vertical wander is normal for a thumb
|
||||
// drag; the horizontal travel is what matters.
|
||||
await fireEvent.touchMove(slider, { touches: [touch(400, 690)] });
|
||||
slider.value = String(target);
|
||||
await fireEvent.input(slider);
|
||||
await fireEvent.touchMove(slider, { touches: [touch(700, 705)] });
|
||||
await fireEvent.change(slider);
|
||||
await fireEvent.touchEnd(slider, { touches: [] });
|
||||
if (video) await fireEvent(video, new Event("seeked"));
|
||||
await tick();
|
||||
}
|
||||
|
||||
describe("VideoPlayer seek bar — touch drag (Android)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
for (const key of Object.keys(channelHandlers)) delete channelHandlers[key];
|
||||
});
|
||||
|
||||
it("a touch drag on the seek bar seeks to the dragged position", async () => {
|
||||
const { slider, video } = await mountAndroidPlayer();
|
||||
|
||||
await touchScrubTo(slider, video, 600);
|
||||
|
||||
await waitFor(() =>
|
||||
expect(playerSeekVideo).toHaveBeenCalledWith("repo-1", 600, "src-1", null, true)
|
||||
);
|
||||
expect(parseFloat(slider.value)).toBeCloseTo(600);
|
||||
});
|
||||
|
||||
it("a touch drag on the seek bar never toggles play/pause", async () => {
|
||||
const { slider, video } = await mountAndroidPlayer();
|
||||
|
||||
await touchScrubTo(slider, video, 600);
|
||||
|
||||
// The container gesture layer must stay out of a control drag entirely:
|
||||
// no swipe mis-read, so no play/pause correction.
|
||||
expect(playerToggle).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("commits the seek on touchend even when the engine never fires `change`", async () => {
|
||||
const { slider, video } = await mountAndroidPlayer();
|
||||
|
||||
// Android's WebView does not reliably fire `change` for a touch interaction
|
||||
// on a range input. A tap on the track still moves the thumb and fires
|
||||
// `input` — the seek must be committed on release regardless.
|
||||
await fireEvent.touchStart(slider, { touches: [touch(400, 700)] });
|
||||
slider.value = "600";
|
||||
await fireEvent.input(slider);
|
||||
await fireEvent.touchEnd(slider, { touches: [] });
|
||||
if (video) await fireEvent(video, new Event("seeked"));
|
||||
await tick();
|
||||
|
||||
await waitFor(() =>
|
||||
expect(playerSeekVideo).toHaveBeenCalledWith("repo-1", 600, "src-1", null, true)
|
||||
);
|
||||
});
|
||||
|
||||
it("commits the seek exactly once when both touchend and change fire", async () => {
|
||||
const { slider, video } = await mountAndroidPlayer();
|
||||
|
||||
await touchScrubTo(slider, video, 600);
|
||||
|
||||
expect(playerSeekVideo).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("a touch drag on the seek bar does not hijack into brightness control", async () => {
|
||||
const { slider, video, container } = await mountAndroidPlayer();
|
||||
|
||||
await touchScrubTo(slider, video, 600);
|
||||
|
||||
// Brightness is applied as a CSS filter on the <video>; a control drag must
|
||||
// leave it untouched.
|
||||
const el = container.querySelector("video") as HTMLVideoElement | null;
|
||||
if (el) {
|
||||
expect(el.style.filter).toBe("brightness(1)");
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user