fix(player): guard the play overlay against the synthesized touch click

After the DR-098 tap rewrite, pausing became impossible while unpausing
always worked — an asymmetry that pointed straight at the overlay.

Pausing renders a full-screen play-overlay button over the video. The
compatibility click Android synthesizes from the tap arrives ~30-130ms
later, by which time that button exists, so the click lands on the
OVERLAY rather than the <video>. Its onclick called togglePlayPause with
no guard at all, resuming immediately. Unpausing was unaffected because
it removes the overlay, leaving nothing to intercept the click.

The suppression rule was only wired into the video element's handler.
Extract it as isSynthesizedTouchClick() in tapGestures.ts (unit-tested)
and use it from every click target layered over the video, the overlay
included.

Verified: 724 frontend tests pass, svelte-check clean. Bumped to 0.2.5
so the APK installs over 2004.
This commit is contained in:
2026-07-30 15:10:59 +02:00
parent b565c4ae6f
commit b98a530f48
8 changed files with 73 additions and 18 deletions
+1 -1
View File
@@ -248,7 +248,7 @@ Internal architecture, components, and application logic.
| DR-091 | Search result groups split TV into separate Shows and Episodes groups and add a People group (default order: Shows → Episodes → Movies → Songs → Albums → Artists → People); a stored `tvShows` order from before the split expands in place to shows+episodes so an upgrading user keeps their arrangement | UI | UR-060 | Done | | DR-091 | Search result groups split TV into separate Shows and Episodes groups and add a People group (default order: Shows → Episodes → Movies → Songs → Albums → Artists → People); a stored `tvShows` order from before the split expands in place to shows+episodes so an upgrading user keeps their arrangement | UI | UR-060 | Done |
| DR-092 | Video tap gestures resolve in `tapGestures.ts` (pure, unit-tested) rather than inline in `VideoPlayer.svelte`: `registerTap` returns `pending` for a first tap — the component defers `togglePlayPause` behind a `DOUBLE_TAP_WINDOW_MS` (300 ms) timer that a second tap cancels — or `seek` (+30 s right / 10 s left) for a second tap inside the window; a consumed second tap resets the state so a third tap starts fresh, and a swipe cancels the pending tap. The compatibility `click` the browser synthesizes after a touch tap is filtered in `handleVideoClick` so it cannot bypass the deferral. `resolveSeekTarget` converts the delta to the absolute position the facade requires, clamped per DR-095 and chained off a still-in-flight `pendingSeekTarget` so back-to-back skips accumulate instead of all resolving against a not-yet-updated position | UI | UR-061 | Done | | DR-092 | Video tap gestures resolve in `tapGestures.ts` (pure, unit-tested) rather than inline in `VideoPlayer.svelte`: `registerTap` returns `pending` for a first tap — the component defers `togglePlayPause` behind a `DOUBLE_TAP_WINDOW_MS` (300 ms) timer that a second tap cancels — or `seek` (+30 s right / 10 s left) for a second tap inside the window; a consumed second tap resets the state so a third tap starts fresh, and a swipe cancels the pending tap. The compatibility `click` the browser synthesizes after a touch tap is filtered in `handleVideoClick` so it cannot bypass the deferral. `resolveSeekTarget` converts the delta to the absolute position the facade requires, clamped per DR-095 and chained off a still-in-flight `pendingSeekTarget` so back-to-back skips accumulate instead of all resolving against a not-yet-updated position | UI | UR-061 | Done |
| DR-094 | Frontend boundary tripwire (`scripts/check-frontend-boundary.sh`) detects Jellyfin item-type array literals **anywhere** in `src/` rather than only inline at an `includeItemTypes:` query site, so a category→type mapping cannot evade the check by being assigned to a named const (the evasion that let the `scoped-search` leak pass CI); requires two adjacent type literals so single-type presentation and `item.type ===` inspection stay legal, and caps the allowlist to force taxonomy into Rust instead of accumulating exceptions | Tooling | - | Done | | DR-094 | Frontend boundary tripwire (`scripts/check-frontend-boundary.sh`) detects Jellyfin item-type array literals **anywhere** in `src/` rather than only inline at an `includeItemTypes:` query site, so a category→type mapping cannot evade the check by being assigned to a named const (the evasion that let the `scoped-search` leak pass CI); requires two adjacent type literals so single-type presentation and `item.type ===` inspection stay legal, and caps the allowlist to force taxonomy into Rust instead of accumulating exceptions | Tooling | - | Done |
| DR-098 | Video tap gestures act **immediately** — no deferral, no timer, and only first/second taps exist. A first tap toggles play/pause; a second tap inside `DOUBLE_TAP_WINDOW_MS` seeks *and* toggles again, so the two toggles cancel and a double tap preserves the play state (playing → jump and keep playing; paused → jump and stay paused). This replaces a design that deferred the first tap behind a 300 ms timer so a second tap could cancel it: the timer cleared its own handle *before* invoking the toggle, which reopened the `tapTimeout !== null` guard in `handleVideoClick` meant to suppress the compatibility `click` Android's WebView synthesizes after a touch — the late click then toggled a second time, producing a pause/unpause loop (long-press was unaffected, which is what identified the tap path). Click suppression no longer depends on the timer: `handleVideoClick` ignores `detail === 0` *and* any click within `TOUCH_CLICK_SUPPRESS_MS` of a touch tap. A swipe undoes the touchstart toggle exactly once (latched on `swipeGestureActive`) so brightness swipes never change play state | UI | UR-061 | Done | | DR-098 | Video tap gestures act **immediately** — no deferral, no timer, and only first/second taps exist. A first tap toggles play/pause; a second tap inside `DOUBLE_TAP_WINDOW_MS` seeks *and* toggles again, so the two toggles cancel and a double tap preserves the play state (playing → jump and keep playing; paused → jump and stay paused). This replaces a design that deferred the first tap behind a 300 ms timer so a second tap could cancel it: the timer cleared its own handle *before* invoking the toggle, which reopened the `tapTimeout !== null` guard in `handleVideoClick` meant to suppress the compatibility `click` Android's WebView synthesizes after a touch — the late click then toggled a second time, producing a pause/unpause loop (long-press was unaffected, which is what identified the tap path). Click suppression no longer depends on the timer: `handleVideoClick` ignores `detail === 0` *and* any click within `TOUCH_CLICK_SUPPRESS_MS` of a touch tap. A swipe undoes the touchstart toggle exactly once (latched on `swipeGestureActive`) so brightness swipes never change play state. Click suppression is shared by **every** click target layered over the video via `isSynthesizedTouchClick`, not just the `<video>`: pausing renders a full-screen play-overlay button, so the synthesized click lands on *that* and an unguarded handler there resumed immediately — pausing appeared impossible while unpausing worked, because unpausing removes the overlay | UI | UR-061 | Done |
| DR-097 | Transport authority (play/pause/toggle) lives in Rust for **webview-rendered** media, not just native. The controller tracks the state the HTML5 element reports (`html5_playing`, fed by `report_html5_state`, which now *stores* rather than only re-emitting); `play`/`pause`/`toggle_playback` consult it and drive the element by emitting a `ControlCommand` that `playerEvents.handleControlCommand` executes against the active adapter. A `stopped`/`idle` report clears it so the native backend (MPV/ExoPlayer) regains authority for music. The frontend facade no longer short-circuits transport into the adapter: `adapter.toggle()` previously decided play-vs-pause by reading `el.paused` off the DOM, a value that flips transiently while an element buffers or settles a seek — so two intents ~150 ms apart read *different* values, performed *opposing* actions, and self-sustained a play/pause loop needing no further input (observed on Android with a fully-buffered `readyState=4 networkState=1` element). Same "backend decides, adapter executes the primitive" split as `player_seek_video` | Player | UR-005 | Done | | DR-097 | Transport authority (play/pause/toggle) lives in Rust for **webview-rendered** media, not just native. The controller tracks the state the HTML5 element reports (`html5_playing`, fed by `report_html5_state`, which now *stores* rather than only re-emitting); `play`/`pause`/`toggle_playback` consult it and drive the element by emitting a `ControlCommand` that `playerEvents.handleControlCommand` executes against the active adapter. A `stopped`/`idle` report clears it so the native backend (MPV/ExoPlayer) regains authority for music. The frontend facade no longer short-circuits transport into the adapter: `adapter.toggle()` previously decided play-vs-pause by reading `el.paused` off the DOM, a value that flips transiently while an element buffers or settles a seek — so two intents ~150 ms apart read *different* values, performed *opposing* actions, and self-sustained a play/pause loop needing no further input (observed on Android with a fully-buffered `readyState=4 networkState=1` element). Same "backend decides, adapter executes the primitive" split as `player_seek_video` | Player | UR-005 | Done |
| DR-096 | `Html5PlayerAdapter.play()` is resilient to stall recovery: an in-flight attempt is memoised so concurrent callers (UI plus hls.js gap-controller recovery) share one `element.play()` instead of stacking calls, and an `AbortError` ("play() request was interrupted by a call to pause()") is logged at debug rather than pushed to `host.onError`. The browser raises it whenever a pending play promise is superseded by a pause/seek/source change, which hls.js does routinely while nudging past a stall — reporting it surfaced a player error roughly once per second for the whole stall and left the UI stuck showing paused | Player | UR-005 | Done | | DR-096 | `Html5PlayerAdapter.play()` is resilient to stall recovery: an in-flight attempt is memoised so concurrent callers (UI plus hls.js gap-controller recovery) share one `element.play()` instead of stacking calls, and an `AbortError` ("play() request was interrupted by a call to pause()") is logged at debug rather than pushed to `host.onError`. The browser raises it whenever a pending play promise is superseded by a pause/seek/source change, which hls.js does routinely while nudging past a stall — reporting it surfaced a player error roughly once per second for the whole stall and left the UI stuck showing paused | Player | UR-005 | Done |
| DR-095 | Seek targets clamp strictly *inside* the media (`clampSeekTarget`, `END_SEEK_MARGIN_SECONDS` = 6 s ≈ one HLS segment) instead of to the exact `duration`. Landing on the duration makes hls.js request the segment whose start time lies past the end of the media (e.g. a 6330.324 s item → segment 1055 starting at 6336.33 s), which Jellyfin never produces; the fetch times out and hls.js' gap-controller stalls at the last buffered position, presenting as "unpausing or skipping bounces straight back to paused". Applied on both seek paths — the relative-skip `resolveSeekTarget` and the seek-bar drag, whose range input `max` is the duration itself — and floored at 0 so media shorter than the margin still seeks to the start | UI | UR-061 | Done | | DR-095 | Seek targets clamp strictly *inside* the media (`clampSeekTarget`, `END_SEEK_MARGIN_SECONDS` = 6 s ≈ one HLS segment) instead of to the exact `duration`. Landing on the duration makes hls.js request the segment whose start time lies past the end of the media (e.g. a 6330.324 s item → segment 1055 starting at 6336.33 s), which Jellyfin never produces; the fetch times out and hls.js' gap-controller stalls at the last buffered position, presenting as "unpausing or skipping bounces straight back to paused". Applied on both seek paths — the relative-skip `resolveSeekTarget` and the seek-bar drag, whose range input `max` is the duration itself — and floored at 0 so media shorter than the margin still seeks to the start | UI | UR-061 | Done |
+1 -1
View File
@@ -1,6 +1,6 @@
{ {
"name": "jellytau", "name": "jellytau",
"version": "0.2.4", "version": "0.2.5",
"description": "", "description": "",
"type": "module", "type": "module",
"packageManager": "bun@1.3.5", "packageManager": "bun@1.3.5",
+1 -1
View File
@@ -1994,7 +1994,7 @@ dependencies = [
[[package]] [[package]]
name = "jellytau" name = "jellytau"
version = "0.2.4" version = "0.2.5"
dependencies = [ dependencies = [
"aes-gcm", "aes-gcm",
"async-trait", "async-trait",
+1 -1
View File
@@ -1,6 +1,6 @@
[package] [package]
name = "jellytau" name = "jellytau"
version = "0.2.4" version = "0.2.5"
description = "A Tauri App" description = "A Tauri App"
authors = ["you"] authors = ["you"]
edition = "2021" edition = "2021"
+1 -1
View File
@@ -1,7 +1,7 @@
{ {
"$schema": "https://schema.tauri.app/config/2", "$schema": "https://schema.tauri.app/config/2",
"productName": "jellytau", "productName": "jellytau",
"version": "0.2.4", "version": "0.2.5",
"identifier": "com.dtourolle.jellytau", "identifier": "com.dtourolle.jellytau",
"build": { "build": {
"beforeDevCommand": "bun run dev", "beforeDevCommand": "bun run dev",
+12 -13
View File
@@ -25,6 +25,7 @@
registerTap, registerTap,
resolveSeekTarget, resolveSeekTarget,
clampSeekTarget, clampSeekTarget,
isSynthesizedTouchClick,
SEEK_FORWARD_SECONDS, SEEK_FORWARD_SECONDS,
SEEK_BACKWARD_SECONDS, SEEK_BACKWARD_SECONDS,
type TapFeedback, type TapFeedback,
@@ -115,7 +116,6 @@
// When a touch tap last ran the gesture handler, so the compatibility click // When a touch tap last ran the gesture handler, so the compatibility click
// the browser synthesizes afterwards can be ignored (see handleVideoClick). // the browser synthesizes afterwards can be ignored (see handleVideoClick).
let lastTouchTapAt = 0; let lastTouchTapAt = 0;
const TOUCH_CLICK_SUPPRESS_MS = 700;
let brightness = $state(1); // 0-2, default 1 let brightness = $state(1); // 0-2, default 1
let showDoubleTapFeedback = $state<TapFeedback | null>(null); let showDoubleTapFeedback = $state<TapFeedback | null>(null);
let doubleTapFeedbackTimeout: ReturnType<typeof setTimeout> | null = null; let doubleTapFeedbackTimeout: ReturnType<typeof setTimeout> | null = null;
@@ -1513,17 +1513,14 @@
/** /**
* Mouse clicks toggle play/pause immediately. Touch taps are handled fully by * Mouse clicks toggle play/pause immediately. Touch taps are handled fully by
* `handleTouchStart`, so the compatibility click the browser synthesizes after * `handleTouchStart`, so the compatibility click the browser synthesizes after
* a tap must be ignored here or every tap toggles twice. * a tap must be ignored or every tap toggles twice.
* *
* Two independent guards, because neither alone is sufficient: `detail === 0` * Used by EVERY click target layered over the video, not just the <video>:
* catches the synthesized click on engines that report it, and the recency * pausing renders the full-screen play overlay, so the synthesized click lands
* check covers engines that report a real `detail` — Android's WebView can * on that button instead and would re-toggle straight back to playing.
* deliver the click well after the touch, which is what defeated the previous
* timer-based guard (see DR-098).
*/ */
function handleVideoClick(e: MouseEvent) { function handleSurfaceClick(e: MouseEvent) {
if (e.detail === 0) return; if (isSynthesizedTouchClick(e.detail, Date.now(), lastTouchTapAt)) return;
if (Date.now() - lastTouchTapAt < TOUCH_CLICK_SUPPRESS_MS) return;
togglePlayPause(); togglePlayPause();
} }
@@ -1691,7 +1688,7 @@
onwaiting={handleWaiting} onwaiting={handleWaiting}
onplaying={handlePlaying} onplaying={handlePlaying}
onloadstart={handleLoadStart} onloadstart={handleLoadStart}
onclick={handleVideoClick} onclick={handleSurfaceClick}
> >
<!-- Temporarily disabled to debug playback issues <!-- Temporarily disabled to debug playback issues
{#each subtitleTracks() as track} {#each subtitleTracks() as track}
@@ -1784,10 +1781,12 @@
<div class="w-12 h-12 border-4 border-white border-t-transparent rounded-full animate-spin"></div> <div class="w-12 h-12 border-4 border-white border-t-transparent rounded-full animate-spin"></div>
</div> </div>
{:else if !isPlaying} {:else if !isPlaying}
<!-- Play/Pause overlay --> <!-- Play overlay. Must share the touch-click guard: this button appears the
instant a tap pauses, so the synthesized click lands here and would
resume immediately (see DR-098). -->
<button <button
class="absolute inset-0 flex items-center justify-center bg-black/30" class="absolute inset-0 flex items-center justify-center bg-black/30"
onclick={togglePlayPause} onclick={handleSurfaceClick}
aria-label="Play" aria-label="Play"
> >
<svg class="w-20 h-20 text-white" fill="currentColor" viewBox="0 0 24 24"> <svg class="w-20 h-20 text-white" fill="currentColor" viewBox="0 0 24 24">
@@ -8,6 +8,8 @@ import {
resolveSeekTarget, resolveSeekTarget,
clampSeekTarget, clampSeekTarget,
END_SEEK_MARGIN_SECONDS, END_SEEK_MARGIN_SECONDS,
isSynthesizedTouchClick,
TOUCH_CLICK_SUPPRESS_MS,
} from "./tapGestures"; } from "./tapGestures";
const SCREEN_WIDTH = 1000; const SCREEN_WIDTH = 1000;
@@ -197,6 +199,32 @@ describe("seek target resolution", () => {
}); });
}); });
describe("synthesized touch-click suppression", () => {
// Regression: pausing renders a full-screen play-overlay button over the
// video, so the compatibility click Android synthesizes from the tap lands on
// the OVERLAY, not the <video>. With no guard there it re-toggled and undid
// the pause — pausing looked impossible while unpausing worked fine (the
// overlay is removed when playing, so nothing intercepted that direction).
it("suppresses a click with detail 0 (clearly synthesized)", () => {
expect(isSynthesizedTouchClick(0, 10_000, 0)).toBe(true);
});
it("suppresses a real-detail click that closely follows a touch tap", () => {
const tapAt = 10_000;
expect(isSynthesizedTouchClick(1, tapAt + 120, tapAt)).toBe(true);
expect(isSynthesizedTouchClick(1, tapAt + TOUCH_CLICK_SUPPRESS_MS - 1, tapAt)).toBe(true);
});
it("allows a genuine mouse click well after any touch", () => {
const tapAt = 10_000;
expect(isSynthesizedTouchClick(1, tapAt + TOUCH_CLICK_SUPPRESS_MS + 1, tapAt)).toBe(false);
});
it("allows a genuine mouse click when no touch has ever happened", () => {
expect(isSynthesizedTouchClick(1, 10_000, 0)).toBe(false);
});
});
describe("seek target clamping (shared by skip and seek-bar drag)", () => { describe("seek target clamping (shared by skip and seek-bar drag)", () => {
it("keeps a mid-stream target untouched", () => { it("keeps a mid-stream target untouched", () => {
expect(clampSeekTarget(100, 600)).toBe(100); expect(clampSeekTarget(100, 600)).toBe(100);
+28
View File
@@ -24,6 +24,34 @@
/** A second tap within this window pairs with the previous one (seek + re-toggle). */ /** A second tap within this window pairs with the previous one (seek + re-toggle). */
export const DOUBLE_TAP_WINDOW_MS = 300; export const DOUBLE_TAP_WINDOW_MS = 300;
/**
* How long after a touch tap a mouse `click` is assumed to be the compatibility
* event the browser synthesizes from that touch. Android's WebView can deliver it
* noticeably late, so this is generous.
*/
export const TOUCH_CLICK_SUPPRESS_MS = 700;
/**
* Whether a `click` should be ignored because a touch tap already handled it.
*
* EVERY click target layered over the video must consult this not just the
* `<video>` element. Pausing swaps in a full-screen play-overlay button, so the
* synthesized click lands on *that* button rather than the video, and an
* unguarded handler there re-toggles and undoes the pause (pause appeared
* impossible while unpause worked, because unpausing removes the overlay).
*
* `detail === 0` catches the synthesized click on engines that report it; the
* recency check covers engines that report a real `detail`.
*/
export function isSynthesizedTouchClick(
detail: number,
now: number,
lastTouchTapAt: number
): boolean {
if (detail === 0) return true;
return now - lastTouchTapAt < TOUCH_CLICK_SUPPRESS_MS;
}
/** Double tap on the right half: skip forward. */ /** Double tap on the right half: skip forward. */
export const SEEK_FORWARD_SECONDS = 30; export const SEEK_FORWARD_SECONDS = 30;