From b12e99b7e11a3257ed74896830c93f2d1f3eaa51 Mon Sep 17 00:00:00 2001
From: Duncan Tourolle <duncan@tourolle.paris>
Date: Thu, 30 Jul 2026 15:44:38 +0200
Subject: [PATCH] fix(player): keep double-tap seek working over the play
 overlay (DR-098)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The control-surface guard added in the previous commit killed
double-tap-to-seek. The first tap pauses, which renders the full-screen
<button> play overlay over the video, so the SECOND tap lands on a
button — and the guard discarded it as "a tap on a control".

Mark that overlay `data-player-surface`: visually it IS the video, so it
must keep taking tap gestures despite being a <button>. The marker wins
over the interactive-tag check in isControlSurfaceTouch.

Adds VideoPlayer.tapSurface.test.ts, which renders the REAL component
and dispatches real touch/click events at whatever element is genuinely
on top. This is the gap that let four bugs ship in a row: the pure-unit
tests over registerTap/isControlSurfaceTouch/isSynthesizedTouchClick all
passed throughout, because each helper behaved exactly as specified —
every bug was in the composition, i.e. which element actually receives a
tap after Svelte re-renders. Modelling that DOM by hand in a test would
just re-encode the same wrong assumption, so these render it instead.

The new double-tap test was verified to fail with the fix reverted and
pass with it applied, in both directions.
---
 package.json                                  |   2 +-
 src-tauri/Cargo.lock                          |   2 +-
 src-tauri/Cargo.toml                          |   2 +-
 src-tauri/tauri.conf.json                     |   2 +-
 src/lib/components/player/VideoPlayer.svelte  |  18 +-
 .../player/VideoPlayer.tapSurface.test.ts     | 211 ++++++++++++++++++
 src/lib/components/player/tapGestures.ts      |  15 +-
 7 files changed, 240 insertions(+), 12 deletions(-)
 create mode 100644 src/lib/components/player/VideoPlayer.tapSurface.test.ts
diff --git a/package.json b/package.json
index 1d2f0c6f..c714a2de 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "jellytau",
-  "version": "0.2.6",
+  "version": "0.2.7",
   "description": "",
   "type": "module",
   "packageManager": "bun@1.3.5",
diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock
index e1c68c6b..67e05cac 100644
--- a/src-tauri/Cargo.lock
+++ b/src-tauri/Cargo.lock
@@ -1994,7 +1994,7 @@ dependencies = [
 
 [[package]]
 name = "jellytau"
-version = "0.2.6"
+version = "0.2.7"
 dependencies = [
  "aes-gcm",
  "async-trait",
diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml
index 4f588f8c..6e73a381 100644
--- a/src-tauri/Cargo.toml
+++ b/src-tauri/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "jellytau"
-version = "0.2.6"
+version = "0.2.7"
 description = "A Tauri App"
 authors = ["you"]
 edition = "2021"
diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json
index eb2bfc48..f173da03 100644
--- a/src-tauri/tauri.conf.json
+++ b/src-tauri/tauri.conf.json
@@ -1,7 +1,7 @@
 {
   "$schema": "https://schema.tauri.app/config/2",
   "productName": "jellytau",
-  "version": "0.2.6",
+  "version": "0.2.7",
   "identifier": "com.dtourolle.jellytau",
   "build": {
     "beforeDevCommand": "bun run dev",
diff --git a/src/lib/components/player/VideoPlayer.svelte b/src/lib/components/player/VideoPlayer.svelte
index 7e95be49..53e90346 100644
--- a/src/lib/components/player/VideoPlayer.svelte
+++ b/src/lib/components/player/VideoPlayer.svelte
@@ -1448,7 +1448,11 @@
    * `isControlSurfaceTouch` needs, so the rule itself stays DOM-free and testable.
    */
   function ancestorChain(target: EventTarget | null) {
-    const chain: Array<{ tag: string; isPlayerControls?: boolean }> = [];
+    const chain: Array<{
+      tag: string;
+      isPlayerControls?: boolean;
+      isPlayerSurface?: boolean;
+    }> = [];
     let node = target as HTMLElement | null;
     // Bounded walk: controls live a few levels below the player root, and
     // stopping at <body> keeps this cheap and avoids depending on a bound ref.
@@ -1456,6 +1460,7 @@
       chain.push({
         tag: node.tagName ?? "",
         isPlayerControls: node.dataset?.playerControls !== undefined,
+        isPlayerSurface: node.dataset?.playerSurface !== undefined,
       });
       node = node.parentElement;
     }
@@ -1807,10 +1812,15 @@
         <div class="w-12 h-12 border-4 border-white border-t-transparent rounded-full animate-spin"></div>
       </div>
     {:else if !isPlaying}
-      <!-- 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). -->
+      <!-- Play overlay. Visually this IS the video surface, so it is marked
+           `data-player-surface`: it must keep participating in tap gestures even
+           though it is a <button>, or the second tap of a double tap (which lands
+           here, because the first tap paused and raised this overlay) is
+           discarded as "a tap on a control" and seeking dies. It still shares the
+           synthesized-click guard, since it appears exactly when a tap pauses.
+           See DR-098. -->
       <button
+        data-player-surface
         class="absolute inset-0 flex items-center justify-center bg-black/30"
         onclick={handleSurfaceClick}
         aria-label="Play"
diff --git a/src/lib/components/player/VideoPlayer.tapSurface.test.ts b/src/lib/components/player/VideoPlayer.tapSurface.test.ts
new file mode 100644
index 00000000..49631f79
--- /dev/null
+++ b/src/lib/components/player/VideoPlayer.tapSurface.test.ts
@@ -0,0 +1,211 @@
+/**
+ * Behavioural regression tests for the video tap surface — rendered against the
+ * REAL component, not a hand-modelled DOM.
+ *
+ * TRACES: UR-005, UR-061 | DR-098 | UT-092
+ *
+ * Why this file exists:
+ *
+ * `tapGestures.test.ts` tests `registerTap` / `isControlSurfaceTouch` /
+ * `isSynthesizedTouchClick` as isolated pure functions. Every one of those tests
+ * passed while, on the device, in sequence: the player pause-looped, then
+ * pausing became impossible, then the bottom controls went dead, then
+ * double-tap-to-seek stopped working. The helpers were each behaving exactly as
+ * specified — the bugs were all in the *composition*: which element actually
+ * receives a tap once Svelte has re-rendered.
+ *
+ * Testing my own helpers could not catch that, and modelling the DOM by hand in
+ * a test just re-encodes the same wrong assumption. So these tests render
+ * VideoPlayer and dispatch real touch/click events at whatever element is
+ * genuinely on top, asserting user-visible outcomes ("a double tap seeks")
+ * rather than internals.
+ *
+ * The specific traps encoded here, each a bug that shipped:
+ *   - pausing renders a full-screen <button> play overlay OVER the video, so the
+ *     second tap of a double tap lands on a button, not the video;
+ *   - the browser synthesizes a `click` after a touch tap, which must not toggle
+ *     a second time, on ANY layered target;
+ *   - the bottom controls bar must drive its own buttons and NOT the container's
+ *     tap gestures.
+ */
+
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { render } from "@testing-library/svelte";
+import { tick } from "svelte";
+import VideoPlayer from "./VideoPlayer.svelte";
+import { SEEK_FORWARD_SECONDS } from "./tapGestures";
+
+// --- Mocks: everything VideoPlayer reaches for that is not the tap surface. ---
+
+const toggleSpy = vi.fn();
+const seekVideoSpy = vi.fn();
+const seekSpy = vi.fn();
+
+vi.mock("$app/navigation", () => ({ goto: vi.fn() }));
+
+vi.mock("$lib/player", () => ({
+  playerController: {
+    toggle: (...a: unknown[]) => {
+      toggleSpy(...a);
+      return Promise.resolve();
+    },
+    seekVideo: (...a: unknown[]) => {
+      seekVideoSpy(...a);
+      return Promise.resolve();
+    },
+    seek: (...a: unknown[]) => {
+      seekSpy(...a);
+      return Promise.resolve();
+    },
+    setActiveAdapter: vi.fn(),
+    clearActiveAdapter: vi.fn(),
+    getActiveAdapter: vi.fn(() => null),
+  },
+}));
+
+vi.mock("$lib/player/adapters/rustReportHost", () => ({
+  createRustReportHost: () => ({
+    onState: vi.fn(),
+    onPosition: vi.fn(),
+    onMediaLoaded: vi.fn(),
+    onEnded: vi.fn(),
+    onError: vi.fn(),
+    onStreamUrlChanged: vi.fn(),
+    onBuffering: vi.fn(),
+    onReady: vi.fn(),
+  }),
+}));
+
+vi.mock("$lib/player/html5Adapter", () => ({
+  reportState: vi.fn(),
+  reportPosition: vi.fn(),
+  reportMediaLoaded: vi.fn(),
+  resetReporting: vi.fn(),
+}));
+
+vi.mock("$lib/utils/pictureInPicture", () => ({
+  isPipSupported: () => false,
+  enterPip: vi.fn(),
+  setAutoEnterEnabled: vi.fn(),
+}));
+
+vi.mock("$lib/stores/auth", () => ({
+  auth: {
+    getRepository: () => ({ getHandle: () => "h", jrayActorsAt: async () => [] }),
+    subscribe: (fn: (v: unknown) => void) => {
+      fn({ isAuthenticated: true });
+      return () => {};
+    },
+  },
+}));
+
+const MEDIA = {
+  id: "item-1",
+  name: "Test Episode",
+  type: "Episode",
+  runTimeTicks: 6_000_000_000, // 600s
+} as any;
+
+/** Dispatch a touch at (x, y) on whatever element is topmost there. */
+function touchAt(el: Element, x: number) {
+  const touch = { clientX: x, clientY: 300 } as Touch;
+  el.dispatchEvent(
+    new TouchEvent("touchstart", {
+      bubbles: true,
+      cancelable: true,
+      touches: [touch] as unknown as Touch[],
+    })
+  );
+}
+
+function renderPlayer() {
+  return render(VideoPlayer, {
+    props: { media: MEDIA, streamUrl: "http://x/master.m3u8", onClose: vi.fn() },
+  });
+}
+
+describe("VideoPlayer tap surface (real component)", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("a single tap on the video toggles play/pause exactly once", async () => {
+    const { container } = renderPlayer();
+    const video = container.querySelector("video");
+    expect(video).toBeTruthy();
+
+    touchAt(video!, 900);
+
+    expect(toggleSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("the synthesized click after a tap does not toggle a second time", async () => {
+    const { container } = renderPlayer();
+    const video = container.querySelector("video")!;
+
+    touchAt(video, 900);
+    // The compatibility click the browser fires after a touch tap. detail=0 is
+    // how engines mark it; a late real-detail click is covered by the recency
+    // guard, which this exercises too since it lands immediately.
+    video.dispatchEvent(new MouseEvent("click", { bubbles: true, detail: 0 }));
+
+    expect(toggleSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("a double tap seeks even though the first tap raised the play overlay", async () => {
+    // THE regression this file exists for. On device the first tap pauses, which
+    // makes Svelte render a full-screen <button> play overlay over the video —
+    // so the SECOND tap lands on a button, not the video. A control-surface
+    // guard that does not know about that overlay discards it and seeking dies.
+    //
+    // Reproducing it requires the overlay to actually render, which means
+    // driving `isPlaying` the way the real element does: via its `pause` event.
+    vi.useFakeTimers();
+    try {
+      const { container } = renderPlayer();
+      const video = container.querySelector("video")!;
+
+      // Tap 1 on the video.
+      touchAt(video, 900);
+
+      // The element reports it paused → isPlaying=false → overlay renders.
+      video.dispatchEvent(new Event("pause"));
+      await Promise.resolve();
+      await tick();
+
+      const overlay = container.querySelector("[data-player-surface]");
+      expect(overlay, "the play overlay should be covering the video").toBeTruthy();
+
+      vi.advanceTimersByTime(120); // inside DOUBLE_TAP_WINDOW_MS
+      // Tap 2 lands on the OVERLAY, exactly as on device.
+      touchAt(overlay!, 900);
+
+      // Either seek route is acceptable — which one runs depends on whether a
+      // video adapter is registered. What must hold is that a seek happened, to
+      // roughly the forward-skip target.
+      const calls = [...seekVideoSpy.mock.calls, ...seekSpy.mock.calls];
+      expect(calls.length).toBe(1);
+      const [position] = calls[0];
+      expect(position).toBeGreaterThan(0);
+      expect(position).toBeLessThanOrEqual(SEEK_FORWARD_SECONDS);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it("tapping the bottom play/pause button toggles once, not twice", async () => {
+    const { container } = renderPlayer();
+    const controls = container.querySelector("[data-player-controls]");
+    expect(controls).toBeTruthy();
+
+    const playBtn = controls!.querySelector("button");
+    expect(playBtn).toBeTruthy();
+
+    // A real press: touchstart bubbles to the container's gesture handler, then
+    // the button's own click fires. Only ONE toggle may result.
+    touchAt(playBtn!, 40);
+    playBtn!.dispatchEvent(new MouseEvent("click", { bubbles: true, detail: 1 }));
+
+    expect(toggleSpy).toHaveBeenCalledTimes(1);
+  });
+});
diff --git a/src/lib/components/player/tapGestures.ts b/src/lib/components/player/tapGestures.ts
index f0007314..541d5f48 100644
--- a/src/lib/components/player/tapGestures.ts
+++ b/src/lib/components/player/tapGestures.ts
@@ -45,12 +45,19 @@ export const TOUCH_CLICK_SUPPRESS_MS = 700;
  * testable without a DOM.
  */
 export function isControlSurfaceTouch(
-  ancestors: Array<{ tag: string; isPlayerControls?: boolean }>
+  ancestors: Array<{ tag: string; isPlayerControls?: boolean; isPlayerSurface?: boolean }>
 ): boolean {
   const INTERACTIVE = new Set(["button", "a", "input", "select", "textarea", "label"]);
-  return ancestors.some(
-    (node) => node.isPlayerControls === true || INTERACTIVE.has(node.tag.toLowerCase())
-  );
+  for (const node of ancestors) {
+    // `data-player-surface` wins over the tag check: the full-screen play overlay
+    // is a <button> but is visually the video itself, and must keep taking tap
+    // gestures — otherwise the second tap of a double tap (which lands on it,
+    // because the first tap paused and raised it) is discarded and seeking dies.
+    if (node.isPlayerSurface === true) return false;
+    if (node.isPlayerControls === true) return true;
+    if (INTERACTIVE.has(node.tag.toLowerCase())) return true;
+  }
+  return false;
 }
 
 /**
