Skip to content

Firefox optimisation, accessibility, the whole-code review's fixes, and the report - #675

Closed
Nawid3333 wants to merge 106 commits into
mainfrom
feature/firefox-optimization-ui-modernization
Closed

Nawid3333 wants to merge 106 commits into
mainfrom
feature/firefox-optimization-ui-modernization

Conversation

@Nawid3333

@Nawid3333 Nawid3333 commented Nov 13, 2026 •

Copy link
Copy Markdown
Owner

Draft, not for merging yet: this waits for the owner's test and decision. Merging it would release, because a push to main releases.

The full report is in docs/firefox-optimization-ui-review.md: research, backlog, numbers, the review's fixes, verification, the manual test steps, the merge order and the owner's decisions (section 8). The watcher is #674, the rename #676. All open PRs except the mpv update #634 are merged together on test/all-prs, the build to test.

What it changes

  • Saving through the page transport is tested end to end (ext-specs/page-fetch.e2e.mjs): a service-worker video and a current-cookie video are saved byte for byte; a cancelled save leaves no file.
  • Player accessibility (no motion added): Space presses a control the keyboard went to; six menu buttons say aria-haspopup/aria-expanded; the seek bar is a named slider ("Playback position", in all 16 locales) with arrow keys and a focus ring.
  • The MP4 loop rests while paused or ended: 41 → 9.5 wake-ups a second paused, 255 → 9.4 once ended. Two bugs found on the way: replay after the end never started; an audio-only MP4's seek preview busy-looped.
  • The A/B benchmark stays (tools/bench-ab.mjs): two builds by turns, slow-line option, rank-sum verdicts.
  • A review of the whole code: 52 fix commits in seven areas (background, content scripts, player core, saving and audio, settings, player UI, engines), each but a comment's correction with a test. Behaviour changes on purpose are listed in the report (section 8).
  • The settings page loads after the player's first frame: first frame 1005 → 882 ms in its own A/B.
  • Workflow hardening: a release tag must be on main; the fail-safe's token only in the steps that call gh; update-prs.yml's concurrency on its job.
  • Type cleanup: elements and window.fastStream typed once; 1,384 → 1,001 casts, the minified code of all 22 files the same.
  • Measured and taken back: the review made a "spare half-done downloads" check work that never had; it made an MP4 seek on a 12 Mbit/s line a second slower (1557 → 2547 ms), so it is gone and seeking is as on main.

Numbers (main → test branch, 7 rounds, Firefox 160)

  • MP4 callbacks a second: playing 40.9 → 25.0, paused 41.3 → 9.5, ended 255 → 9.4.
  • Player modules ready: faster for DASH, the idle player, and on a 12 Mbit/s line for MP4 and HLS. First frames, seeks, long and dropped frames: the same.
  • Worse: about 3 DOM changes a second more while an MP4 plays (the seek slider's ARIA values).
  • GitHub zip 2,577,753 → 2,597,572 B (mostly comments, and SaveFile.mjs). Permissions unchanged; instagram_inject.js is web-accessible to Instagram only.

Checked

  • lint, typecheck, unit tests (3734 passed on this branch, 3791 on the test branch), build:keep, lint:amo/lint:github (0/0), verify:vtt, verify:fsaunpack, workflow tests in WSL.
  • e2e, every suite except the mpv specs (never run on that PC): all green after four test fixes, described in the report. Two unit tests from main that set stats by hand to keep a nearly done download now expect it cancelled, with the reason in each (the half-done decision above).
  • CI on Windows and Linux, the mpv specs included, and Firefox Beta: green.

🤖 Generated with Claude Code

Nawid3333 and others added 7 commits November 13, 2026 15:19
page-fetch.e2e.mjs saves the videos only the page can load through the
Save button and the name prompt: a service worker's video and a
current-cookie video are the film byte for byte (SHA-256), their far end
asked for through the page. A save cancelled about a quarter in says
"Save cancelled!" (no "failed, save what there is?" question), leaves no
file, and the next save is whole.

Each save is of a video no earlier test played (a player takes over what
the previous player of the same video loaded), and the film is sent
no-store (Firefox's cache answered ranges an earlier test had loaded).

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

- Space presses a control the keyboard went to, as it does a <button>
  (WebUtils.setupTabIndex). After a mouse click the control has the
  focus without :focus-visible, and Space stays play/pause.
- The six menu buttons (audio, subtitles, quality, language, speed,
  loop) say aria-haspopup and whether their menu is open
  (aria-expanded, WebUtils.setExpanded in each openUI/closeUI).
- The seek bar is a named slider in the tab order ("Playback position",
  a new string in all 16 locales), its aria-valuemax/now/valuetext
  written once a second with the time's text; focused, the arrows seek
  as everywhere in the player. It gets the keyboard focus ring.

No transition or animation added. Tests: playerA11y.test.mjs (Space,
aria-expanded, the slider's wiring) and specs/player-a11y.e2e.mjs in the
real player (Tab reaches the slider, its values follow a seek, ArrowRight
seeks from it; Space on a keyboard-focused button presses it, after a
click it plays; the subtitles button's aria-expanded).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The options page sets up its fields with WebUtils.setupTabIndex too, and
a text field always has :focus-visible: the Space rule of the commit
before swallowed every space typed in one (the mpv path, "1 2" in the
speed limit became "12", measured on that build in the e2e). Inputs,
text areas, selects and buttons keep Space to themselves.

Tests: playerA11y.test.mjs (each kind of form control), and
player-a11y.e2e.mjs on the options page (a box ticked and unticked once
each, a space typed).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs/firefox-optimization-ui-review.md: what the two branches do, the
research (the watcher's high findings for 153-160 checked against the
code, documentId, the replaced player's measurement, the page transport,
media APIs, the UI), the backlog with its five labels, the numbers, what
was verified and what was not, and the steps of the owner's hand test.

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

The player's speed was compared by turns several times (docs/notes/player.md), each time with
a harness thrown away afterwards. `node tools/bench-ab.mjs --a main` (pnpm run bench:ab) builds a
git ref in a worktree of its own (node_modules linked, same lockfile required) and this checkout,
and runs tests/e2e/wdio.bench.conf.mjs: one server per build, the same fixtures, both builds in
one e2e Firefox in ABBA order, a warm-up round thrown away, the media optionally on one shared
slow line (--rate, --ttfb).

A probe added before the page's scripts (browser.addInitScript) records player modules ready,
first frame, playing, a seek to 80 s, a seek preview, and counts timer/frame callbacks, DOM
changes, frames more than 50 ms apart and dropped frames while playing, paused, after a seek
preview, once ended and in an idle player; --where names the places in the player's code that
asked for the callbacks. logs/bench/ab-<time>.md/.json: medians, MAD, a verdict only beyond
twice the noise and the measure's floor (tests/unit/benchStats.test.mjs). It never fails on a
number. --clean removes its worktrees, the node_modules link first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oop while paused or ended

Both found with tools/bench-ab.mjs, on main as well:

- play() on an ended MP4 seeks the element to 0 by itself, past MP4Player's currentTime setter,
  the only caller of resetHLS. mp4box went on from the end, the start had left the buffer, and
  the video stood seeking at 0 for good: the play button after the end of any MP4 longer than
  about 40 s. Firefox's picture-in-picture seek controls seek the element the same way.
  onElementSeeking (the element's seeking event) checks such a seek as the setter does
  (reloadUnlessBuffered); one made through the setter is recognised (ownSeek) and not checked
  twice.
- The main loop ran every 50 ms while paused, in each MP4 player (the seek preview is a second
  one, always paused), and every 1 ms once the video had ended (readyState below 3 counted as
  urgent): 41 and 250 timer callbacks a second for as long as the tab stayed open. Paused or
  ended it runs every 250 ms now, and a seek or a play wakes it at once (wakeLoop). Measured:
  9.3 a second paused and ended; a seek while paused lands in 62-109 ms.

tests/e2e/specs/mp4-loop-pace.e2e.mjs fails on main (41 a second paused; replay stuck at 0,
readyState 1) and passes here. The whole web e2e suite: 48 of 48 spec files.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread tests/e2e/bench/ab.bench.mjs Fixed
Comment thread tests/e2e/wdio.bench.conf.mjs Fixed
Comment thread tools/bench-ab.mjs Fixed
Nawid3333 and others added 3 commits November 13, 2026 17:47
…found

Three bugs on main, each checked against the code and shown by a test that failed before:

- The next episode's continuation (autoplay, the carried fullscreen, no remembered place)
  stayed on the tab until some player took it: a next page with no player, a page the user
  went to himself, and the video opened there minutes later went fullscreen, played by itself
  and skipped its place. It holds only for the first page after the click now
  (continuationArrived; MPV's mpvNext is unchanged), and a next episode on another hostname
  keeps it (the new site's reset dropped it).
- Escape in the page could not end a carried fullscreen whose player's frame went or
  reloaded: the background passed it to that frame only. The player answers whether it ended
  it (exitCarriedFullscreen returns true); otherwise the background does.
- The page around an embed kept its own menu hidden after the player left the embed while the
  iframe stayed: only the player's leaving page released the overlay guard. A player frame's
  FRAME_REMOVED now tells each frame above to let go of the iframe on the way
  (RELEASE_OVERLAY_GUARD), unless another player lives under it.

Tests: backgroundFullscreen (continuation to the next page only, on another site too; the
exit when the player is gone or new), backgroundOverlayGuard, contentScript (the release),
content-overlays e2e (the embed's page goes elsewhere: fails on main, passes here).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…for an audio-only preview

What a review found in the MP4 fixes before:

- ownSeek was set by the client's start, made at readyState 0, where no seek runs and no
  seeked ever clears it: a video played to its end without a seek still stood seeking at 0
  when played again. It is set only when the element started a seek now.
- An audio-only MP4's seek preview made no SourceBuffer (a preview plays no audio), its element
  stayed at HAVE_NOTHING and its loop waited for a first frame every 4 ms for the tab's life
  (236 callbacks a second). Such a preview stops once the metadata is in (nothingToPreview):
  6.7 a second.

mp4-loop-pace.e2e.mjs: both new tests fail on the build before and pass here. docs/notes:
these, the background fixes, and what the review found and was not changed yet.

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

On the slow line, each of the player's modules waited 150 ms too, and every start took 4.5 s
in both builds: an extension's files come from its package at once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread tests/e2e/wdio.bench.conf.mjs Fixed
Nawid3333 and others added 18 commits November 13, 2026 17:54
Three alerts on #675, all in the A/B harness (development only, nothing ships):

- Remote property injection: a scenario name from BENCH_SCENARIOS named a key of the results.
  The names now come from SCENARIOS itself; an unknown one is a failing test of its own.
- Resource exhaustion: BENCH_TTFB_MS set a timer's delay unchecked. Every number the harness
  reads from the environment (rounds, warm-up, rate, first byte, phase lengths) is refused
  outside its range now (boundedNumber, unit-tested).
- File system race: the build stamp was checked with existsSync and written later. It is read
  and must name the commit now, which also rebuilds a tree whose stamp is another commit's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… calls for a closed tab

Two more of the review's findings, each with a test that fails before:

- A player inside a frame that went (an embed's page left, with the player in it) is
  forgotten with that frame. Its own FRAME_REMOVED can come after: its entry in
  playerPageFrames was still there, so the background took it for a player lost while Firefox
  had unloaded it (restorePlayerFrame), made it and its page frame again and counted it gone a
  second time. With two players in a tab, the next one to open was then told it was the tab's
  main player. Every frame forgotten with the one removed now leaves playerPageFrames and
  playingPlayers. backgroundFrameRemoved.test.mjs.
- The toolbar button's calls (badge, title, icon) for a tab closed in between were rejected
  unhandled: the "Off" badge's second, a start that waited for its page. They go through
  setAction now, which takes the rejection. backgroundRobustness.test.mjs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The MPV allowlist's or auto-enable list's start waits half a second for the new page
(startWithTrackedLater) and then checked the tab's address was still the same string. A page
that sets #t=... or an anchor right away is still the same page (isSamePageUrlChange), which no
new start takes up, as the tab is on already: the start was lost, and the stream found before
the address change went nowhere. It now goes on for the same page; a #/ or #! route is a new
page and still cancels it. backgroundMpv.test.mjs fails before.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n from their numbers

The 12 Mbit/s run called HLS's first frame 475 ms better on the branch: the medians were that
far apart, over twice the MAD. Its rounds were in two modes on both builds - about 1.47 s or
2.0-2.1 s - three slow of five on main, two on the branch; the order says nothing. A verdict now
also needs the Wilcoxon-Mann-Whitney rank-sum test (exact over every split of the runs, ties at
their mean rank; seeded sampling past 200,000 splits) below 0.05: five runs a side reach 0.008
at best, three only 0.1. The table shows p.

The measures and the report moved to report.mjs, so `node tools/bench-ab.mjs --render <name>`
prints an earlier run's report from its .json in logs/bench with the verdicts as they are now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Twelve places forward and back (one before the last in the same part of the file), a sweep of
quick moves and a move within a part, paused and playing: each must show (not seeking,
readyState 2 or more, at the place) within 10 s. The A/B benchmark had timed out once in 21
waiting for the preview at 120 s, on main's player and on this branch's alike; its check looked
only at 'seeked' and 'loadeddata'. Shown in 29-137 ms on this branch, 57-169 ms on main.

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

About one run in 20 timed out waiting for the preview at 120 s, on main's player and the
branch's alike, while the preview did show (mp4-seek-preview.e2e.mjs: 340 places of 340 on both
builds, 10 runs each): the frame can come after 'seeked' with no other event of the two checked.
canplay and timeupdate count too, and the wait checks the state itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mp4-loop-pace's audio-only test played mp4-shapes/long-audio-only.mp4, which only
mp4-loading.e2e.mjs wrote: on a fresh checkout (the test branch's worktree) it failed with "the
audio did not load", and a CI shard running it before mp4-loading would too. The ffmpeg helper
that two specs each had a copy of is one module now (ensureMp4Shape, ensureLongAudioOnly), and
mp4-loop-pace asks for its file itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…after a DRM refusal

What the user saw: in MPV mode on a copy-protected video, the toolbar's On -> MPV
(or a start by address) was refused with "!", and then the page's next stream (an
ad or a piece of the film) went to mpv, the "!" was cleared and the page paused.

Cause: openMpvWithSources had its own copy of the allowlist's hand-off and
re-armed the once-per-page latch after every failure, a DRM refusal too; since
#606 autoOpenInMpv keeps it after one.

Fix: openMpvWithSources sends through autoOpenInMpv, so there is one latch
handling. sendPlayedToMpv (the MPV key's play) keeps re-arming after a DRM
refusal on purpose, now said in a comment and pinned by a test: kept, a repeat
play only paused the page.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What the user saw: nothing; a private window's playlists and the first 64 KB of
its files, read for their length by the background, stayed in the normal disk
cache after the private window closed.

Cause: the background is no private window, and its length reads
(StreamLengths.get) passed credentials 'omit' but no cache option.

Fix: cache 'no-store', as MpvSubtitles' reads already do for the same reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the silence skipper on, D, S and the speed preset keys changed the speed for a
moment only: the OSD showed the new speed, and within 100 ms the skipper's loop put its
regular speed back, which was also what got saved. Only a pick in the speed menu became
the skipper's regular (or skip) speed.

A speed key now goes through the same step as a menu pick (chooseSpeed). Speeds the player
holds for a moment (holding the video, a GIF's 4x) and the skipper's own switches do not,
so they never become the regular speed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every seek (arrow keys, frame step, a click on the timeline) and every yield to a
watched player cancelled the running downloads ahead, nearly finished ones too, which
then came again from their first byte. The half-done check read the downloader's
stats, which were null until FetchLoader's progress, and FetchLoader gives that once,
after the whole body. The downloader now takes FetchLoader's own stats as its request
goes out (loaded and total as the body comes), and both callers share one check.

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

What the user saw: nothing; a refused stream in a private window or a container
tab had its page fetched again by the background with the normal window's
cookies. What the page set landed in the normal cookie jar, a container's own
login was not the one sent, and the request missed a private window's proxy.

Cause: REQUEST_FRESH_LINK fetched the page (credentials 'include') with no
check of the sender tab's window or cookie store; the background is neither
private nor in a container.

Fix: answer no link for a private window's tab or a container tab. The player
there handles the refusal as before FreshLink.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A key bound to "+" (the German + key, the numpad +, Shift+= on a US
keyboard) never fired, and two actions on it got no warning.
getKeyString names that key by its character, so it is saved as '+' or
'Control++'; actionsForKey split the string at every '+', found an
empty key and gave up before the exact match.

The modifiers and the key are now read the way keyDisplayName reads
them: the leading Meta/Control/Alt/Shift, each with its '+', and the
rest is the key. 'Shift+' still names no key.

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

A broken server or proxy that answers each range with 206, no Content-Range and the
rest of the file had the whole rest read for every 1 MB range: held in RAM as one
buffer, stored in the RAM budget, and only then cut to the range by MP4Player. The
cut-to-range reading covered only a 200 (a server that ignores Range). A 206 without
Content-Range is now read from the range's start up to the range's length, and the
rest is cancelled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nothing called EventEmitter.logAllEvents (the only setter of its debug flag, read on
every emit) or EmitterRelay.prependOutput, and nothing sends the channel counter
worklet a 'configure' message: its handler called a configure() the class does not
have. Removed, with the debug flag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s as the tab's player

What the user saw: after Firefox unloaded the idle event page while a player
played, a second player in the same tab was told it is the main player and
played without a click, as the first did.

Cause: restorePlayerFrame made the frame a player's again (isPlayer) but did not
count it in tab.playerCount, which acceptPlayer's isMainPlayer goes by; its
leaving still counted it gone.

Fix: count the frame when it becomes a player's, in restorePlayerFrame and in
acceptPlayer alike, so a frame restored and then loading its player anew is
counted once, not twice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Nawid3333 and others added 11 commits November 13, 2026 19:19
… the player

The player's settings page is an iframe in every player, and it loaded with the player: its
scripts ran beside the player's own as it started. Measured with tools/bench-ab.mjs
(2026-11-13, 7 rounds, MP4): the first frame came 137 ms later with it (945 ms against 808 ms
without, p 0.004, 7 of 7 rounds); seeks, wake-ups and frame drops the same. It loads now when
the page is idle after the first frame, at once when the settings window opens, and after
5 s in a player that shows nothing, so the GitHub build's update check still runs and shows
its mark on the settings button.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…yer external links)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What the user saw: with a video delay set on an MP4, the two extra audio players (and
the audio analyzer's player) each buffered and appended the video as well, frame by
frame: the MediaSource memory (about 30 s ahead and 10 s behind of video in each) and
the per-frame work three times over.

Cause: MP4Player never looked at isAudioOnly beyond making an <audio>: it made a video
SourceBuffer, had mp4box cut every video frame into a segment, appended it, and chose
where to read from by the video's keyframes.

Fix: playedVideoTrack() gives no video track to a player for sound only of a file with
sound; makeSourceBuffers, startSegmentation and the keyframe index (currentFragment) use
it, as the seek preview leaves out the sound. The ranges' times still come from the
video track, as the main player's do (the fragment store is shared). A file without
sound keeps its picture in such a player, as before; the main player, the preview and an
audio-only file are unchanged.

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

OPEN_EXTERNAL_URL answered only the player page, but the settings page's report and update
buttons now open their pages through EnvUtils.openExternalURL as well, and in a player framed
in a page that settings page, an iframe in the player, has no chrome.tabs either: the buttons
did nothing there. The two pages are now matched by their exact address, not by its start.

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

What the user saw: nothing directly; each HLS segment hls.js aborted while the client
also downloaded it stayed in HLSPlayer.activeRequests for the session (its loader and
callbacks kept in memory), and the seek preview, the only reader of the list, took it
for a segment still loading.

Cause: a loader leaves the list in its download's onSuccess, onFail or onAbort. A
download with other watchers tells only its last watcher that it was aborted
(DownloadEntry.abortWatcher), so an aborted loader that was not the last heard nothing.

Fix: HLSLoader.abort() takes its loader out of the player's list itself. DownloadEntry is
unchanged: telling every watcher would reach the other players' and the saves' handlers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r fixes (MO-1..9)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The e2e test of the seek past one track's end (PE-2, MP4Player.seekMp4box) played a 20 s
cut of the long fixture: the player loads 30 s ahead, so both tracks were appended to
their ends before the seek, and the test passed on the old code too. It now plays the
first 100 s with the sound cut to 98 s, and checks that neither track is appended to its
end before the seek (the old code's stall reproduced with mp4box on a file of this shape,
written by the same ffmpeg arguments).

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

Part of the fix before (PE-1, previewSeek): a preview reading on to a place past the
duration it knew held every range it read until it got there. A player's hold is one
the memory budget cannot let go of: a far hover into a long fragmented file without a
mehd box could hold most of it in RAM, and stop the main player's downloads ahead.

The ranges behind the place held are let go of as the preview reads on, as those behind
the playhead are while a video plays; the request loop starts at the range of the last
keyframe known before the place, so none is asked for again. The SourceBuffer's own
trimming is unchanged.

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

# Conflicts:
#	tests/unit/FastStreamClient.test.mjs
No caller passes it (FastStreamClient, SyncedAudioPlayer), and DashPlayer, the one player
that stored it, no longer does.

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

DashPlayer, the one player that stored it, did not read it either (fix/vendor removed it).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread tests/unit/UrlMatchList.test.mjs Dismissed
Comment thread tests/unit/UrlMatchList.test.mjs Dismissed
Comment thread tests/unit/loopMenuFrames.test.mjs Fixed
Nawid3333 and others added 14 commits November 13, 2026 19:36
What the fix branches changed, recorded where a maintainer looks for it:
keybinds.md (speed keys with the silence skipper, Escape, Space and Enter,
a malformed settings file, the "+" key, save echoes), storage.md (the
hand-off's files, a save's disk copy after its download, SaveFile in place
of mp4merger's slices, FreshLink and the length reads in private and
container tabs), player.md (206 without Content-Range, the 256 GiB cap,
MP4Player's seek, preview and sound-only fixes, half-done downloads, the
A-B and volume loops, partial-save lengths, OPEN_EXTERNAL_URL, the settings
page after the first frame, the review's second round), mpv.md (one latch
for every automatic hand-off), build-and-release.md (the update check,
instagram_inject.js) and libraries.md (dash.js's segment cap). Statements
the fixes made wrong are corrected in place, with the date.

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

CodeQL flagged three of the review's new tests. loopMenuFrames ran LoopMenu's source through
new Function to step around the generated gif.mjs: it imports the class now, gif.mjs and the
player's elements stand-ins (vi.mock), as `pnpm test` syncs the generated modules first.
visibilityAction ran handleVisibilityChange's source: it checks the switch's cases against
the actions VisChangeActions declares instead, which fails for the PIP case the fix removed.
playerFieldKeys matched comment lines with a nested repeat that can backtrack: it compares
the listener's first statements with its comments left out.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ree has (1.7.4, 5.3.0)

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

Space presses a control only when the keyboard went to it (WebUtils.setupKeyPress, since the
review's fixes): the test's plain focus() after a mouse click is no keyboard focus in Firefox,
and Space played the video instead (CI, Windows playback 2/3). focus({focusVisible: true}) is
the focus Tab gives.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each entry of DOMElements.mjs is cast once where it is found: HTMLElement
for the divs, HTMLIFrameElement for the options frame, NodeListOf<HTMLElement>
for the status messages. Every element is in index.html from the start.
The 437 casts at the use sites that only repeated this (to HTMLElement,
Element, ?HTMLElement, and HTMLButtonElement on moreButton, a div) are
gone, and one on a status message read from the typed list. The code's
own null checks stay. Comments only: the minified code is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
types/player-window.d.ts adds the player's client to Window, optional
(main.mjs deletes it at beforeunload, and other pages have none), so
window.fastStream type-checks directly. The 26 casts of window that the
code's own checks make needless are gone; the 13 in recieveSources() and
loadOptions(), which read the client without a check, keep
FastStreamWindow, now defined from the declaration. Comments and a
declaration only: the minified code is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sts fewer, same code)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Firefox leaves fullscreen as soon as another tab comes to the front, and
sendToPageInFront switches to the extension's tab to read each answer: the
first toggle's fullscreen was over before the test looked, and the second
toggle could not be tested. Both toggles now go out on timers from the
extension's tab while the site's tab stays in front, and the page records
each fullscreen change. Checked against the old compare (the document's
fullscreenElement): answers enter, enter; seen host - the test fails there.

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

The review's PC-1 fix made the "spare a download at least half in" check work (its
stats were null until the whole body was in, so it never held). The A/B of the
finished test branch then showed a seek to 80 s in an MP4 on a 12 Mbit/s line a
second slower, 1557 -> 2547 ms in 7 of 7 rounds (local files: no change): the
spared downloads, outside the new place's window, shared the line with the ones
the seek waited for. The same build with only the check off seeks in 1543 ms
(2370 -> 1543, 5 of 5 rounds, p 0.008).

The check is gone from cancelIfCheap and setYield, and StandardDownloader is as on
main. The tests say the measured choice: a download 900 of 1000 bytes in is
cancelled by a seek and by a yield (both fail with the check back); the two older
tests that set stats by hand to keep one nearly done now expect it cancelled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Set on the element alone, the rate went back to the client's 1x at the next
canplay (FastStreamClient's CANPLAY handler), which comes again after every
stall: on a busy runner the 160 s clip could not end within the 60 s wait ("the
video did not reach its end at 8x", CI Windows and once locally, each passing on
its retry). The state with its rate now goes into the message if it ever times out.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…page-fetch says where a save stood

page-qualities ("switches to another where the video is", from 2026-10-24) stopped looking
as soon as the time moved on, before its watcher had noted any 'playing' or 'timeupdate'
of the new video (firstPlayed -1, CI Linux, passed on its retry). It now waits for a
noted time as well; the assertion is unchanged.

page-fetch's worker save once never began on the Linux runner (no banner, no download,
the player never logged its save; passed on its retry). The cause is not in the log, so
a timeout now reports the open dialog and the save manager's askingToSave and
makingDownload.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d the owner's decisions

Sections 4-8 rewritten: the final A/B of main against the test branch (local and a
12 Mbit/s line, 7 rounds), the whole-code review's fixes by area with the behaviour
changes marked, the half-done check measured and taken back, the workflow hardening,
the type cleanup, sizes, local and CI verification with the flakes looked at, the
manual test steps for the test branch, the merge order with the rename, and the
behaviour questions for the owner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Nawid3333 Nawid3333 changed the title Page-transport save tests, player accessibility, and the review report Firefox optimisation, accessibility, the whole-code review's fixes, and the report Nov 14, 2026
@Nawid3333

Copy link
Copy Markdown
Owner Author

Folded into the test branch test/all-prs (347a3b6), which carries every open pull request except #634 (mpv) and goes to main as one merge after the owner's test. Closed so that only main, mpv-update and the test branch remain.

@Nawid3333 Nawid3333 closed this Nov 14, 2026
@Nawid3333
Nawid3333 deleted the feature/firefox-optimization-ui-modernization branch November 14, 2026 15:08
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