booru-viewer/booru_viewer
pax 31d02d3c7b Popout/grid: stop calling _on_post_activated twice per keyboard nav
Pre-existing bug in `_navigate_preview` that surfaced after the
preceding perf round shifted timing enough to expose the race. For
every tab, `_navigate_preview` was calling `grid._select(idx)`
followed by an explicit activate-handler call:

    self._grid._select(idx)
    self._on_post_activated(idx)   # ← redundant

`grid._select(idx)` ends with `self.post_selected.emit(index)`,
which is wired to `_on_post_selected` (or the bookmark/library
equivalents), which already calls `_on_post_activated` after a
multi-select length check that's always 1 here because `_select`
calls `_clear_multi` first. So the activation handler ran TWICE per
keyboard nav.

Each `_on_post_activated` schedules an async `_load`, which fires
`image_done` → `_on_image_done` → `_update_fullscreen` →
`set_media` → `_video.stop()` + `_video.play_file(path)`. Two
activations produced two `set_media` cycles in quick succession.

The stale-eof suppression race:

  1. First `play_file` opens window A: `_eof_ignore_until = T+250ms`
  2. Second `play_file` runs ~10-50ms later
  3. Inside the second `play_file`: `_eof_pending = False` runs
     BEFORE `_eof_ignore_until` is reset
  4. Window A may have already expired by this point if the load
     was slow
  5. An async `eof-reached=True` event from the second
     `_video.stop()` lands in the un-armed gap
  6. The gate check `monotonic() < _eof_ignore_until` fails (window A
     expired, window B not yet open)
  7. `_eof_pending = True` sticks
  8. Next `_poll` cycle: `_handle_eof` sees Loop=Next, emits
     `play_next` → `_on_video_end_next` → `_navigate_preview(1, wrap=True)`
     → ANOTHER post advance
  9. User pressed Right once, popout skipped a post

Random and timing-dependent. Hard to reproduce manually but happens
often enough to be visible during normal browsing.

Fix: stop calling the activation handler directly after `_select`.
The signal chain handles it. Applied to all five sites in
`_navigate_preview`:

  - browse view (line 2046-2047)
  - bookmarks view normal nav (line 2024-2025)
  - bookmarks view wrap-edge (line 2028-2029)
  - library view normal nav (line 2036-2037)
  - library view wrap-edge (line 2040-2041)

The wrap-edge cases were called out in the original plan as "leave
alone for scope creep" but they have the same duplicate-call shape
and the same race exposure during auto-advance from EOF. Fixing
them keeps the code consistent and removes a latent bug from a
less-traveled path.

Verified by reading: `_grid._select(idx)` calls `_clear_multi()`
first, so by the time `post_selected` fires, `selected_indices`
returns `[idx]` (length 1), `_on_post_selected`'s multi-select
early-return doesn't fire, and `_on_post_activated(index)` is
always called. Same for the bookmark/library `_on_selected` slots
which have no early-return at all.

Net: ~5 lines deleted, ~25 lines of comments added explaining the
race and the trust-the-signal-chain rule for future contributors.
2026-04-08 02:34:09 -05:00
..