fix(controller): keep popup pause and press handling reliable (#324)

changes/macos-controller-double-press.md

@@ -0,0 +1,5 @@
+type: fixed
+area: overlay
+
+- On macOS and Windows, one controller press no longer runs its action twice, which cancelled out toggles like mpv pause.
+- On macOS, the press that brings the overlay forward from mpv no longer runs its action a second time.

changes/popup-pause-controller-lookup.md

@@ -0,0 +1,4 @@
+type: fixed
+area: overlay
+
+- Fixed controller and keyboard lookups sometimes pausing playback without a popup, leaving playback paused after the popup closed until the mouse moved, or not pausing when the popup opened.

e2e/playback.e2e.test.ts

@@ -15,7 +15,7 @@ let started: Promise<Started> | undefined;
 
 function useSession(): Promise<Started> {
   started ??= (async () => {
-    const session = await startE2eSession();
+    const session = await startE2eSession({ config: { controller: { enabled: true } } });
     try {
       return { session, overlay: await session.openPage(OVERLAY_PAGE) };
     } catch (error) {
@@ -273,6 +273,64 @@ test(
   },
 );
 
+// macOS and Windows keep a hidden modal renderer alive, and hidden windows keep polling because
+// background throttling is off. A fake pad goes to every renderer, as a real pad would.
+// Linux has no such window, and under Xvfb without a window manager the overlay reports hidden,
+// so requestAnimationFrame and with it the controller poll never run. Bun's node:test ignores the
+// `skip` option, so this picks `test.skip` instead.
+const testOutsideLinux = process.platform === 'linux' ? test.skip : test;
+testOutsideLinux(
+  'one controller press toggles mpv pause once across every window',
+  { timeout: TEST_TIMEOUT_MS },
+  async () => {
+    const { session } = await useSession();
+    const START_BUTTON = 9;
+    const targets = (await listCdpTargets(session.cdpPort)).filter((target) =>
+      target.url.includes('renderer/index.html'),
+    );
+    const renderers = await Promise.all(targets.map(connectCdpPage));
+    const everyRenderer = (expression: string) =>
+      Promise.all(renderers.map((renderer) => renderer.evaluate(expression)));
+    const nextPolls = `new Promise((done) =>
+      requestAnimationFrame(() => requestAnimationFrame(() => done(true))))`;
+    try {
+      await everyRenderer(`(() => {
+        window.__fakePadPressed = false;
+        Object.defineProperty(navigator, 'getGamepads', {
+          configurable: true,
+          value: () => [{
+            id: 'E2E Pad (STANDARD GAMEPAD)',
+            index: 0,
+            connected: true,
+            mapping: 'standard',
+            axes: [0, 0, 0, 0],
+            buttons: Array.from({ length: 17 }, (_, index) => {
+              const pressed = window.__fakePadPressed && index === ${START_BUTTON};
+              return { value: pressed ? 1 : 0, pressed, touched: pressed };
+            }),
+          }],
+        });
+        return true;
+      })()`);
+      await everyRenderer(nextPolls);
+      await session.mpv.command('set_property', 'pause', true);
+
+      await everyRenderer('window.__fakePadPressed = true');
+      await everyRenderer(nextPolls);
+      await everyRenderer('window.__fakePadPressed = false');
+      await everyRenderer(nextPolls);
+      // Long enough for a second `cycle pause` to reach mpv and cancel the first.
+      await new Promise((done) => setTimeout(done, 300));
+
+      assert.equal(await session.mpv.command('get_property', 'pause'), false);
+    } finally {
+      await everyRenderer('delete navigator.getGamepads');
+      renderers.forEach((renderer) => renderer.close());
+      await session.mpv.command('set_property', 'pause', true);
+    }
+  },
+);
+
 // Runs last: it leaves the lookup popup open over the overlay.
 test(
   'holding Shift over a word opens the Yomitan popup with its dictionary entry',

src/renderer/handlers/controller-press-replay.test.ts

@@ -20,12 +20,39 @@ function pad(pressed: number[] = [], mapping = 'standard'): GamepadLike {
 const pressedIndices = (gamepad: GamepadLike | null | undefined): number[] =>
   gamepad?.buttons.flatMap((button, index) => (button.pressed ? [index] : [])) ?? [];
 
-test('a queued press reads as pressed for exactly one poll', () => {
+test('a queued press the pad never reports is released after the window', () => {
   const replay = createControllerPressReplay();
   replay.queue(0, 1000);
 
   assert.deepEqual(pressedIndices(replay.apply([pad()], 1010)[0]), [0]);
-  assert.deepEqual(pressedIndices(replay.apply([pad()], 1026)[0]), []);
+  assert.deepEqual(pressedIndices(replay.apply([pad()], 1026)[0]), [0]);
+  assert.deepEqual(pressedIndices(replay.apply([pad()], 1499)[0]), [0]);
+  assert.deepEqual(pressedIndices(replay.apply([pad()], 1500)[0]), []);
+});
+
+// The helper's IPC beats Chromium, which only starts reading the pad once the overlay has focus.
+// Going unpressed before the late report would give the controller a second press edge.
+test('a press the pad reports late reads as one continuous press', () => {
+  const replay = createControllerPressReplay();
+  replay.queue(11, 1000);
+
+  const pressedAt = (real: number[], now: number) =>
+    pressedIndices(replay.apply([pad(real)], now)[0]);
+  assert.deepEqual(pressedAt([], 1016), [11]);
+  assert.deepEqual(pressedAt([], 1032), [11]);
+  assert.deepEqual(pressedAt([11], 1048), [11]);
+  assert.deepEqual(pressedAt([11], 1064), [11]);
+  assert.deepEqual(pressedAt([], 1080), []);
+});
+
+test('a fresh press after the replayed one was released reads as a new press', () => {
+  const replay = createControllerPressReplay();
+  replay.queue(11, 1000);
+
+  assert.deepEqual(pressedIndices(replay.apply([pad([11])], 1016)[0]), [11]);
+  assert.deepEqual(pressedIndices(replay.apply([pad()], 1032)[0]), []);
+  assert.deepEqual(pressedIndices(replay.apply([pad([11])], 1048)[0]), [11]);
+  assert.deepEqual(pressedIndices(replay.apply([pad()], 1064)[0]), []);
 });
 
 test('a press the Gamepad API already reported is not replayed', () => {

src/renderer/handlers/controller-press-replay.ts

@@ -1,16 +1,18 @@
 import type { GamepadLike } from './gamepad-controller.js';
 
 /**
- * How far back a press the Gamepad API reported itself still counts as the same press the
- * main process is replaying. Covers the helper -> main -> renderer IPC hop.
+ * How far apart the helper's report and the Gamepad API's report of one physical press can be.
+ * The Gamepad API can lead (the helper -> main -> renderer IPC hop) or lag (Chromium only reads
+ * the pad once the overlay has focus).
  */
 const SAME_PRESS_WINDOW_MS = 500;
 
 /**
  * Replays macOS background presses (see `src/main/runtime/macos-controller-watch.ts`) into the
- * controller poll. A queued button reads as pressed for one poll, so the controller sees a
- * normal press edge; the next poll reads the real pad again, so a button still held does not
- * fire twice. A press the Gamepad API already reported is dropped.
+ * controller poll. A queued button reads as pressed until the Gamepad API has reported that
+ * press and its release, or until the window passes if it never does. Holding it across the gap
+ * before a late report keeps one physical press to one press edge. A press the Gamepad API
+ * already reported is dropped.
  *
  * Only a lone standard-mapping pad gets the replay: the press carries no pad identity, so with
  * several visible it could run an action on the wrong one. With none visible yet, or a pad
@@ -18,6 +20,7 @@ const SAME_PRESS_WINDOW_MS = 500;
  */
 export function createControllerPressReplay() {
   const pendingPresses: Array<{ buttonIndex: number; queuedAt: number }> = [];
+  const heldPresses = new Map<number, { releaseAt: number; realPressSeen: boolean }>();
   const lastRealPressAt = new Map<number, number>();
 
   function recordRealPresses(gamepads: GamepadLike[], now: number): void {
@@ -55,20 +58,37 @@ export function createControllerPressReplay() {
         Boolean(gamepad?.connected),
       );
       recordRealPresses(connected, now);
-      if (pendingPresses.length === 0) return gamepads;
+      if (pendingPresses.length === 0 && heldPresses.size === 0) return gamepads;
 
-      const replayed = pendingPresses
-        .filter(({ buttonIndex, queuedAt }) => {
-          const seenAt = lastRealPressAt.get(buttonIndex);
-          return seenAt === undefined || seenAt < queuedAt - SAME_PRESS_WINDOW_MS;
-        })
-        .map(({ buttonIndex }) => buttonIndex);
+      for (const { buttonIndex, queuedAt } of pendingPresses) {
+        const seenAt = lastRealPressAt.get(buttonIndex);
+        if (seenAt === undefined || seenAt < queuedAt - SAME_PRESS_WINDOW_MS) {
+          heldPresses.set(buttonIndex, {
+            releaseAt: queuedAt + SAME_PRESS_WINDOW_MS,
+            realPressSeen: false,
+          });
+        }
+      }
       pendingPresses.length = 0;
-      const onlyPad = connected.length === 1 ? connected[0] : undefined;
-      if (replayed.length === 0 || onlyPad?.mapping !== 'standard') return gamepads;
 
+      const onlyPad = connected.length === 1 ? connected[0] : undefined;
+      if (onlyPad?.mapping !== 'standard') {
+        heldPresses.clear();
+        return gamepads;
+      }
+
+      for (const [buttonIndex, hold] of heldPresses) {
+        if (onlyPad.buttons[buttonIndex]?.pressed) {
+          hold.realPressSeen = true;
+        } else if (hold.realPressSeen || now >= hold.releaseAt) {
+          heldPresses.delete(buttonIndex);
+        }
+      }
+      if (heldPresses.size === 0) return gamepads;
+
+      const held = [...heldPresses.keys()];
       return gamepads.map((gamepad) =>
-        gamepad === onlyPad ? withPressed(onlyPad, replayed) : gamepad,
+        gamepad === onlyPad ? withPressed(onlyPad, held) : gamepad,
       );
     },
   };

src/renderer/handlers/mouse.test.ts

@@ -147,7 +147,7 @@ function createMouseHarness(
     /** Moves the tracked pointer onto `hovered` and fires a document mousemove. */
     move: (hovered: HoverTarget, clientX = 120, clientY = 240) => {
       harness.hovered = hovered;
-      harness.fire('document', 'mousemove', { clientX, clientY });
+      harness.fire('document', 'mousemove', { clientX, clientY, isTrusted: true });
     },
     isInteractive: () => overlayClassList.contains('interactive'),
     isSecondaryHoverActive: () =>
@@ -357,6 +357,69 @@ test('popup open pauses and popup close resumes when yomitan popup auto-pause is
   assert.deepEqual(h.mpvCommands, [PAUSE, RESUME]);
 });
 
+// The lookup's fallback scan dispatches untrusted mouse events on the selected word, and
+// those bubble to the pointer tracker while the real cursor is elsewhere.
+test('popup close resumes after a fallback scan event lands on the subtitle', async (t) => {
+  const h = createMouseHarness(t, { hoverPause: true, popupPause: true });
+  h.handlers.setupPointerTracking();
+  h.handlers.setupYomitanObserver();
+
+  h.hovered = 'primary';
+  h.fire('document', 'pointermove', { clientX: 120, clientY: 240, isTrusted: false });
+  h.fire('document', 'mousemove', { clientX: 120, clientY: 240, isTrusted: false });
+  await waitForNextTick();
+  assert.equal(h.ctx.state.isOverSubtitle, false);
+
+  h.fire('window', YOMITAN_POPUP_SHOWN_EVENT);
+  await waitForNextTick();
+  h.fire('window', YOMITAN_POPUP_HIDDEN_EVENT);
+
+  assert.deepEqual(h.mpvCommands, [PAUSE, RESUME]);
+});
+
+// Controller and keyboard pause toggles change playback without touching the hover state.
+test('popup open pauses when the user resumed playback during a hover pause', async (t) => {
+  let paused = false;
+  const h = createMouseHarness(t, {
+    hoverPause: true,
+    popupPause: true,
+    getPlaybackPaused: async () => paused,
+  });
+  h.handlers.setupYomitanObserver();
+
+  // `paused` stays false: the pause button resumed playback after this hover pause.
+  await h.handlers.handleMouseEnter();
+  assert.deepEqual(h.mpvCommands, [PAUSE]);
+
+  // The lookup event lands while the shown event's pause check is still in flight.
+  h.popupHostVisible = true;
+  h.fire('window', YOMITAN_POPUP_SHOWN_EVENT);
+  h.fire('window', YOMITAN_LOOKUP_EVENT);
+  await waitForNextTick();
+
+  assert.deepEqual(h.mpvCommands, [PAUSE, PAUSE]);
+});
+
+test('popup taking over a hover pause leaves playback paused without a second command', async (t) => {
+  let paused = false;
+  const h = createMouseHarness(t, {
+    hoverPause: true,
+    popupPause: true,
+    getPlaybackPaused: async () => paused,
+  });
+  h.handlers.setupYomitanObserver();
+
+  await h.handlers.handleMouseEnter();
+  paused = true;
+  h.fire('window', YOMITAN_POPUP_SHOWN_EVENT);
+  await waitForNextTick();
+  await h.handlers.handleMouseLeave();
+  assert.deepEqual(h.mpvCommands, [PAUSE]);
+
+  h.fire('window', YOMITAN_POPUP_HIDDEN_EVENT);
+  assert.deepEqual(h.mpvCommands, [PAUSE, RESUME]);
+});
+
 for (const c of [
   {
     name: 'nested popup close reasserts interactive state and focus when another popup remains visible on Windows',

src/renderer/handlers/mouse.ts

@@ -275,13 +275,14 @@ export function createMouseHandlers(
       return;
     }
 
-    const requestId = ++popupPauseRequestId;
     if (pausedByYomitanPopup) return;
+    const requestId = ++popupPauseRequestId;
 
+    // The popup takes over the hover pause so that closing it owns the resume. A pause toggle
+    // can resume playback without clearing the hover flag, so mpv still decides below.
     if (pausedBySubtitleHover) {
       pausedBySubtitleHover = false;
       pausedByYomitanPopup = true;
-      return;
     }
 
     let paused: boolean | null = null;
@@ -442,16 +443,17 @@ export function createMouseHandlers(
   }
 
   function setupPointerTracking(): void {
-    document.addEventListener('mousemove', (event: MouseEvent) => {
+    // The lookup's fallback scan dispatches untrusted events on the selected word, which say
+    // nothing about where the cursor is. Counting them as hover pauses playback until the next
+    // real pointer move.
+    const trackPointer = (event: MouseEvent | PointerEvent): void => {
+      if (!event.isTrusted) return;
       updatePointerPosition(event);
       syncHoverStateFromTrackedPointer(event);
       maybeResyncPointerHoverState(event);
-    });
-    document.addEventListener('pointermove', (event: PointerEvent) => {
-      updatePointerPosition(event);
-      syncHoverStateFromTrackedPointer(event);
-      maybeResyncPointerHoverState(event);
-    });
+    };
+    document.addEventListener('mousemove', trackPointer);
+    document.addEventListener('pointermove', trackPointer);
     document.addEventListener('visibilitychange', () => {
       if (document.visibilityState !== 'visible') {
         return;

src/renderer/renderer.ts

@@ -657,7 +657,12 @@ function startControllerPolling(): void {
         : ctx.state.kikuModalOpen
           ? fieldGroupingControllerLayout
           : null,
-    getSuspended: () => ctx.state.controllerSuspendedByModalWindow,
+    // macOS and Windows keep the modal window's renderer alive while hidden, and it keeps
+    // polling. It only owns the controller with a modal open; otherwise every press would run
+    // here and in the main overlay.
+    getSuspended: () =>
+      ctx.state.controllerSuspendedByModalWindow ||
+      (ctx.platform.isModalLayer && !isAnyModalOpen()),
   });
 
   const poll = (now: number): void => {