fix(player): tap gestures act immediately, no deferral timer (DR-098)

Tapping the video surface pause-looped: it would unpause and bounce
straight back to paused about a second later. Long-press unpaused fine,
which is what pinned it to the tap path rather than the media pipeline.

The gesture handler deferred the first tap's play/pause behind a 300ms
timer so a second tap could cancel it and seek instead. But the timer
callback cleared its own handle *before* invoking the toggle, and
handleVideoClick used exactly that handle (`tapTimeout !== null`) to
suppress the compatibility click Android's WebView synthesizes after a
touch. So the guard was already open when the late click arrived, and it
toggled a second time.

Replace the deferral with immediate action — there are only first and
second taps:

  1st tap: toggle play/pause
  2nd tap: seek, then toggle play/pause again

The second toggle undoes the first, so a double tap seeks while leaving
the play state exactly as it was: playing jumps and keeps playing,
paused jumps and stays paused. No timer, no window race, no loop.

Click suppression no longer depends on the timer: ignore detail === 0
and any click within 700ms of a touch tap, since Android can deliver the
synthesized click late and with a real detail value.

A swipe now undoes the touchstart toggle (latched on swipeGestureActive
so it happens once, not per touchmove frame), keeping brightness swipes
from changing the play state.

UT-085..087 described the old deferred behaviour and are updated to the
new contract. UT-091 is used for the DR-097 facade tests, since UT-089
and UT-090 were already claimed by extract-traces.test.ts.
This commit is contained in:
2026-07-30 14:53:20 +02:00
parent 79e10d7485
commit b565c4ae6f
11 changed files with 144 additions and 104 deletions
+33 -31
View File
@@ -112,7 +112,10 @@
let touchStartY = $state(0);
let touchStartTime = $state(0);
let tapGestures = createTapGestureState();
let tapTimeout: ReturnType<typeof setTimeout> | null = null;
// When a touch tap last ran the gesture handler, so the compatibility click
// the browser synthesizes afterwards can be ignored (see handleVideoClick).
let lastTouchTapAt = 0;
const TOUCH_CLICK_SUPPRESS_MS = 700;
let brightness = $state(1); // 0-2, default 1
let showDoubleTapFeedback = $state<TapFeedback | null>(null);
let doubleTapFeedbackTimeout: ReturnType<typeof setTimeout> | null = null;
@@ -719,11 +722,6 @@
if (debugLogInterval) {
clearInterval(debugLogInterval);
}
// A deferred single tap must not fire play/pause after teardown.
if (tapTimeout) {
clearTimeout(tapTimeout);
tapTimeout = null;
}
tapGestures.cancel();
if (doubleTapFeedbackTimeout) {
clearTimeout(doubleTapFeedbackTimeout);
@@ -1457,25 +1455,22 @@
now: Date.now(),
});
if (tapTimeout) {
clearTimeout(tapTimeout);
tapTimeout = null;
}
// Suppress the compatibility click this touch will synthesize.
lastTouchTapAt = Date.now();
if (outcome.action === "seek") {
e.preventDefault();
handleDoubleTap(outcome.seekSeconds, outcome.feedback);
// Re-toggle so the first tap's toggle is undone: a double tap seeks and
// leaves the play state as it was (playing keeps playing, paused stays
// paused).
if (outcome.togglePlayPause) togglePlayPause();
return;
}
// Single tap so far: defer play/pause until the double-tap window closes,
// so a double tap seeks without also toggling pause.
tapTimeout = setTimeout(() => {
tapTimeout = null;
if (tapGestures.resolvePending(Date.now())) {
togglePlayPause();
}
}, outcome.pendingAfterMs);
// First tap: act now. Nothing is deferred, so there is no timer to race the
// compatibility click Android synthesizes after a touch tap (see DR-098).
togglePlayPause();
}
function handleTouchMove(e: TouchEvent) {
@@ -1488,14 +1483,16 @@
// Minimum movement to register as swipe (50px)
if (Math.abs(deltaY) > 50 && timeDelta > 50) {
swipeGestureActive = true;
// This is a swipe, not a tap — drop the deferred play/pause.
tapGestures.cancel();
if (tapTimeout) {
clearTimeout(tapTimeout);
tapTimeout = null;
// Only on the frame the gesture is first recognised as a swipe — this runs
// on every touchmove, and the correction below must happen exactly once.
if (!swipeGestureActive) {
// The touchstart already toggled play/pause (taps act immediately now),
// so undo it: a swipe must not change the play state. Forget the tap too,
// so it cannot pair with a later tap into a spurious seek.
togglePlayPause();
tapGestures.cancel();
}
swipeGestureActive = true;
// Brightness control on vertical swipe
swipeType = "brightness";
@@ -1514,14 +1511,19 @@
}
/**
* Mouse clicks toggle play/pause immediately. Touch taps are already handled
* by `handleTouchStart` (which defers play/pause past the double-tap window),
* so the compatibility click that follows a tap must be ignored here —
* otherwise it pauses on the first tap of a double tap.
* Mouse clicks toggle play/pause immediately. Touch taps are handled fully by
* `handleTouchStart`, so the compatibility click the browser synthesizes after
* a tap must be ignored here or every tap toggles twice.
*
* Two independent guards, because neither alone is sufficient: `detail === 0`
* catches the synthesized click on engines that report it, and the recency
* check covers engines that report a real `detail` — Android's WebView can
* deliver the click well after the touch, which is what defeated the previous
* timer-based guard (see DR-098).
*/
function handleVideoClick(e: MouseEvent) {
// A click synthesized from a touch reports no pointer movement detail.
if (e.detail === 0 || tapTimeout !== null) return;
if (e.detail === 0) return;
if (Date.now() - lastTouchTapAt < TOUCH_CLICK_SUPPRESS_MS) return;
togglePlayPause();
}
+45 -27
View File
@@ -27,24 +27,22 @@ function asSeek(outcome: ReturnType<typeof tap>) {
}
describe("tap gesture resolution", () => {
it("defers the single-tap action until the double-tap window has elapsed", () => {
const state = createTapGestureState();
const first = tap(state, RIGHT, 1000);
// Every tap acts IMMEDIATELY — there is no deferral and no timer.
//
// 1st tap: toggle play/pause
// 2nd tap: seek, then toggle play/pause AGAIN
//
// The second toggle undoes the first, so a double tap seeks while leaving the
// play state exactly as it was: playing -> jump and keep playing; paused ->
// jump and stay paused. The old design deferred the first tap behind a 300ms
// timer, which raced the synthesized click and produced a pause/unpause loop.
// The first tap must NOT immediately toggle play/pause — it may still
// become a double tap.
expect(first).toEqual({ action: "pending", pendingAfterMs: DOUBLE_TAP_WINDOW_MS });
it("toggles play/pause immediately on the first tap", () => {
const state = createTapGestureState();
expect(tap(state, RIGHT, 1000)).toEqual({ action: "togglePlayPause" });
});
it("resolves an isolated tap to togglePlayPause once the window expires", () => {
const state = createTapGestureState();
tap(state, RIGHT, 1000);
const resolved = state.resolvePending(1000 + DOUBLE_TAP_WINDOW_MS);
expect(resolved).toEqual({ action: "togglePlayPause" });
});
it("seeks forward 30s on a double tap on the right half and never pauses", () => {
it("seeks forward 30s AND toggles again on a second right-side tap", () => {
const state = createTapGestureState();
tap(state, RIGHT, 1000);
const second = asSeek(tap(state, RIGHT, 1150));
@@ -52,12 +50,11 @@ describe("tap gesture resolution", () => {
expect(second.seekSeconds).toBe(SEEK_FORWARD_SECONDS);
expect(second.seekSeconds).toBe(30);
expect(second.feedback).toBe("right");
// The deferred single-tap pause must have been cancelled.
expect(state.resolvePending(1150 + DOUBLE_TAP_WINDOW_MS)).toBeNull();
// The re-toggle is what preserves the play state across a double tap.
expect(second.togglePlayPause).toBe(true);
});
it("seeks back 10s on a double tap on the left half", () => {
it("seeks back 10s AND toggles again on a second left-side tap", () => {
const state = createTapGestureState();
tap(state, LEFT, 1000);
const second = asSeek(tap(state, LEFT, 1100));
@@ -65,24 +62,44 @@ describe("tap gesture resolution", () => {
expect(second.seekSeconds).toBe(SEEK_BACKWARD_SECONDS);
expect(second.seekSeconds).toBe(-10);
expect(second.feedback).toBe("left");
expect(second.togglePlayPause).toBe(true);
});
it("treats a second tap after the window as a new pending single tap", () => {
it("net play state is unchanged by a double tap (two toggles cancel out)", () => {
const state = createTapGestureState();
let playing = true;
const apply = (outcome: ReturnType<typeof tap>) => {
if (outcome.action === "togglePlayPause") playing = !playing;
else if (outcome.action === "seek" && outcome.togglePlayPause) playing = !playing;
};
apply(tap(state, RIGHT, 1000)); // toggle -> paused
apply(tap(state, RIGHT, 1100)); // seek + toggle -> playing again
expect(playing).toBe(true);
// And from paused, a double tap leaves it paused.
playing = false;
apply(tap(state, RIGHT, 2000));
apply(tap(state, RIGHT, 2100));
expect(playing).toBe(false);
});
it("treats a tap after the window as a fresh first tap", () => {
const state = createTapGestureState();
tap(state, RIGHT, 1000);
const late = tap(state, RIGHT, 1000 + DOUBLE_TAP_WINDOW_MS + 1);
expect(late.action).toBe("pending");
expect(late.action).toBe("togglePlayPause");
});
it("does not treat a third tap as another double tap", () => {
it("only ever has first and second taps — the tap after a pair is a fresh toggle", () => {
const state = createTapGestureState();
tap(state, RIGHT, 1000);
expect(tap(state, RIGHT, 1100).action).toBe("seek");
// Triple tap: the third tap starts a fresh pending tap rather than
// seeking again off the consumed second tap.
expect(tap(state, RIGHT, 1200).action).toBe("pending");
// The pair is consumed. The next tap is a FIRST tap again, so it toggles
// play/pause — there is no "third tap" concept.
expect(tap(state, RIGHT, 1200).action).toBe("togglePlayPause");
});
it("accumulates repeated double taps on the same side", () => {
@@ -105,12 +122,13 @@ describe("tap gesture resolution", () => {
expect(second.feedback).toBe("right");
});
it("cancel() drops a pending tap so an interpreted swipe cannot pause", () => {
it("cancel() makes the next tap a fresh first tap (swipe interrupted the pair)", () => {
const state = createTapGestureState();
tap(state, RIGHT, 1000);
state.cancel();
expect(state.resolvePending(1000 + DOUBLE_TAP_WINDOW_MS)).toBeNull();
// Without cancel() this would have been the seeking second tap.
expect(tap(state, RIGHT, 1100).action).toBe("togglePlayPause");
});
});
+52 -34
View File
@@ -1,16 +1,27 @@
/**
* Tap-gesture interpretation for the video player surface.
*
* Pulled out of `VideoPlayer.svelte` so the timing rules are unit-testable:
* a tap cannot be classified at the moment it lands, because it may still turn
* out to be the first half of a double tap. Play/pause is therefore *deferred*
* until the double-tap window closes, and cancelled outright if a second tap
* arrives otherwise a double tap both toggles pause and seeks.
* Every tap acts IMMEDIATELY there are only first and second taps, and no
* deferral:
*
* TRACES: UR-005, UR-061 | DR-092, DR-095 | UT-085, UT-086, UT-087, UT-088
* 1st tap: toggle play/pause
* 2nd tap (within the window): seek, then toggle play/pause AGAIN
*
* The second toggle undoes the first, so a double tap seeks while leaving the
* play state exactly as it started playing stays playing, paused stays paused.
*
* This replaced a design that deferred the first tap behind a 300ms timer so it
* could be cancelled if a second tap arrived. That deferral raced the
* compatibility `click` Android's WebView synthesizes after a touch tap: the
* timer cleared its own handle *before* running the toggle, reopening the guard
* that was meant to suppress the late click, which then toggled a second time.
* The result was a play/pause loop about a second apart. Acting immediately
* removes the timer, the window race, and the loop.
*
* TRACES: UR-005, UR-061 | DR-092, DR-095, DR-098 | UT-085, UT-086, UT-087, UT-088
*/
/** A second tap within this window makes a double tap. */
/** A second tap within this window pairs with the previous one (seek + re-toggle). */
export const DOUBLE_TAP_WINDOW_MS = 300;
/** Double tap on the right half: skip forward. */
@@ -22,9 +33,18 @@ export const SEEK_BACKWARD_SECONDS = -10;
export type TapFeedback = "left" | "right";
export type TapOutcome =
/** Deferred: play/pause fires only if no second tap lands within the window. */
| { action: "pending"; pendingAfterMs: number }
| { action: "seek"; seekSeconds: number; feedback: TapFeedback };
/** First tap: toggle play/pause right now. */
| { action: "togglePlayPause" }
/**
* Second tap: seek, and toggle play/pause again so the first tap's toggle is
* undone and the play state survives the double tap unchanged.
*/
| {
action: "seek";
seekSeconds: number;
feedback: TapFeedback;
togglePlayPause: true;
};
export interface TapInput {
/** Tap x position, viewport pixels. */
@@ -35,32 +55,20 @@ export interface TapInput {
export interface TapGestureState {
/**
* Resolve a still-pending single tap. Returns the play/pause action once the
* double-tap window has elapsed, or null if there is nothing pending (the tap
* became a double tap, or was cancelled).
* Forget the previous tap, so the next one is treated as a first tap. Used
* when the gesture turns out to be a swipe.
*/
resolvePending(now: number): { action: "togglePlayPause" } | null;
/** Drop any pending tap — used when the gesture turns into a swipe. */
cancel(): void;
}
interface InternalState extends TapGestureState {
lastTapTime: number;
pendingSince: number | null;
}
export function createTapGestureState(): TapGestureState {
const state: InternalState = {
lastTapTime: 0,
pendingSince: null,
resolvePending(now: number) {
if (state.pendingSince === null) return null;
if (now - state.pendingSince < DOUBLE_TAP_WINDOW_MS) return null;
state.pendingSince = null;
return { action: "togglePlayPause" };
},
cancel() {
state.pendingSince = null;
state.lastTapTime = 0;
},
};
@@ -68,27 +76,37 @@ export function createTapGestureState(): TapGestureState {
}
/**
* Classify a tap. The first tap of a potential pair returns `pending` the
* caller schedules `resolvePending` after `pendingAfterMs`. A second tap inside
* the window returns the seek and clears the pending play/pause.
* Classify a tap and return the action to perform *now*.
*
* A tap that closely follows another is the second of a pair: it seeks and
* re-toggles play/pause (undoing the first tap's toggle). Any other tap is a
* first tap and simply toggles. Nothing is deferred, so there is no window to
* race and no third-tap case a consumed pair resets the state.
*/
export function registerTap(state: TapGestureState, input: TapInput): TapOutcome {
const s = state as InternalState;
const sinceLastTap = input.now - s.lastTapTime;
if (s.lastTapTime > 0 && sinceLastTap > 0 && sinceLastTap < DOUBLE_TAP_WINDOW_MS) {
// Second tap: cancel the deferred play/pause and seek instead.
s.pendingSince = null;
s.lastTapTime = 0; // consumed, so a third tap starts fresh
s.lastTapTime = 0; // pair consumed; the next tap is a first tap again
const isLeftSide = input.x < input.screenWidth / 2;
return isLeftSide
? { action: "seek", seekSeconds: SEEK_BACKWARD_SECONDS, feedback: "left" }
: { action: "seek", seekSeconds: SEEK_FORWARD_SECONDS, feedback: "right" };
? {
action: "seek",
seekSeconds: SEEK_BACKWARD_SECONDS,
feedback: "left",
togglePlayPause: true,
}
: {
action: "seek",
seekSeconds: SEEK_FORWARD_SECONDS,
feedback: "right",
togglePlayPause: true,
};
}
s.lastTapTime = input.now;
s.pendingSince = input.now;
return { action: "pending", pendingAfterMs: DOUBLE_TAP_WINDOW_MS };
return { action: "togglePlayPause" };
}
/**
+1 -1
View File
@@ -12,7 +12,7 @@
* derived + merged (remote-session-aware) stores so UI can import state and
* actions from one place, in both local and remote modes.
*
* TRACES: UR-005 | DR-001, DR-009, DR-097 | UT-089
* TRACES: UR-005 | DR-001, DR-009, DR-097 | UT-091
*/
import { get } from "svelte/store";
+1 -1
View File
@@ -1,7 +1,7 @@
/**
* Transport authority: play/pause/toggle are DECIDED in Rust, never in the webview.
*
* TRACES: UR-005 | DR-097 | UT-089
* TRACES: UR-005 | DR-097 | UT-091
*
* The frontend used to short-circuit transport controls whenever a video adapter
* was registered: `toggle()` read `el.paused` off the DOM and flipped the element