Skip to content

Add handleSpeedChange hook to site handler API#1457

Merged
igrigorik merged 1 commit into
masterfrom
claude/audit-1185-review-DfzB2
Mar 27, 2026
Merged

Add handleSpeedChange hook to site handler API#1457
igrigorik merged 1 commit into
masterfrom
claude/audit-1185-review-DfzB2

Conversation

@igrigorik

@igrigorik igrigorik commented Mar 27, 2026

Copy link
Copy Markdown
Owner

Route all video.playbackRate assignments through the site handler
system, matching the existing handleSeek pattern. This gives site
handlers a clean way to sync speed changes with custom player APIs
(e.g. YouTube subtitle sync per #1185) without global monkey-patching.

  • Add handleSpeedChange(video, speed) to BaseSiteHandler (default: sets
    video.playbackRate)
  • Add delegation in SiteHandlerManager
  • Wire ActionHandler.setSpeed() through the handler
  • Wire EventManager fight-back and cooldown-restore through the handler
  • Remove dead onPlayerStateChange from YouTubeHandler
  • Add tests for all paths (base, delegation, setSpeed, fight-back,
    cooldown, custom override)
  • Add docs/custom-player-handler.md developer guide

Problem
-------
PR #1185 proposed fixing YouTube subtitle desync by monkey-patching
window.setTimeout globally and adding a second MAIN-world content
script to the manifest. The fix worked but the approach was wrong:
global timer patching is fragile, the delay%10 heuristic depends on
YouTube internals, and it bypassed the existing site handler system
that was designed for exactly this kind of site-specific adaptation.

Root cause
----------
When VSC sets video.playbackRate directly, YouTube's internal subtitle
scheduler doesn't know about it. YouTube schedules caption refreshes
based on its own assumed playback rate, so at high speeds the video
outruns the captions.

The real fix is to let site handlers participate in speed changes so
they can sync with a site's custom player API. But the handler system
had no hook for it — despite being the extension's primary operation.

Design
------
The site handler API already had handleSeek (total replacement — the
handler owns the operation, base class provides default behavior,
overrides replace it entirely). Speed changes had no equivalent: all
three places that assigned video.playbackRate did so directly,
bypassing the handler system.

This change adds handleSpeedChange(video, speed) following the same
pattern:

  BaseSiteHandler.handleSpeedChange(video, speed)
    Default: video.playbackRate = speed

  SiteHandlerManager.handleSpeedChange(video, speed)
    Delegates to current handler

All video.playbackRate assignments now route through this hook:
  - ActionHandler.setSpeed() — the primary speed change path
  - EventManager cooldown restore — site pushed back during cooldown
  - EventManager fight-back — site reset speed, extension restores it

Fight-back safety: handleSpeedChange is always called inside the
cooldown window. The cooldown guard at the top of handleRateChange
prevents re-entry, so even if a handler's implementation triggers a
ratechange event, it won't loop.

What this enables
-----------------
A YouTube handler override can now sync with YouTube's player:

  handleSpeedChange(video, speed) {
    video.playbackRate = speed;
    const player = document.querySelector('#movie_player');
    if (player?.setPlaybackRate) player.setPlaybackRate(speed);
  }

This makes YouTube's subtitle scheduler aware of the real speed —
no timer hacking, no manifest changes, no global patches. That
override is left for a follow-up; this commit provides the
infrastructure.

Cleanup
-------
Removed YouTubeHandler.onPlayerStateChange() — dead code, defined
but never called from anywhere.

Handler API surface after this change
-------------------------------------
  Lifecycle:    initialize(doc), cleanup()
  Discovery:    shouldIgnoreVideo(video), getVideoContainerSelectors(),
                detectSpecialVideos(doc)
  UI:           getControllerPosition(parent, video)
  Playback:     handleSpeedChange(video, speed), handleSeek(video, sec)

See docs/custom-player-handler.md for the full developer guide.

Files changed
-------------
  src/site-handlers/base-handler.js   — new handleSpeedChange method
  src/site-handlers/index.js          — delegation in manager
  src/core/action-handler.js          — setSpeed routes through handler
  src/utils/event-manager.js          — fight-back routes through handler
  src/site-handlers/youtube-handler.js — remove dead onPlayerStateChange
  docs/custom-player-handler.md       — developer guide
  tests/unit/site-handlers/           — 7 tests covering all paths
@igrigorik
igrigorik force-pushed the claude/audit-1185-review-DfzB2 branch from 9cff13e to ad511db Compare March 27, 2026 06:27
@igrigorik
igrigorik merged commit 28f0ef2 into master Mar 27, 2026
2 checks passed
@igrigorik
igrigorik deleted the claude/audit-1185-review-DfzB2 branch March 27, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants