Skip to content

The polish round of 2026-11-15 - #680

Merged
Nawid3333 merged 17 commits into
mainfrom
polish/handoff
Nov 15, 2026
Merged

Nawid3333 merged 17 commits into
mainfrom
polish/handoff

Conversation

@Nawid3333

Copy link
Copy Markdown
Owner

The polish round of 2026-11-15: two passes over the code (bugs, security, dead code, duplicates, speed), each change checked by a test that fails without it. The details, with the A/B numbers and what was looked at and left, are in docs/notes/player.md ("The polish round") and docs/notes/workflows.md.

Security

  • patched-libraries.yml runs in two jobs, so the write token is kept apart from the install (SEC-3). The recut job has no pnpm cache. The publish step checks the link it posts. The stray-file check also catches renames and mode changes.

Players

  • MP4: a start that gets nowhere waits at the steady pace, not every millisecond. A fragmented file's keyframe index takes in only the new samples.
  • DASH, HLS and MP4 share one fragment status. DASH's dead hooks are removed.
  • A dropped .fsa file is read from the File itself.
  • FastStreamClient:
    • A fragment without times counts in the download window.
    • "Buffer ahead" is taken anew in private windows, for live streams, and when there is no bitrate.
    • The seek preview's listeners stay with the preview they belong to, and are let go when the preview is switched off.
    • A player that cannot take its source cleans up.

content.js

  • Shadow roots: the pop-up guard, the pending-player watch, link clicks, pausing and the media report now reach into them.
  • Play buttons: control-bar words like Display, Autoplay, Replay, Playlist and Playback are no longer clicked as play buttons.
  • A redirect to the player that did not happen no longer stops later leave reports. A slow unload no longer reports the redirect as a leave.

background.mjs

  • "Clear Sources" sticks.
  • "Open link in player" no longer goes to MPV on an allowlisted site, including when the new tab shows about:blank first.
  • An Off no longer reloads an On that came meanwhile.
  • Next-episode requests from a top-frame player are no longer sent to the player itself.

CSS and tools

  • No transition or animation is left under the zero-motion override. Unused rules are removed.
  • New tools/same-code.mjs tells whether a change touches only comments and types.

Checked locally

  • pnpm test: 3915 passed, 6 skipped.
  • lint and typecheck: 0 problems.
  • build:keep works.
  • lint:amo and lint:github: 0 problems.
  • The workflow tests in WSL pass, and actionlint and zizmor are clean.
  • An earlier build passed every non-mpv e2e suite. They are running again on this head.
  • The fixes went through two Claude API reviews; every claim was checked against the code.

🤖 Generated with Claude Code

Nawid3333 and others added 16 commits November 15, 2026 01:37
…tall (SEC-3)

The re-cut installs a new npm release, whose install scripts could read the write token from
the step's environment or plant something in later steps through $GITHUB_ENV. Now "recut"
(contents and issues read) installs, re-cuts and hands each result on as an artifact; "publish"
(the write permissions) installs nothing, checks what it was handed - npm-shaped name and
versions, and a change to the files a re-cut may change only, applied with git am onto main -
and pushes, opens and closes as before. "report" opens the failure issue for either job.

tests/workflows/patched-libraries-publish.test.sh runs the publish step with a real git
repository, a bare origin and a stub gh (19 checks; each guard broken by hand turns it red).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llisecond

While the player waits for its first frame (needsInit) the main loop ran at the urgent pace
(1 ms, 4 ms in Firefox) for as long as that took - for good when the element could not decode
the data. Now only for 2 s after the start last got somewhere (an append, a seek, a play),
then at the steady 50 ms; loadeddata and each append wake the loop at once. LoopPace.mjs holds
the choice (LoopPace.test.mjs, with a property test). A/B against main, 7 rounds, local line:
no measure moved (docs/notes/player.md, "The polish round").

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
refreshSampleIndex filtered and sorted every sample of every track again after each range:
7.7 ms a range on a 2-hour file (338,000 audio samples, each a keyframe; Node, synthetic
samples), on the player's thread. addKeyframes (SampleIndex.mjs) sorts only those mp4box added
since, appends them when they all come later (as while a file plays on), and merges them in
otherwise (an earlier part read after a seek back): 0.75 ms with the merge, about nothing with
the append. mp4box only appends to a track's list (it builds it once, at the moov), so the
result is the full sort's, ties included: SampleIndex.test.mjs checks that for any samples
added in any steps (fast-check); a merge that breaks ties the other way fails it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…SH's dead hooks out

The three fragment requesters each kept a fragment's download status the same way - waiting,
downloading, complete, failed, waiting again after an abort, a stored copy that cannot be read
counting as a failure - in three copies of the same 40 lines. FragmentStatus.mjs holds it once
(FragmentStatus.test.mjs; an abort left failed, not waiting, fails it), and each requester keeps
only what is its own: HLS's decryption and hls.js's answer, MP4's piece-by-piece feed and
fileStart, DASH's plain answer.

DashFragmentRequester and DashLoader gave each download a preProcessor and a postProcessor
that called the player's preProcessFragment/postProcessFragment, which no player has had: the
hooks, their HookedDashPlayer type and the arguments only they read are gone. A DASH download
now has no postProcessor, so DownloadEntry keeps the references to its pieces while it
downloads (the chunks the loader joins anyway; nothing reads them for DASH), as an MP4 range's.
The MP4 requester's orphaned JSDoc block went with it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… its object URL

parseFSAFile made an object URL of the file and RequestUtils.httpGetLarge fetched it in chunks
with Range requests, reading the length back from Content-Range; the URL had to live for the
whole read. It reads each chunk with file.slice().arrayBuffer() now, the next one ahead as
before (LargeBuffer), with the same chunk size; an empty file says so. httpGetLarge had no
other use and is gone, with the two imports only it needed. archive-roundtrip.e2e.mjs opens a
saved archive in 64 KiB chunks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…unused rules and key out

fluidplayer.css declared 20 transitions, 2 animations and the "shaking" keyframes that the
player's global override cancelled: they are gone, with the rules and selectors nothing in the
player uses (.no_volume_bar, the playback-rate menu's, the pseudo poster, .freeze's buffered
bar), and the override is one selector. options.css lost #submitter's three rules. The test
"keep still, for everyone" (playerA11y.test.mjs) now fails on any transition, animation or
@Keyframes that is not none. The locale key options_general_vischangeaction_pip (a choice
removed long ago) is gone from combined-locales.json and the 16 locales, with its commented
line in VisChangeActions.mjs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pes only

node tools/same-code.mjs <base> [path ...] parses each changed .js/.mjs file (untracked new
ones too) as it was at <base> and as it is, and compares the programs without positions,
comments, the parentheses a JSDoc cast needs and a literal's quotes; it names each file whose
code differs, with the line, and exits 1 then. A typing pass checks itself with it. It was a
handoff script that found esbuild inside node_modules/.pnpm; it uses ast-compare.mjs's acorn
parse now, a direct dev dependency.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…as left

docs/notes/player.md gets "The polish round (2026-11-15)": each change of this branch, what was
measured for it (the loop pace's A/B against main, local and on a 12 Mbit/s line: all same; the
keyframe index's 7.7 ms a range), PC-4 and CS-7 measured and left, and what was looked at and
left, each with its reason. docs/review-2026-11-14.md points the review round at this branch,
updates its open lists (SEC-3 built, the needsInit loop, PC-4, the CSS, the background and UI
items as found), and adds the site scripts to the owner's questions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A removed element sent firstDifference to the missing index first, which has no position, so
it reported the enclosing node (often line 1); the list branch meant for it read positions
from the stripped copy and never ran. Lists are now compared over their common part first,
then an added or removed element is located in the parsed tree.

tests/unit/sameCode.test.mjs runs the tool in a temporary repository: comments and casts are
the same code, a change, an addition, a removal, new/untracked/deleted files, the path list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- patched-libraries publish: list changes with --no-renames (a rename showed only its new
  name, so a file off the list could be moved away unseen), take plain files only (no link,
  submodule or executable), and only a link to a release on npm or GitHub. recut keeps no
  pnpm cache (its post step would have saved the store a new release went into under main's
  key) and may read pull requests, which the issues endpoint lists only then. Tests for
  each, each failing when its check is taken out.
- DashLoader passes an identity postProcessor again: without one, every DASH download kept a
  second copy of its pieces while it loaded, which nothing reads.
- same-code: a template's raw text counts (a tag gets it); names with other than ASCII are
  read (core.quotePath off).
- playerA11y: motion is found however it is written (longhands, one line, a duration).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- wantsNetwork places a fragment without times as predownloadFragments does (windowSpan): read
  as -1, it counted as behind the playhead, and a player in a background tab told its peers it
  wanted nothing it would have downloaded.
- "Buffer ahead" is taken anew each tick in a private window, for a live stream and without a
  bitrate: the fit in the RAM share only lowered it, so a share that shrank once held it there,
  and a new "Buffer ahead" waited for the next video.
- The seek preview's context is stored only with the preview it belongs to: a build for a
  video replaced meanwhile took the field over (the shown one's never let go of), and its error
  destroyed the shown preview. A preview's own error lets go of its context; a failed rebuild
  is caught.
- A player that could not take its source lets go of its listeners and its once-a-second wait
  for a picture, as resetPlayer does: after the load error no reset followed.
- Dead: setVolume's branches for Web Audio support changing at run time; three fields never
  read (lastTime, previousLevel, previousAudioLevel).

Each fix has a test that fails without it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Windowed fullscreen and the miniplayer: an exit asked of a state never entered (force
  false, its `active` still undefined) entered it.
- The pop-up guard is armed for a player in a shadow root too (OverlayGuard.focused, as
  PAGE_TYPING looks): the document's active element is the host there.
- A player iframe in a shadow root is seen going: the observer watches each shadow root it is
  in, as an observer of the page sees nothing inside one.
- A generic play button must say "play" as a word: "Display settings", "Autoplay", "Replay",
  "Playlist" and "Playback speed" were clicked instead.
- The next season's link made from the address is clicked in the page where the page's
  <base target> sends links to the whole tab: out of the page it could not navigate.
- A redirect to the player that did not happen ("Leave page?" answered no) no longer stops
  every later leave from being reported.
- A link clicked inside an open shadow root is found by the click's composed path.
- pauseAllWithin and loadedMedia look into closed shadow roots, as pauseAllMedia does.
- Dead: sendToPlayer's array and 'all' forms (one frame always); two fields never read.

The test page (contentDom.mjs) now runs a MutationObserver only for changes in the tree it
watches, not across a shadow root, as Firefox does, and matches classes and [attr*=] with
the i flag. Each fix has a test that fails without it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- "Clear Sources" in a player tab stays cleared: the tab is sent other tabs' streams too, the
  whole of a tab's list each time it finds one, and the next one brought the cleared ones back.
  What was found before the clear is left out (TabHolder.sourcesClearedAt).
- "Open link in player" keeps the player on a site the MPV Allowlist names: the list started
  MPV on the new tab's first page over the choice.
- An Off whose question to the tab outlasted an On clicked meanwhile no longer reloads that
  On's player away.
- The next-episode request of a player in a tab's top frame is answered 'error', as its poll
  is: the click was asked of the player's own frame, where nothing answers.
- Dead: loadCustomPatternsFile's isPrimary.

Tests for each but the Off/On race (the tab's question only waits after a restart).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- content.js: the play-button filter takes the words that only hold
  "play" out (display, autoplay, replay, playlist, playback, speed,
  rate, player) in any case and looks for "play" in what is left, so
  lower-case compounds (playbtn, bigplay) count again and camelCase
  controls (autoPlayToggle, playList) do not.
- content.js: the redirect to the player owns only the first
  beforeunload after it, not two seconds: a slow unload after a
  cancelled leave reported the frame gone and the player got no
  sources.
- content.js: the next-season link is clicked out of the page again;
  a disconnected <a> navigates by the HTML spec, and in the page it
  went through the page's click handlers.
- background.mjs: Clear Sources keeps the entries other tabs held, not
  a time; a stream recovered later keeps the page's request time.
- background.mjs: a step from about:blank keeps the player chosen for
  a link opened in a new tab.
- FastStreamClient: switching the preview off lets go of its context.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread tools/same-code.mjs Fixed
A missing or mistyped base gets the usage and exit 2 instead of a
stack trace. CodeQL took the plain check of the argument for a
user-controlled bypass (js/user-controlled-bypass).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Nawid3333
Nawid3333 merged commit 7b27a08 into main Nov 15, 2026
23 checks passed
@Nawid3333
Nawid3333 deleted the polish/handoff branch November 15, 2026 02:12
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