From dd37f7239f23783b3a684de2f86ee4244d0202d0 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:37:50 +0100 Subject: [PATCH 01/17] ci: patched-libraries in two jobs, the write token apart from the install (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 --- .github/workflows/patched-libraries.yml | 201 ++++++++++++++---- docs/notes/workflows.md | 17 ++ .../patched-libraries-publish.test.sh | 143 +++++++++++++ 3 files changed, 320 insertions(+), 41 deletions(-) create mode 100644 tests/workflows/patched-libraries-publish.test.sh diff --git a/.github/workflows/patched-libraries.yml b/.github/workflows/patched-libraries.yml index b68ecbd2..b2703808 100644 --- a/.github/workflows/patched-libraries.yml +++ b/.github/workflows/patched-libraries.yml @@ -23,6 +23,14 @@ name: Patched libraries # pull request closes the library's older issue and pull request, its issue only the older # issue (an older pull request is still a step forward while the newer version needs work). +# +# Two jobs, so that no code from a new library release runs where the write token is: "recut" +# installs the release (its install scripts are its authors' code) and re-cuts the patch, with +# a read-only token, and hands each result on as an artifact - the change as a git patch, or the +# report of why it stopped. "publish" runs on a runner of its own with the write token, installs +# nothing, and pushes, opens and closes with git, gh and jq alone (SEC-3 of the review of +# 2026-11-13: one job did both, and a release's install script could read the token from its +# step's environment, or plant something in the job's later steps through $GITHUB_ENV). on: schedule: # Daily, 06:30 UTC. @@ -36,22 +44,26 @@ on: - package.json workflow_dispatch: -permissions: - contents: write # push the patched/- branch - pull-requests: write # open the update pull request - issues: write # open the issue when the re-cut stops - actions: write # start CI and the dependency review on the branch + +permissions: {} concurrency: group: patched-libraries cancel-in-progress: false jobs: - check: - name: Check patched libraries for updates + recut: + name: Re-cut the patches onto new releases + # A push only closes (publish); re-cutting is the daily run's job. + if: github.event_name != 'push' runs-on: ubuntu-latest # A dash.js re-cut lints a 3 MB bundle four times; the rest takes seconds. timeout-minutes: 30 + permissions: + contents: read # the repository + issues: read # the titles raised before (the issues endpoint lists pull requests too) + outputs: + count: ${{ steps.recut.outputs.count }} steps: # The token is not kept in .git/config (persist-credentials: false): every later # step could read it there, dev dependencies' install scripts included. @@ -71,6 +83,120 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + - name: Re-cut each patch whose library has a new release + id: recut + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PREFIX: 'Patched library update: ' + run: | + set -euo pipefail + node tools/check-patched-updates.mjs --json > "$RUNNER_TEMP/libraries.jsonl" + node tools/check-patched-updates.mjs + + # --- Every title raised before, as an issue or a pull request, open or closed: the issues + # endpoint lists pull requests too, and pages through all of them. Not the search index, + # which can lag behind one made minutes ago. + # Only this workflow's own: anyone's pull request with such a title blocks nothing. + gh api --paginate "repos/$GITHUB_REPOSITORY/issues?state=all&per_page=100" \ + --jq '.[] | select(.user.login == "github-actions[bot]") | .title' > "$RUNNER_TEMP/titles.txt" + + git config user.name 'github-actions[bot]' + git config user.email '41898282+github-actions[bot]@users.noreply.github.com' + start=$(git rev-parse HEAD) + # One folder per library to publish, numbered in order: library.json, report.txt, and + # outcome - "cut" with change.patch, or "stopped". + recuts="$RUNNER_TEMP/recuts" + mkdir -p "$recuts" + count=0 + + # Not a pipe into the loop, and not on stdin either: a failed command in a piped loop + # only ends the subshell, and pnpm or gh inside it could read the list off stdin. + jq -c 'select(.behind)' "$RUNNER_TEMP/libraries.jsonl" > "$RUNNER_TEMP/behind.jsonl" + while read -r library <&3; do + name=$(jq -r .name <<< "$library") + current=$(jq -r .current <<< "$library") + latest=$(jq -r .latest <<< "$library") + title="$PREFIX$name $latest" + if grep -qxF "$title" "$RUNNER_TEMP/titles.txt"; then + echo "Already raised: $title." + continue + fi + + git checkout -q -f -B recut "$start" + status=0 + node tools/recut-patch.mjs "$name" "$latest" --apply --out "$RUNNER_TEMP/recut-$name" \ + > "$RUNNER_TEMP/report.txt" 2>&1 || status=$? + cat "$RUNNER_TEMP/report.txt" + + if [ "$status" = 3 ]; then + echo "$name $latest is too new to install yet; the next run tries again." + else + count=$((count + 1)) + dir="$recuts/$count" + mkdir "$dir" + printf '%s\n' "$library" > "$dir/library.json" + cp "$RUNNER_TEMP/report.txt" "$dir/report.txt" + if [ "$status" = 0 ]; then + # .github/dependabot.yml: a patch dropped hands the library (and what moves + # with it) back to Dependabot (tools/recut-patch.mjs). + git add -A -- package.json pnpm-lock.yaml pnpm-workspace.yaml patches tools/sync-vendor.mjs .github/dependabot.yml + # Anything else changed would be a surprise worth stopping for, not committing. + stray=$(git status --porcelain | grep -v '^[MADR] ' || true) + if [ -n "$stray" ]; then + echo "Unexpected changes after the re-cut:"; echo "$stray"; exit 1 + fi + git commit -q -m "deps: $name $current -> $latest, patch re-cut by tools/recut-patch.mjs" + git format-patch -1 --stdout > "$dir/change.patch" + echo cut > "$dir/outcome" + else + echo stopped > "$dir/outcome" + fi + fi + git checkout -q -f "$start" + git clean -q -fd + done 3< "$RUNNER_TEMP/behind.jsonl" + echo "count=$count" >> "$GITHUB_OUTPUT" + + - name: Hand the re-cuts on + if: steps.recut.outputs.count != '0' + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: recuts + path: ${{ runner.temp }}/recuts + retention-days: 1 + if-no-files-found: error + + publish: + name: Check patched libraries for updates + needs: recut + # On a push recut is skipped, and this only closes; a failed re-cut publishes nothing. + if: ${{ !cancelled() && needs.recut.result != 'failure' }} + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: write # push the patched/- branch + pull-requests: write # open the update pull request + issues: write # open the issue when the re-cut stops + actions: write # start CI and the dependency review on the branch + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + # For tools/check-patched-updates.mjs, which needs nothing but Node itself: nothing is + # installed in this job. + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version-file: .nvmrc + check-latest: true + + - name: Take the re-cuts + if: needs.recut.outputs.count != '' && needs.recut.outputs.count != '0' + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 + with: + name: recuts + path: ${{ runner.temp }}/recuts + - name: Sync update pull requests and issues with the patched versions env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -146,50 +272,35 @@ jobs: exit 0 fi - # --- Every title raised before, as an issue or a pull request, open or closed: the issues - # endpoint lists pull requests too, and pages through all of them. Not the search index, - # which can lag behind one made minutes ago. - # Only this workflow's own: anyone's pull request with such a title blocks nothing. - gh api --paginate "repos/$GITHUB_REPOSITORY/issues?state=all&per_page=100" \ - --jq '.[] | select(.user.login == "github-actions[bot]") | .title' > "$RUNNER_TEMP/titles.txt" - git config user.name 'github-actions[bot]' git config user.email '41898282+github-actions[bot]@users.noreply.github.com' start=$(git rev-parse HEAD) - # Not a pipe into the loop, and not on stdin either: a failed command in a piped loop - # only ends the subshell, and pnpm or gh inside it could read the list off stdin. - jq -c 'select(.behind)' "$RUNNER_TEMP/libraries.jsonl" > "$RUNNER_TEMP/behind.jsonl" - while read -r library <&3; do + # --- Publish what the re-cut job made, in its order. Its contents come from a job + # that ran a new release's code, so they are checked here: a name and a version as + # npm has them, and a change to the files a re-cut may change, nothing else. + find "$RUNNER_TEMP/recuts" -mindepth 1 -maxdepth 1 -type d 2>/dev/null | sort -V > "$RUNNER_TEMP/recut-dirs.txt" || true + while read -r dir <&3; do + library=$(cat "$dir/library.json") name=$(jq -r .name <<< "$library") current=$(jq -r .current <<< "$library") latest=$(jq -r .latest <<< "$library") url=$(jq -r .url <<< "$library") - title="$PREFIX$name $latest" - if grep -qxF "$title" "$RUNNER_TEMP/titles.txt"; then - echo "Already raised: $title." - continue + if ! [[ $name =~ ^(@[a-z0-9._-]+/)?[a-zA-Z0-9._-]+$ && $latest =~ ^[0-9A-Za-z.+-]+$ && $current =~ ^[0-9A-Za-z.+-]+$ ]]; then + echo "Not a library and version: '$name' '$current' '$latest'."; exit 1 fi + title="$PREFIX$name $latest" + cp "$dir/report.txt" "$RUNNER_TEMP/report.txt" - branch="patched/$name-$latest" - git checkout -q -f -B "$branch" "$start" - status=0 - node tools/recut-patch.mjs "$name" "$latest" --apply --out "$RUNNER_TEMP/recut-$name" \ - > "$RUNNER_TEMP/report.txt" 2>&1 || status=$? - cat "$RUNNER_TEMP/report.txt" - - if [ "$status" = 3 ]; then - echo "$name $latest is too new to install yet; the next run tries again." - elif [ "$status" = 0 ]; then - # .github/dependabot.yml: a patch dropped hands the library (and what moves - # with it) back to Dependabot (tools/recut-patch.mjs). - git add -A -- package.json pnpm-lock.yaml pnpm-workspace.yaml patches tools/sync-vendor.mjs .github/dependabot.yml - # Anything else changed would be a surprise worth stopping for, not committing. - stray=$(git status --porcelain | grep -v '^[MADR] ' || true) + if [ "$(cat "$dir/outcome")" = cut ]; then + branch="patched/$name-$latest" + git checkout -q -f -B "$branch" "$start" + git am -q "$dir/change.patch" + stray=$(git diff --name-only "$start" HEAD | + grep -vxE 'package\.json|pnpm-lock\.yaml|pnpm-workspace\.yaml|patches/[^/]+\.patch|tools/sync-vendor\.mjs|\.github/dependabot\.yml' || true) if [ -n "$stray" ]; then - echo "Unexpected changes after the re-cut:"; echo "$stray"; exit 1 + echo "The re-cut of $name $latest changed more than a re-cut may:"; echo "$stray"; exit 1 fi - git commit -q -m "deps: $name $current -> $latest, patch re-cut by tools/recut-patch.mjs" authgit push -q -f origin "$branch" # The backticks are Markdown for the body, not command substitution. # shellcheck disable=SC2016 @@ -223,10 +334,18 @@ jobs: gh issue create --title "$title" --body-file "$RUNNER_TEMP/body.md" fi git checkout -q -f "$start" - git clean -q -fd - done 3< "$RUNNER_TEMP/behind.jsonl" + done 3< "$RUNNER_TEMP/recut-dirs.txt" supersede + report: + name: Report a failure + needs: [recut, publish] + if: failure() + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + issues: write # the failure issue + steps: # A failed run of this workflow is emailed only to whoever started it (for the schedule, # whoever last changed the cron line): one issue while it is open, as # toolchain-updates.yml does. A failure while it is open adds a diff --git a/docs/notes/workflows.md b/docs/notes/workflows.md index e2649759..efe54388 100644 --- a/docs/notes/workflows.md +++ b/docs/notes/workflows.md @@ -766,6 +766,23 @@ path: PR, issue, too new, already raised, caught up, no longer patched, push). `tests/unit/checkPatchedUpdates.test.mjs` fails if Dependabot's ignore list and `patchedDependencies` drift apart; `tests/unit/recutPatch.test.mjs` covers the tool. +- **Patched libraries: two jobs** (2026-11-15, SEC-3 of the review of 2026-11-13). The re-cut + installs the new release, and its install scripts are its authors' code: in one job with the + write token they could read the token from the step's environment, or plant something in the + later steps through `$GITHUB_ENV`. Now `recut` (contents and issues read) installs, re-cuts and + hands each result on as the artifact `recuts` (one folder per library: `library.json`, + `report.txt`, `outcome` - `cut` with `change.patch` from `git format-patch`, or `stopped`); + `publish` (the write permissions) installs nothing, closes and supersedes as before, and + publishes what it was handed in order. As that came from a job that ran foreign code, it is + checked first: the name and versions as npm has them (no shell or path characters), and the + change, applied with `git am` onto main, may touch `package.json`, `pnpm-lock.yaml`, + `pnpm-workspace.yaml`, `patches/*.patch`, `tools/sync-vendor.mjs` and `.github/dependabot.yml` + only; anything else stops the run before a push. A push to main skips `recut` and only closes. + `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`: a clean re-cut + becomes a branch, a pull request and two dispatches, a stopped one an issue; a change to a + workflow and a name with a `;` fail with nothing pushed or opened (each check, broken by hand, + turns it red). - All of it was dry-run with a stub `gh` against the real upstream history (sync: adopted -> closed, push -> no rebuild, new release named, comment only on upstream movement; patched libraries: open once, no duplicates, close when caught up or unpatched). diff --git a/tests/workflows/patched-libraries-publish.test.sh b/tests/workflows/patched-libraries-publish.test.sh new file mode 100644 index 00000000..2469c95b --- /dev/null +++ b/tests/workflows/patched-libraries-publish.test.sh @@ -0,0 +1,143 @@ +#!/usr/bin/env bash +# patched-libraries.yml's publish job: what the re-cut job handed on becomes a branch, a pull +# request and CI runs (a clean re-cut), or an issue (a stopped one). The re-cut job ran a new +# release's code, so what it hands on is checked first: a patch that changes anything a re-cut +# may not change, or a library name that is no npm name, stops the run with nothing pushed or +# opened. The step's script, a real git repository with a bare one as origin, and stubs for +# gh and for node (the update checker). +source "$(dirname "$0")/lib.sh" + +here=$(mktemp -d) +trap 'rm -rf "$here"' EXIT +step_script patched-libraries.yml 'Sync update pull requests and issues with the patched versions' > "$here/publish.sh" || exit 1 + +mkdir -p "$here/bin" +cat > "$here/bin/gh" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FIX/calls" +case "$1 $2" in + 'issue list'|'pr list') ;; + 'pr create'|'issue create'|'workflow run'|'issue close'|'pr close') ;; + 'api repos/o/r/issues?state=all&per_page=100') ;; + *) echo "unexpected gh $*" >&2; exit 99 ;; +esac +EOF +# The update checker: nothing is behind any more (the re-cut job decided what to do). +cat > "$here/bin/node" <<'EOF' +#!/usr/bin/env bash +if [ "${2-}" = --json ]; then + printf '{"name":"hls.js","current":"1.7.4","latest":"1.7.5","behind":true}\n' +else + echo 'hls.js 1.7.4 (latest 1.7.5)' +fi +EOF +chmod +x "$here/bin/gh" "$here/bin/node" + +# repo: main with the files a re-cut changes, and a bare origin. +make_repo() { + rm -rf "$here/origin.git" "$here/repo" + git init -q --bare "$here/origin.git" + git init -q -b main "$here/repo" + ( + cd "$here/repo" || exit 1 + git config user.name t + git config user.email t@example.invalid + mkdir -p patches .github/workflows tools + echo '{}' > package.json + echo 'lock 1' > pnpm-lock.yaml + echo 'patched: 1' > pnpm-workspace.yaml + echo 'old' > patches/hls.js@1.7.4.patch + echo 'on: push' > .github/workflows/ci.yml + git add -A + git commit -q -m main + git remote add origin "$here/origin.git" + git push -q origin main + ) +} + +# recut [files to change...]: what the re-cut job hands on. +recut() { + local n=$1 library=$2 outcome=$3 + shift 3 + local dir="$here/fix/tmp/recuts/$n" + mkdir -p "$dir" + printf '%s\n' "$library" > "$dir/library.json" + echo "report of $n" > "$dir/report.txt" + echo "$outcome" > "$dir/outcome" + if [ "$outcome" = cut ]; then + ( + cd "$here/repo" || exit 1 + git checkout -q -b tmp + for file in "$@"; do echo "changed $n" > "$file"; git add -A -- "$file"; done + git commit -q -m "deps: re-cut $n" + git format-patch -1 --stdout > "$dir/change.patch" + git checkout -q main + git branch -q -D tmp + ) + fi +} + +reset() { + export FIX="$here/fix" RUNNER_TEMP="$here/fix/tmp" + rm -rf "$FIX" + mkdir -p "$RUNNER_TEMP" + : > "$FIX/calls" + make_repo +} + +run() { + echo "$1" + (cd "$here/repo" && GH_TOKEN=t GITHUB_REPOSITORY=o/r GITHUB_SHA=abc PREFIX='Patched library update: ' EVENT=${EVENT:-schedule} \ + RUN_URL=https://github.com/o/r/actions/runs/9 PATH="$here/bin:$PATH" run_step "$here/publish.sh") > "$FIX/out" 2>&1 + status=$? +} +pushed() { git --git-dir="$here/origin.git" rev-parse -q --verify "refs/heads/$1" > /dev/null; } +not_pushed() { ! pushed "$1"; } + +HLS='{"name":"hls.js","current":"1.7.4","latest":"1.7.5","url":"https://example.invalid/hls","behind":true}' +DASH='{"name":"dashjs","current":"5.3.0","latest":"5.4.0","url":"https://example.invalid/dash","behind":true}' + +reset +recut 1 "$HLS" cut pnpm-lock.yaml patches/hls.js@1.7.4.patch +recut 2 "$DASH" stopped +run 'a clean re-cut and a stopped one' +check 'succeeds' test "$status" -eq 0 +check 'pushes the branch with the re-cut' pushed 'patched/hls.js-1.7.5' +check 'the branch holds the change' test "$(git --git-dir="$here/origin.git" show 'patched/hls.js-1.7.5:pnpm-lock.yaml')" = 'changed 1' +check 'opens the pull request' contains "$FIX/calls" 'pr create --base main --head patched/hls.js-1.7.5 --title Patched library update: hls.js 1.7.5' +check 'starts CI and the dependency review on it' contains "$FIX/calls" 'workflow run ci.yml --ref patched/hls.js-1.7.5' +check 'opens the issue for the stopped one' contains "$FIX/calls" 'issue create --title Patched library update: dashjs 5.4.0' +check 'pushes nothing for the stopped one' not_pushed 'patched/dashjs-5.4.0' + +reset +recut 1 "$HLS" cut pnpm-lock.yaml .github/workflows/ci.yml +run 'a re-cut that changes a workflow' +check 'fails' test "$status" -ne 0 +check 'says what it changed' contains "$FIX/out" '.github/workflows/ci.yml' +check 'pushes nothing' not_pushed 'patched/hls.js-1.7.5' +check 'opens nothing' lacks "$FIX/calls" 'create' + +reset +recut 1 '{"name":"hls.js;touch x","current":"1.7.4","latest":"1.7.5","url":"u"}' stopped +run 'a library name that is no npm name' +check 'fails' test "$status" -ne 0 +check 'opens nothing' lacks "$FIX/calls" 'create' + +reset +recut 1 '{"name":"@scope/lib","current":"1.0.0","latest":"1.0.1-beta.2","url":"u"}' stopped +run 'a scoped name and a pre-release' +check 'succeeds' test "$status" -eq 0 +check 'opens the issue' contains "$FIX/calls" 'issue create --title Patched library update: @scope/lib 1.0.1-beta.2' + +reset +run 'nothing handed on' +check 'succeeds' test "$status" -eq 0 +check 'opens nothing' lacks "$FIX/calls" 'create' + +reset +recut 1 "$HLS" cut pnpm-lock.yaml +EVENT=push run 'a push: only closes' +check 'succeeds' test "$status" -eq 0 +check 'publishes nothing' lacks "$FIX/calls" 'create' + +finish From 3d1ebb4b8b81f6bf89e84aee5f2c57ba78756f13 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:37:59 +0100 Subject: [PATCH 02/17] MP4: a start that gets nowhere waits at the steady pace, not every millisecond 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 --- chrome/player/players/mp4/LoopPace.mjs | 33 ++++++++++++++++++ chrome/player/players/mp4/MP4Player.mjs | 35 +++++++++++-------- tests/unit/LoopPace.test.mjs | 45 +++++++++++++++++++++++++ tests/unit/typeChecked.test.mjs | 1 + 4 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 chrome/player/players/mp4/LoopPace.mjs create mode 100644 tests/unit/LoopPace.test.mjs diff --git a/chrome/player/players/mp4/LoopPace.mjs b/chrome/player/players/mp4/LoopPace.mjs new file mode 100644 index 00000000..c44c273e --- /dev/null +++ b/chrome/player/players/mp4/LoopPace.mjs @@ -0,0 +1,33 @@ +// @ts-check +// How long MP4Player's main loop waits before its next round (MP4Player.mainLoop). Pure, so +// it can be tested without the player (which loads mp4box and the MediaSource code). + +/** Playing with media ahead: a range that ends starts the next itself. */ +export const STEADY_LOOP_MS = 50; +/** Paused or ended: a seek or a play wakes the loop. */ +export const PAUSED_LOOP_MS = 250; +/** At once (Firefox makes 1 ms 4). */ +export const URGENT_LOOP_MS = 1; +/** + * How long a start (no first frame yet) keeps the urgent pace after its last progress: the + * start, an append, the element's first frame data, a seek or a play. Each of those wakes the + * loop itself; past this, a start that makes no progress - data the element cannot decode, a + * file it refuses - waits at the steady pace. It woke the page 250 times a second for as long + * as its tab stayed open (the review of 2026-11-13, "needsInit"). + */ +export const INIT_URGENT_MS = 2000; + +/** + * @param {{ended: boolean, paused: boolean, seeking: boolean, readyState: number, + * needsInit: boolean, sinceProgress: number}} state - The element's state, whether the + * player still waits for its first frame, and the ms since the start's last progress. + * @return {number} The ms until the loop's next round. + */ +export function loopDelay({ended, paused, seeking, readyState, needsInit, sinceProgress}) { + if (ended) return PAUSED_LOOP_MS; + // A start: urgent while it gets somewhere, playing or not; then steady, playing or not (a + // video set to play that never gets its first frame is out of media for good). + if (needsInit) return sinceProgress < INIT_URGENT_MS ? URGENT_LOOP_MS : STEADY_LOOP_MS; + if (seeking || (!paused && readyState < 3)) return URGENT_LOOP_MS; + return paused ? PAUSED_LOOP_MS : STEADY_LOOP_MS; +} diff --git a/chrome/player/players/mp4/MP4Player.mjs b/chrome/player/players/mp4/MP4Player.mjs index cacff7ea..e67cf2c7 100644 --- a/chrome/player/players/mp4/MP4Player.mjs +++ b/chrome/player/players/mp4/MP4Player.mjs @@ -34,6 +34,7 @@ import {MP4FragmentRequester} from './MP4FragmentRequester.mjs'; */ import {keyframeOffset, sampledDuration} from './SampleIndex.mjs'; import {DurationSync} from './DurationSync.mjs'; +import {loopDelay} from './LoopPace.mjs'; import {SegmentAppender} from './SegmentAppender.mjs'; import {SourceBufferWrapper, removalPending} from './SourceBufferWrapper.mjs'; import {SaveFragmentFetcher, savedEntry} from '../SaveFragmentFetcher.mjs'; @@ -45,11 +46,6 @@ const BACK_BUFFER_SLACK = 1; // again after each of these waits, in ms. Still failing, it is the player's error once // playback has reached it. const RANGE_RETRY_DELAYS_MS = [2000, 4000, 8000]; -// How often the main loop runs while the video plays with media ahead, in ms (mainLoop). -const STEADY_LOOP_MS = 50; -// How often it runs while the video is paused or has ended, in ms. A seek or a play runs the -// next round at once (wakeLoop). -const PAUSED_LOOP_MS = 250; const VIDEO_TRACK = 0; const AUDIO_TRACK = 1; @@ -75,6 +71,9 @@ export default class MP4Player extends EventEmitter { this.client = client; // One function for every round of the main loop, not a new one each time. this.boundMainLoop = this.mainLoop.bind(this); + // Waiting for the first frame (setSource), and when that start last got somewhere. + this.needsInit = false; + this.initProgressAt = 0; this.isPreview = config?.isPreview || false; this.isAudioOnly = config?.isAudioOnly || false; @@ -82,6 +81,10 @@ export default class MP4Player extends EventEmitter { this.video = /** @type {HTMLVideoElement} */ (document.createElement(this.isAudioOnly ? 'audio' : 'video')); // A seek or a play while the main loop waits at its paused pace: its next round at once. this.video.addEventListener('play', () => this.wakeLoop()); + // The first frame's data: the start is over (needsInit), at the loop's next round. + this.video.addEventListener('loadeddata', () => { + if (this.needsInit) this.wakeLoop(); + }); this.video.addEventListener('seeking', () => { this.wakeLoop(); this.onElementSeeking(); @@ -145,7 +148,12 @@ export default class MP4Player extends EventEmitter { // The element gets _duration once its SourceBuffers let it (DurationSync.mjs). this.durationSync = new DurationSync(() => this.mediaSource, () => /** @type {SourceBufferWrapper[]} */ ([this.videoSourceBuffer, this.audioSourceBuffer].filter(Boolean))); - this.boundApplyDuration = () => this.durationSync.apply(); + // On each SourceBuffer's updateend. While the player waits for its first frame, an + // append is its progress: the loop's next round at once (it seeks to the buffered start). + this.boundApplyDuration = () => { + this.durationSync.apply(); + if (this.needsInit) this.wakeLoop(); + }; // A seek past the element's duration, waiting for the duration to be set; null: none. /** @type {?number} */ this.pendingSeek = null; @@ -508,6 +516,7 @@ export default class MP4Player extends EventEmitter { this.source = source; this.needsInit = true; + this.initProgressAt = performance.now(); const levelID = this.getCurrentVideoLevelID(); if (!this.client.getFragment(levelID, 0)) { @@ -573,15 +582,11 @@ export default class MP4Player extends EventEmitter { // 20 times a second while paused - the seek preview, a second player, always is - and an // MP4 that had ended, whose readyState stays below 3 with nothing ahead, 250 times a // second, for as long as its tab stayed open (measured with tools/bench-ab.mjs). + // A start that gets nowhere (no first frame 2 s after its last append, seek or play: data + // the element cannot decode) waits at the steady pace: LoopPace.mjs. const video = this.video; - let delay = STEADY_LOOP_MS; - if (video.ended) { - delay = PAUSED_LOOP_MS; - } else if (this.needsInit || video.seeking || (!video.paused && this.readyState < 3)) { - delay = 1; - } else if (video.paused) { - delay = PAUSED_LOOP_MS; - } + const delay = loopDelay({ended: video.ended, paused: video.paused, seeking: video.seeking, + readyState: this.readyState, needsInit: this.needsInit, sinceProgress: performance.now() - this.initProgressAt}); this.loopTimeout = setTimeout(this.boundMainLoop, delay); } @@ -589,6 +594,8 @@ export default class MP4Player extends EventEmitter { * Runs the main loop's next round at once, not at its paused pace: a seek or a play. */ wakeLoop() { + // Progress for a start that waits for its first frame (LoopPace.INIT_URGENT_MS). + this.initProgressAt = performance.now(); // null: a round is running now, and arms the next itself. if (!this.running || this.loopTimeout === null) return; clearTimeout(this.loopTimeout); diff --git a/tests/unit/LoopPace.test.mjs b/tests/unit/LoopPace.test.mjs new file mode 100644 index 00000000..ae79a983 --- /dev/null +++ b/tests/unit/LoopPace.test.mjs @@ -0,0 +1,45 @@ +import fc from 'fast-check'; +import {describe, expect, it} from 'vitest'; + +import {INIT_URGENT_MS, loopDelay, PAUSED_LOOP_MS, STEADY_LOOP_MS, URGENT_LOOP_MS} from '../../chrome/player/players/mp4/LoopPace.mjs'; + +// MP4Player's main loop: at once while something is urgent, a quarter of a second while +// nothing can change by itself. A start that never got its first frame (data the element cannot +// decode) ran the loop 250 times a second for as long as its tab stayed open. + +const playing = {ended: false, paused: false, seeking: false, readyState: 4, needsInit: false, sinceProgress: 0}; + +describe('The MP4 player\'s loop pace', () => { + it('runs steadily while it plays with media ahead, and slowly while paused or ended', () => { + expect(loopDelay(playing)).toBe(STEADY_LOOP_MS); + expect(loopDelay({...playing, paused: true})).toBe(PAUSED_LOOP_MS); + expect(loopDelay({...playing, ended: true, readyState: 2})).toBe(PAUSED_LOOP_MS); + }); + + it('runs at once while seeking or out of media', () => { + expect(loopDelay({...playing, seeking: true})).toBe(URGENT_LOOP_MS); + expect(loopDelay({...playing, seeking: true, paused: true})).toBe(URGENT_LOOP_MS); + expect(loopDelay({...playing, readyState: 2})).toBe(URGENT_LOOP_MS); + }); + + it('runs at once while a start gets somewhere, and steadily once it has not for a while', () => { + const start = {...playing, paused: true, readyState: 1, needsInit: true}; + expect(loopDelay({...start, sinceProgress: 0})).toBe(URGENT_LOOP_MS); + expect(loopDelay({...start, sinceProgress: INIT_URGENT_MS - 1})).toBe(URGENT_LOOP_MS); + expect(loopDelay({...start, sinceProgress: INIT_URGENT_MS})).toBe(STEADY_LOOP_MS); + // Set to play, and never a first frame: not out of media for a moment, but for good. + expect(loopDelay({...start, paused: false, sinceProgress: 60000})).toBe(STEADY_LOOP_MS); + expect(loopDelay({...start, seeking: true, sinceProgress: 60000})).toBe(STEADY_LOOP_MS); + }); + + it('never waits longer than paused, nor more than the steady pace while a start goes on', () => { + const state = fc.record({ended: fc.boolean(), paused: fc.boolean(), seeking: fc.boolean(), + readyState: fc.integer({min: 0, max: 4}), needsInit: fc.boolean(), sinceProgress: fc.nat({max: 1e7})}); + fc.assert(fc.property(state, (s) => { + const delay = loopDelay(s); + expect([URGENT_LOOP_MS, STEADY_LOOP_MS, PAUSED_LOOP_MS]).toContain(delay); + if (s.needsInit && !s.ended) expect(delay).toBeLessThanOrEqual(STEADY_LOOP_MS); + if (!s.needsInit && !s.ended && (s.seeking || (!s.paused && s.readyState < 3))) expect(delay).toBe(URGENT_LOOP_MS); + })); + }); +}); diff --git a/tests/unit/typeChecked.test.mjs b/tests/unit/typeChecked.test.mjs index dc50c396..980d139e 100644 --- a/tests/unit/typeChecked.test.mjs +++ b/tests/unit/typeChecked.test.mjs @@ -131,6 +131,7 @@ const CHECKED = [ 'chrome/player/players/mp4/AacLayout.mjs', 'chrome/player/players/mp4/AudioTracks.mjs', 'chrome/player/players/mp4/DurationSync.mjs', + 'chrome/player/players/mp4/LoopPace.mjs', 'chrome/player/players/mp4/MP4Fragment.mjs', 'chrome/player/players/mp4/MP4FragmentRequester.mjs', 'chrome/player/players/mp4/MP4Player.mjs', From 3639f542189d4faedd322c2e5daac0edca5a99c9 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:38:14 +0100 Subject: [PATCH 03/17] MP4: a fragmented file's keyframe index takes in only the new samples 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 --- chrome/player/players/mp4/MP4Player.mjs | 27 +++-------- chrome/player/players/mp4/SampleIndex.mjs | 46 ++++++++++++++++++ tests/unit/SampleIndex.test.mjs | 57 +++++++++++++++++++++++ 3 files changed, 110 insertions(+), 20 deletions(-) create mode 100644 tests/unit/SampleIndex.test.mjs diff --git a/chrome/player/players/mp4/MP4Player.mjs b/chrome/player/players/mp4/MP4Player.mjs index e67cf2c7..5ffc3f51 100644 --- a/chrome/player/players/mp4/MP4Player.mjs +++ b/chrome/player/players/mp4/MP4Player.mjs @@ -27,12 +27,12 @@ import {MP4FragmentRequester} from './MP4FragmentRequester.mjs'; /** @typedef {import('../../network/FetchLoader.mjs').FetchLoader['stats']} LoadStats */ /** * A track of the file as this player reads it (onMetadataParsed): mp4box's trak box and - * track, its samples, and its keyframes by time (sortSamples), sorted again when a + * track, its samples, and its keyframes by time (sortKeyframes), brought up to date when a * fragmented file's samples grow (refreshSampleIndex). * @typedef {{trak: TrakBox|undefined, track: Track, samples: Array, * sortedSamples: Array, sortedCount: number}} PlayedTrack */ -import {keyframeOffset, sampledDuration} from './SampleIndex.mjs'; +import {addKeyframes, keyframeOffset, sampledDuration, sortKeyframes} from './SampleIndex.mjs'; import {DurationSync} from './DurationSync.mjs'; import {loopDelay} from './LoopPace.mjs'; import {SegmentAppender} from './SegmentAppender.mjs'; @@ -405,20 +405,6 @@ export default class MP4Player extends EventEmitter { })); } - /** - * @param {Array} samples - * @return {Array} Its keyframes, by composition time. - */ - sortSamples(samples) { - samples = samples.filter((sample) => { - return sample.is_sync; - }); - samples.sort((a, b) => { - return a.cts - b.cts; - }); - return samples; - } - estimateTotalSizeFromMetadats() { if (this.fileLength || !this.metaData) return; // A fragmented file's samples are only those of the fragments parsed so far: taken for its @@ -468,7 +454,7 @@ export default class MP4Player extends EventEmitter { trak, track: info.videoTracks[this.currentVideoTrack], samples: samples, - sortedSamples: this.sortSamples(samples), + sortedSamples: sortKeyframes(samples), sortedCount: samples.length, }); } @@ -482,7 +468,7 @@ export default class MP4Player extends EventEmitter { trak, track: info.audioTracks[l], samples: samples, - sortedSamples: this.sortSamples(samples), + sortedSamples: sortKeyframes(samples), sortedCount: samples.length, }); } @@ -666,14 +652,15 @@ export default class MP4Player extends EventEmitter { * mp4box adds them to the track's sample list. The keyframe index that picks the range to * load, and the fragments' times, were made only from the samples known when the metadata * was parsed, so the player loaded the first maxFragmentsBuffered ranges (30 MB) and - * stopped there. Both are made again whenever the list has grown. + * stopped there. Both are brought up to date whenever the list has grown: the keyframes + * with the new samples only (addKeyframes). */ refreshSampleIndex() { if (!this.metaData?.isFragmented) return; const tracks = [...this.videoTracks, ...this.audioTracks]; if (tracks.every((track) => track.sortedCount === track.samples.length)) return; tracks.forEach((track) => { - track.sortedSamples = this.sortSamples(track.samples); + track.sortedSamples = addKeyframes(track.sortedSamples, track.samples, track.sortedCount); track.sortedCount = track.samples.length; }); this.setFragmentTimes(); diff --git a/chrome/player/players/mp4/SampleIndex.mjs b/chrome/player/players/mp4/SampleIndex.mjs index 0bd055cb..48658127 100644 --- a/chrome/player/players/mp4/SampleIndex.mjs +++ b/chrome/player/players/mp4/SampleIndex.mjs @@ -15,6 +15,7 @@ import {Utils} from '../../utils/Utils.mjs'; * @property {number} timescale * @property {number} cts * @property {number} offset + * @property {boolean} is_sync */ // MP4Player's arithmetic on mp4box's sample lists, kept apart from MP4Player so that unit @@ -41,6 +42,51 @@ export function sampledDuration(traks) { }, 0); } +/** + * A track's keyframes, by composition time (stable: of two at the same time, the one first in + * the list comes first). + * @template {Pick} T + * @param {T[]} samples + * @return {T[]} A new array. + */ +export function sortKeyframes(samples) { + return samples.filter((sample) => sample.is_sync).sort((a, b) => a.cts - b.cts); +} + +/** + * A track's keyframes brought up to date with the samples mp4box added to its list since + * `count` (a fragmented file's, one moof at a time): what sortKeyframes(samples) gives, with + * only the new ones sorted. Sorting the whole list again for every range took 7.7 ms a range + * on a 2-hour file (338,000 audio samples, each a keyframe). New keyframes that all come at or + * after the last one, as they do while a file plays on, go onto the end of `sorted` itself; + * those of an earlier part (read after a seek back) are merged in, into a new array. mp4box + * only appends to a track's list (it builds it once, at the moov). + * @template {Pick} T + * @param {T[]} sorted - sortKeyframes(samples.slice(0, count)). + * @param {T[]} samples - The track's samples, `count` of them known before. + * @param {number} count + * @return {T[]} `sorted` itself or a new array. + */ +export function addKeyframes(sorted, samples, count) { + const added = sortKeyframes(samples.slice(count)); + if (!added.length) return sorted; + if (!sorted.length || sorted[sorted.length - 1].cts <= added[0].cts) { + for (const sample of added) sorted.push(sample); + return sorted; + } + // Of two at the same time the one from before comes first, as in a sort of the whole list. + const merged = new Array(sorted.length + added.length); + let i = 0; + let j = 0; + let k = 0; + while (i < sorted.length && j < added.length) { + merged[k++] = sorted[i].cts <= added[j].cts ? sorted[i++] : added[j++]; + } + while (i < sorted.length) merged[k++] = sorted[i++]; + while (j < added.length) merged[k++] = added[j++]; + return merged; +} + /** * The byte offset of the last keyframe at or before a time, in one track's keyframes. * @param {Sample[]} samples - The keyframes, sorted by time; not empty. diff --git a/tests/unit/SampleIndex.test.mjs b/tests/unit/SampleIndex.test.mjs new file mode 100644 index 00000000..39eae618 --- /dev/null +++ b/tests/unit/SampleIndex.test.mjs @@ -0,0 +1,57 @@ +import {describe, expect, it} from 'vitest'; +import fc from 'fast-check'; +import {addKeyframes, sortKeyframes} from '../../chrome/player/players/mp4/SampleIndex.mjs'; + +// A fragmented MP4's keyframe index (MP4Player.refreshSampleIndex) is brought up to date with +// the samples mp4box adds as each range is parsed. It was made again from all of them each +// time: 7.7 ms a range on a 2-hour file (338,000 audio samples, each a keyframe), on the +// player's thread. addKeyframes sorts only the new ones in, and must give exactly what the full +// sort gives: the same samples, in the same order, ties included. + +/** @param {number} cts @param {boolean} sync @param {number} id */ +const sample = (cts, sync, id) => ({cts, is_sync: sync, offset: id, timescale: 1000, id}); + +describe('addKeyframes', () => { + it('appends new keyframes that all come later, in the same array', () => { + const samples = [sample(0, true, 0), sample(40, false, 1), sample(80, true, 2)]; + const sorted = sortKeyframes(samples); + samples.push(sample(120, false, 3), sample(160, true, 4)); + const next = addKeyframes(sorted, samples, 3); + expect(next).toBe(sorted); + expect(next.map((s) => s.id)).toEqual([0, 2, 4]); + }); + + it('merges keyframes of an earlier part parsed later (a seek back)', () => { + const samples = [sample(1000, true, 0), sample(2000, true, 1)]; + const sorted = sortKeyframes(samples); + samples.push(sample(500, true, 2), sample(1500, true, 3), sample(1500, false, 4)); + expect(addKeyframes(sorted, samples, 2).map((s) => s.id)).toEqual([2, 0, 3, 1]); + }); + + it('keeps the earlier sample first among equal times, as the full sort does', () => { + const samples = [sample(100, true, 0)]; + const sorted = sortKeyframes(samples); + samples.push(sample(100, true, 1), sample(50, true, 2), sample(100, true, 3)); + expect(addKeyframes(sorted, samples, 1).map((s) => s.id)).toEqual([2, 0, 1, 3]); + }); + + it('gives what sorting every sample again gives, for any samples added in any steps', () => { + fc.assert(fc.property( + fc.array(fc.tuple(fc.integer({min: 0, max: 50}), fc.boolean()), {maxLength: 200}), + fc.array(fc.nat({max: 200}), {maxLength: 10}), + (all, cuts) => { + const full = all.map(([cts, sync], id) => sample(cts, sync, id)); + const steps = [...new Set(cuts.map((cut) => Math.min(cut, full.length)))].sort((a, b) => a - b); + /** @type {ReturnType[]} */ + const samples = []; + let sorted = sortKeyframes(samples); + let count = 0; + for (const end of [...steps, full.length]) { + samples.push(...full.slice(samples.length, end)); + sorted = addKeyframes(sorted, samples, count); + count = samples.length; + } + expect(sorted.map((s) => s.id)).toEqual(sortKeyframes(full).map((s) => s.id)); + }), {numRuns: 500}); + }); +}); From 31c335c8d34614cd8d51516b992bd1bfe5b42014 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:38:24 +0100 Subject: [PATCH 04/17] players: one fragment status for the DASH, HLS and MP4 requesters; DASH'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 --- chrome/player/players/FragmentStatus.mjs | 77 +++++++++++++ .../players/dash/DashFragmentRequester.mjs | 89 +-------------- chrome/player/players/dash/DashLoader.mjs | 35 +----- .../players/hls/HLSFragmentRequester.mjs | 58 ++-------- .../players/mp4/MP4FragmentRequester.mjs | 58 ++-------- tests/unit/FragmentStatus.test.mjs | 107 ++++++++++++++++++ tests/unit/typeChecked.test.mjs | 1 + 7 files changed, 215 insertions(+), 210 deletions(-) create mode 100644 chrome/player/players/FragmentStatus.mjs create mode 100644 tests/unit/FragmentStatus.test.mjs diff --git a/chrome/player/players/FragmentStatus.mjs b/chrome/player/players/FragmentStatus.mjs new file mode 100644 index 00000000..95fef859 --- /dev/null +++ b/chrome/player/players/FragmentStatus.mjs @@ -0,0 +1,77 @@ +// @ts-check +import {DefaultPlayerEvents} from '../enums/DefaultPlayerEvents.mjs'; +import {DownloadStatus} from '../enums/DownloadStatus.mjs'; + +/** @typedef {import('../network/DownloadEntry.mjs').DownloadEntry} DownloadEntry */ +/** @typedef {import('./Fragment.mjs').Fragment} Fragment */ +/** @typedef {{emit: (event: string, ...args: any[]) => any}} FragmentPlayer */ + +// A fragment's download status, kept and told to its player at each change: what the DASH, +// HLS and MP4 fragment requesters share (three copies until 2026-11-15). + +/** + * Sets a fragment's status and tells its player. + * @param {FragmentPlayer} player + * @param {Fragment} fragment + * @param {number} status - A DownloadStatus. + */ +function setStatus(player, fragment, status) { + fragment.status = status; + player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); +} + +/** + * A waiting fragment is downloading now; one in any other state is left as it is. + * @param {FragmentPlayer} player + * @param {Fragment} fragment + */ +export function markRequested(player, fragment) { + if (fragment.status === DownloadStatus.WAITING) { + setStatus(player, fragment, DownloadStatus.DOWNLOAD_INITIATED); + } +} + +/** + * The download's onSuccess, onFail and onAbort for a fragment: the stored data read back + * (unless skipProcess asks for none) and the fragment complete, then `succeed`; a read that + * fails counts as a failed download. A failed download leaves the fragment failed, an aborted + * one waiting (it is asked for again). + * @param {FragmentPlayer} player + * @param {Fragment} fragment + * @param {{skipProcess?: boolean, onFail: (entry: DownloadEntry) => void, + * onAbort?: (entry: DownloadEntry) => void}} callbacks - The requester's caller's. + * @param {(entry: DownloadEntry, data: Awaited>|undefined) => void} succeed - + * Told once the data is read: the requester hands it on in its player's form. + * @return {{onSuccess: (entry: DownloadEntry) => Promise, onFail: (entry: DownloadEntry) => void, + * onAbort: (entry: DownloadEntry) => void}} + */ +export function fragmentDownloadCallbacks(player, fragment, callbacks, succeed) { + return { + onSuccess: async (entry) => { + let data; + try { + if (!callbacks.skipProcess) { + data = await entry.getDataFromBlob(); + } + fragment.dataSize = entry.dataSize; + } catch (e) { + console.error(e); + setStatus(player, fragment, DownloadStatus.DOWNLOAD_FAILED); + callbacks.onFail(entry); + return; + } + if (fragment.status !== DownloadStatus.DOWNLOAD_COMPLETE) { + setStatus(player, fragment, DownloadStatus.DOWNLOAD_COMPLETE); + } + succeed(entry, data); + }, + onFail: (entry) => { + setStatus(player, fragment, DownloadStatus.DOWNLOAD_FAILED); + callbacks.onFail(entry); + }, + onAbort: (entry) => { + setStatus(player, fragment, DownloadStatus.WAITING); + callbacks.onAbort?.(entry); + }, + }; +} diff --git a/chrome/player/players/dash/DashFragmentRequester.mjs b/chrome/player/players/dash/DashFragmentRequester.mjs index 6c8482ad..8664a133 100644 --- a/chrome/player/players/dash/DashFragmentRequester.mjs +++ b/chrome/player/players/dash/DashFragmentRequester.mjs @@ -1,20 +1,8 @@ // @ts-check -import {DefaultPlayerEvents} from '../../enums/DefaultPlayerEvents.mjs'; -import {DownloadStatus} from '../../enums/DownloadStatus.mjs'; +import {fragmentDownloadCallbacks, markRequested} from '../FragmentStatus.mjs'; /** @typedef {import('../../network/DownloadEntry.mjs').DownloadEntry} DownloadEntry */ -/** - * The DashPlayer, with the hooks a player may have to change a fragment's request or answer; - * no player has them now, so both are optional. - * @typedef {import('./DashPlayer.mjs').default & { - * preProcessFragment?: (entry: DownloadEntry, request: Awaited>, - * startTime: number, isInit: boolean) => Promise>>, - * postProcessFragment?: (entry: DownloadEntry, response: import('../../network/FetchLoader.mjs').LoaderResponse, - * startTime: number, isInit: boolean) => Promise - * }} HookedDashPlayer - */ - /** * What a fragment's request tells (requestFragment): the download's entry once it is * stored, with its data unless skipProcess asked for none; FetchLoader's progress, with the @@ -29,7 +17,7 @@ import {DownloadStatus} from '../../enums/DownloadStatus.mjs'; export class DashFragmentRequester { /** - * @param {HookedDashPlayer} player + * @param {import('./DashPlayer.mjs').default} player */ constructor(player) { this.player = player; @@ -37,7 +25,7 @@ export class DashFragmentRequester { /** * Downloads a fragment through the download manager, keeping its status and telling the - * player each change of it. + * player each change of it (FragmentStatus.mjs). * @param {import('./DashFragment.mjs').DashFragment} fragment * @param {FragmentCallbacks} callbacks * @param {?{headers?: Object}} config - The download's config (null for @@ -49,12 +37,9 @@ export class DashFragmentRequester { const context = fragment.getContext(); config = config || {}; - if (fragment.status === DownloadStatus.WAITING) { - fragment.status = DownloadStatus.DOWNLOAD_INITIATED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - } + markRequested(this.player, fragment); - const loader = this.player.getClient().downloadManager.getFile({ + return this.player.getClient().downloadManager.getFile({ ...context, config, headers: { @@ -62,56 +47,8 @@ export class DashFragmentRequester { // Set: fragments are asked for only once the player has its source. ...(/** @type {import('../../VideoSource.mjs').VideoSource} */ (this.player.source)).headers, }, - /** - * @param {DownloadEntry} entry - * @param {Awaited>} request - The request it makes. - */ - preProcessor: async (entry, request) => { - if (this.player.preProcessFragment) { - const startTime = fragment.start; - const isInit = fragment.sn === -1; - return await this.player.preProcessFragment(entry, request, startTime, isInit); - } - return request; - }, - /** - * @param {DownloadEntry} entry - * @param {{url: string, status: number, headers: Object, data: ArrayBuffer|string}} response - - * FetchLoader's answer. - */ - postProcessor: async (entry, response) => { - if (this.player.postProcessFragment) { - const startTime = fragment.start; - const isInit = fragment.sn === -1; - return await this.player.postProcessFragment(entry, response, startTime, isInit); - } - return response; - }, }, { - /** - * @param {DownloadEntry} entry - * @param {null} [xhr] - FetchLoader has none; none at all for a stored copy. - */ - onSuccess: async (entry, xhr) => { - let data; - try { - if (!callbacks.skipProcess) { - data = await entry.getDataFromBlob(); - } - fragment.dataSize = entry.dataSize; - } catch (e) { - console.error(e); - fragment.status = DownloadStatus.DOWNLOAD_FAILED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - callbacks.onFail(entry); - return; - } - if (fragment.status !== DownloadStatus.DOWNLOAD_COMPLETE) { - fragment.status = DownloadStatus.DOWNLOAD_COMPLETE; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - } - callbacks.onSuccess(entry, data); - }, + ...fragmentDownloadCallbacks(this.player, fragment, callbacks, (entry, data) => callbacks.onSuccess(entry, data)), /** * @param {import('../../network/FetchLoader.mjs').FetchLoader['stats']} stats * @param {Awaited>} context2 - The request. @@ -121,20 +58,6 @@ export class DashFragmentRequester { onProgress: (stats, context2, data, xhr) => { if (callbacks.onProgress) callbacks.onProgress(stats, context, data, xhr); }, - /** @param {DownloadEntry} entry */ - onFail: (entry) => { - fragment.status = DownloadStatus.DOWNLOAD_FAILED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - callbacks.onFail(entry); - }, - /** @param {DownloadEntry} entry */ - onAbort: (entry) => { - fragment.status = DownloadStatus.WAITING; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - if (callbacks.onAbort) callbacks.onAbort(entry); - }, }, priority); - - return loader; } } diff --git a/chrome/player/players/dash/DashLoader.mjs b/chrome/player/players/dash/DashLoader.mjs index 1fec2258..549c1868 100644 --- a/chrome/player/players/dash/DashLoader.mjs +++ b/chrome/player/players/dash/DashLoader.mjs @@ -93,7 +93,7 @@ function isSameRequest(a, b) { /** - * @param {import('./DashFragmentRequester.mjs').HookedDashPlayer} player + * @param {import('./DashPlayer.mjs').default} player */ export function DASHLoaderFactory(player) { // Failures in a row, per segment, for this player; a success forgets them. @@ -130,7 +130,7 @@ export function DASHLoaderFactory(player) { const representation = requestObj.representation; if (!representation) { console.error('Representation not found', requestObj); - request(httpRequest, end, true, requestObj.startTime || 0, true); + request(httpRequest, end, true); return; } let segmentIndex = requestObj.index; @@ -152,7 +152,7 @@ export function DASHLoaderFactory(player) { if (!frag) { console.warn('Fragment not found', requestObj, level, player.client.getFragments(level)); // throw new Error("Fragment not found"); - request(httpRequest, end, true, requestObj.startTime || 0, requestObj.type === 'InitializationSegment' || isNaN(segmentIndex)); + request(httpRequest, end, true); return; } @@ -199,10 +199,8 @@ export function DASHLoaderFactory(player) { * @param {DashLoaderRequest} httpRequest * @param {ReturnType} end * @param {boolean} [isSegment] - * @param {number} [startTime] - A segment's, in seconds. - * @param {boolean} [isInit] - Whether the segment is an init segment. */ - function request(httpRequest, end, isSegment = false, startTime = 0, isInit = false) { + function request(httpRequest, end, isSegment = false) { // Variables will be used in the callback functions const request = httpRequest.customData.request; @@ -256,30 +254,7 @@ export function DASHLoaderFactory(player) { if (player.loadedManifests.has(key)) downloadManager.forgetCompletedFile(context); player.loadedManifests.add(key); } - const loader = downloadManager.getFile({ - ...context, - /** - * @param {DownloadEntry} entry - * @param {Awaited>} request - The request it makes. - */ - preProcessor: async (entry, request) => { - if (isSegment && player.preProcessFragment) { - return await player.preProcessFragment(entry, request, startTime, isInit); - } - return request; - }, - /** - * @param {DownloadEntry} entry - * @param {{url: string, status: number, headers: Object, data: ArrayBuffer|string}} response - - * FetchLoader's answer. - */ - postProcessor: async (entry, response) => { - if (isSegment && player.postProcessFragment) { - return await player.postProcessFragment(entry, response, startTime, isInit); - } - return response; - }, - }, { + const loader = downloadManager.getFile(context, { /** * @param {DownloadEntry} entry * @param {null} [xhr] - FetchLoader has none; none at all for a stored copy. diff --git a/chrome/player/players/hls/HLSFragmentRequester.mjs b/chrome/player/players/hls/HLSFragmentRequester.mjs index 14f56385..feae33c8 100644 --- a/chrome/player/players/hls/HLSFragmentRequester.mjs +++ b/chrome/player/players/hls/HLSFragmentRequester.mjs @@ -1,7 +1,7 @@ // @ts-check import {DefaultPlayerEvents} from '../../enums/DefaultPlayerEvents.mjs'; -import {DownloadStatus} from '../../enums/DownloadStatus.mjs'; import {isDecryptable} from '../../utils/Decryptable.mjs'; +import {fragmentDownloadCallbacks, markRequested} from '../FragmentStatus.mjs'; import {HLSDecrypter} from './HLSDecrypter.mjs'; /** @typedef {import('../../modules/hls.mjs').Fragment} HlsFragment */ @@ -128,7 +128,8 @@ export class HLSFragmentRequester { /** * Downloads a segment through the download manager, decrypting it if its decryption was - * taken over, keeping its status and telling the player each change of it. + * taken over, keeping its status and telling the player each change of it + * (FragmentStatus.mjs). * @param {import('./HLSFragment.mjs').HLSFragment} fragment * @param {FragmentCallbacks} callbacks * @param {?(Partial & {headers?: Object})} config - @@ -149,10 +150,7 @@ export class HLSFragmentRequester { throw new Error('unexpected decryptdata'); } - if (fragment.status === DownloadStatus.WAITING) { - fragment.status = DownloadStatus.DOWNLOAD_INITIATED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - } + markRequested(this.player, fragment); const headers = { ...config.headers, @@ -161,42 +159,20 @@ export class HLSFragmentRequester { }; const decrypt = this.decryptionFor(frag, headers); - const loader = this.player.getClient().downloadManager.getFile({ + return this.player.getClient().downloadManager.getFile({ ...context, config, headers, postProcessor: decrypt || (async (entry, response) => response), }, { - /** - * @param {DownloadEntry} entry - * @param {null} [xhr] - FetchLoader has none; none at all for a stored copy. - */ - onSuccess: async (entry, xhr) => { - /** @type {ArrayBuffer|undefined} */ - let data; - try { - if (!callbacks.skipProcess) { - // An ArrayBuffer: a segment is asked for as one (HLSFragment.getContext). - data = /** @type {ArrayBuffer} */ (await entry.getDataFromBlob()); - } - fragment.dataSize = entry.dataSize; - } catch (e) { - console.error(e); - fragment.status = DownloadStatus.DOWNLOAD_FAILED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - callbacks.onFail(entry); - return; - } - if (fragment.status !== DownloadStatus.DOWNLOAD_COMPLETE) { - fragment.status = DownloadStatus.DOWNLOAD_COMPLETE; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - } - // The data is there exactly when skipProcess did not ask for none (FragmentCallbacks). + ...fragmentDownloadCallbacks(this.player, fragment, callbacks, (entry, data) => { + // An ArrayBuffer (a segment is asked for as one: HLSFragment.getContext), there exactly + // when skipProcess did not ask for none (FragmentCallbacks). /** @type {(response: {url: string, data: ArrayBuffer|undefined}, stats: LoaderStats, context: SegmentContext, xhr: null) => void} */ (callbacks.onSuccess)({ url: entry.url, - data: data, + data: /** @type {ArrayBuffer|undefined} */ (data), }, /** @type {LoaderStats} */ (entry.stats), context, null); // set on success - }, + }), /** * @param {LoaderStats} stats * @param {import('../../network/DownloadEntry.mjs').DownloadRequest} context2 - The request. @@ -207,20 +183,6 @@ export class HLSFragmentRequester { // An ArrayBuffer: a segment is asked for as one. if (callbacks.onProgress) callbacks.onProgress(stats, context, /** @type {ArrayBuffer} */ (data), xhr); }, - /** @param {DownloadEntry} entry */ - onFail: (entry) => { - fragment.status = DownloadStatus.DOWNLOAD_FAILED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - callbacks.onFail(entry); - }, - /** @param {DownloadEntry} entry */ - onAbort: (entry) => { - fragment.status = DownloadStatus.WAITING; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - if (callbacks.onAbort) callbacks.onAbort(entry); - }, }, priority); - - return loader; } } diff --git a/chrome/player/players/mp4/MP4FragmentRequester.mjs b/chrome/player/players/mp4/MP4FragmentRequester.mjs index 920e551a..d0538a42 100644 --- a/chrome/player/players/mp4/MP4FragmentRequester.mjs +++ b/chrome/player/players/mp4/MP4FragmentRequester.mjs @@ -1,6 +1,5 @@ // @ts-check -import {DefaultPlayerEvents} from '../../enums/DefaultPlayerEvents.mjs'; -import {DownloadStatus} from '../../enums/DownloadStatus.mjs'; +import {fragmentDownloadCallbacks, markRequested} from '../FragmentStatus.mjs'; /** @typedef {import('../../network/DownloadEntry.mjs').DownloadEntry} DownloadEntry */ /** @typedef {import('../../modules/mp4box/mp4box.all.mjs').MP4BoxBuffer} MP4BoxBuffer */ @@ -27,7 +26,7 @@ export class MP4FragmentRequester { /** * Downloads a range through the download manager, keeping its status and telling the - * player each change of it. + * player each change of it (FragmentStatus.mjs). * @param {MP4Fragment} fragment * @param {FragmentCallbacks} callbacks * @param {?{headers?: Object}} config - The download's config (null for @@ -39,12 +38,9 @@ export class MP4FragmentRequester { const context = fragment.getContext(); config = config || {}; - if (fragment.status === DownloadStatus.WAITING) { - fragment.status = DownloadStatus.DOWNLOAD_INITIATED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - } + markRequested(this.player, fragment); - const loader = this.player.getClient().downloadManager.getFile({ + return this.player.getClient().downloadManager.getFile({ ...context, config, headers: { @@ -53,10 +49,6 @@ export class MP4FragmentRequester { .../** @type {import('../../VideoSource.mjs').VideoSource} */(this.player.source).headers, }, }, { - /** - * @param {DownloadEntry} entry - * @param {null} [xhr] - FetchLoader has none; none at all for a stored copy. - */ /** * A piece of the range as it downloads (progressive: MP4Player.feedPiece). * @param {Uint8Array} chunk @@ -64,32 +56,14 @@ export class MP4FragmentRequester { * @param {import('../../network/FetchLoader.mjs').PartialAnswer} answer */ onChunk: (chunk, offset, answer) => callbacks.onChunk?.(chunk, offset, answer), - onSuccess: async (entry, xhr) => { - /** @type {MP4BoxBuffer|undefined} */ - let data; - try { - if (!callbacks.skipProcess) { - // An ArrayBuffer (a range is asked for as one), an MP4BoxBuffer once its - // fileStart is set below. - data = /** @type {MP4BoxBuffer} */ (await entry.getDataFromBlob()); - } - fragment.dataSize = entry.dataSize; - } catch (e) { - console.error(e); - fragment.status = DownloadStatus.DOWNLOAD_FAILED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - callbacks.onFail(entry); - return; - } - if (fragment.status !== DownloadStatus.DOWNLOAD_COMPLETE) { - fragment.status = DownloadStatus.DOWNLOAD_COMPLETE; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - } - // A range's entry has its start (MP4Fragment.getContext). + ...fragmentDownloadCallbacks(this.player, fragment, callbacks, (entry, read) => { + // An ArrayBuffer (a range is asked for as one), an MP4BoxBuffer once its fileStart is + // set: a range's entry has its start (MP4Fragment.getContext). + const data = /** @type {MP4BoxBuffer|undefined} */ (read); if (data) data.fileStart = /** @type {number} */ (entry.rangeStart); // The data is there exactly when skipProcess did not ask for none (FragmentCallbacks). /** @type {(entry: DownloadEntry, data: MP4BoxBuffer|undefined) => void} */ (callbacks.onSuccess)(entry, data); - }, + }), /** * @param {import('../../network/FetchLoader.mjs').LoaderStats} stats * @param {import('../../network/DownloadEntry.mjs').DownloadRequest} context2 - The request. @@ -100,20 +74,6 @@ export class MP4FragmentRequester { // An ArrayBuffer: a range is asked for as one. if (callbacks.onProgress) callbacks.onProgress(stats, context, /** @type {ArrayBuffer} */ (data), xhr); }, - /** @param {DownloadEntry} entry */ - onFail: (entry) => { - fragment.status = DownloadStatus.DOWNLOAD_FAILED; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - callbacks.onFail(entry); - }, - /** @param {DownloadEntry} entry */ - onAbort: (entry) => { - fragment.status = DownloadStatus.WAITING; - this.player.emit(DefaultPlayerEvents.FRAGMENT_UPDATE, fragment); - if (callbacks.onAbort) callbacks.onAbort(entry); - }, }, priority); - - return loader; } } diff --git a/tests/unit/FragmentStatus.test.mjs b/tests/unit/FragmentStatus.test.mjs new file mode 100644 index 00000000..407e0371 --- /dev/null +++ b/tests/unit/FragmentStatus.test.mjs @@ -0,0 +1,107 @@ +import {describe, expect, it, vi} from 'vitest'; +import {DefaultPlayerEvents} from '../../chrome/player/enums/DefaultPlayerEvents.mjs'; +import {DownloadStatus} from '../../chrome/player/enums/DownloadStatus.mjs'; +import {Fragment} from '../../chrome/player/players/Fragment.mjs'; +import {fragmentDownloadCallbacks, markRequested} from '../../chrome/player/players/FragmentStatus.mjs'; + +// The fragment status every player's fragment requester keeps (DASH, HLS, MP4: three copies +// of it until 2026-11-15). The progress bar draws from it, and the players pick by it what to +// download: a fragment left "downloading" after an abort was never asked for again. + +/** A player that records each FRAGMENT_UPDATE's status as it was told. */ +const player = () => { + const told = []; + return {told, emit: vi.fn((event, fragment) => { + if (event === DefaultPlayerEvents.FRAGMENT_UPDATE) told.push(fragment.status); + })}; +}; +const statuses = (p) => p.told; +const entry = (data, dataSize = 3) => ({dataSize, getDataFromBlob: vi.fn(async () => data)}); + +describe('markRequested', () => { + it('marks a waiting fragment as downloading, and tells the player', () => { + const p = player(); + const fragment = new Fragment(0, 1); + markRequested(p, fragment); + expect(fragment.status).toBe(DownloadStatus.DOWNLOAD_INITIATED); + expect(statuses(p)).toEqual([DownloadStatus.DOWNLOAD_INITIATED]); + }); + + it('leaves a fragment in any other state as it is, and says nothing', () => { + const p = player(); + const fragment = new Fragment(0, 1); + fragment.status = DownloadStatus.DOWNLOAD_COMPLETE; + markRequested(p, fragment); + expect(fragment.status).toBe(DownloadStatus.DOWNLOAD_COMPLETE); + expect(p.emit).not.toHaveBeenCalled(); + }); +}); + +describe('fragmentDownloadCallbacks', () => { + it('reads the data, marks the fragment complete, then hands the data on', async () => { + const p = player(); + const fragment = new Fragment(0, 1); + const succeed = vi.fn(() => expect(fragment.status).toBe(DownloadStatus.DOWNLOAD_COMPLETE)); + const data = new ArrayBuffer(3); + const e = entry(data); + await fragmentDownloadCallbacks(p, fragment, {onFail: vi.fn()}, succeed).onSuccess(e); + expect(succeed).toHaveBeenCalledWith(e, data); + expect(fragment.dataSize).toBe(3); + expect(statuses(p)).toEqual([DownloadStatus.DOWNLOAD_COMPLETE]); + }); + + it('reads nothing when skipProcess asks for none', async () => { + const p = player(); + const succeed = vi.fn(); + const e = entry(new ArrayBuffer(3)); + await fragmentDownloadCallbacks(p, new Fragment(0, 1), {skipProcess: true, onFail: vi.fn()}, succeed).onSuccess(e); + expect(e.getDataFromBlob).not.toHaveBeenCalled(); + expect(succeed).toHaveBeenCalledWith(e, undefined); + }); + + it('tells a fragment complete already nothing more', async () => { + const p = player(); + const fragment = new Fragment(0, 1); + fragment.status = DownloadStatus.DOWNLOAD_COMPLETE; + await fragmentDownloadCallbacks(p, fragment, {onFail: vi.fn()}, vi.fn()).onSuccess(entry(new ArrayBuffer(1))); + expect(p.emit).not.toHaveBeenCalled(); + }); + + it('counts stored data that cannot be read as a failed download', async () => { + const p = player(); + const fragment = new Fragment(0, 1); + const onFail = vi.fn(); + const succeed = vi.fn(); + const e = {dataSize: 0, getDataFromBlob: vi.fn(async () => { + throw new Error('gone'); + })}; + vi.spyOn(console, 'error').mockImplementation(() => {}); + await fragmentDownloadCallbacks(p, fragment, {onFail}, succeed).onSuccess(e); + expect(fragment.status).toBe(DownloadStatus.DOWNLOAD_FAILED); + expect(onFail).toHaveBeenCalledWith(e); + expect(succeed).not.toHaveBeenCalled(); + expect(statuses(p)).toEqual([DownloadStatus.DOWNLOAD_FAILED]); + }); + + it('leaves a failed download failed and an aborted one waiting, to be asked for again', () => { + const p = player(); + const fragment = new Fragment(0, 1); + const onFail = vi.fn(); + const onAbort = vi.fn(); + const callbacks = fragmentDownloadCallbacks(p, fragment, {onFail, onAbort}, vi.fn()); + const e = entry(null); + callbacks.onFail(e); + expect(fragment.status).toBe(DownloadStatus.DOWNLOAD_FAILED); + expect(onFail).toHaveBeenCalledWith(e); + callbacks.onAbort(e); + expect(fragment.status).toBe(DownloadStatus.WAITING); + expect(onAbort).toHaveBeenCalledWith(e); + expect(statuses(p)).toEqual([DownloadStatus.DOWNLOAD_FAILED, DownloadStatus.WAITING]); + }); + + it('takes an abort without an onAbort of the caller', () => { + const fragment = new Fragment(0, 1); + fragmentDownloadCallbacks(player(), fragment, {onFail: vi.fn()}, vi.fn()).onAbort(entry(null)); + expect(fragment.status).toBe(DownloadStatus.WAITING); + }); +}); diff --git a/tests/unit/typeChecked.test.mjs b/tests/unit/typeChecked.test.mjs index 980d139e..7145b23e 100644 --- a/tests/unit/typeChecked.test.mjs +++ b/tests/unit/typeChecked.test.mjs @@ -111,6 +111,7 @@ const CHECKED = [ 'chrome/player/players/DirectVideoPlayer.mjs', 'chrome/player/players/DroppedFrameWatch.mjs', 'chrome/player/players/Fragment.mjs', + 'chrome/player/players/FragmentStatus.mjs', 'chrome/player/players/LevelManager.mjs', 'chrome/player/players/Levels.mjs', 'chrome/player/players/PlayerLoader.mjs', From 01b35f9dd48e4adb0159c9f733df13467e7b7e93 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:38:35 +0100 Subject: [PATCH 05/17] archives: a dropped .fsa file is read from the File, not fetched from 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 --- .../player/utils/FastStreamArchiveUtils.mjs | 27 ++++---- chrome/player/utils/RequestUtils.mjs | 62 ------------------- tests/unit/RequestUtils.test.mjs | 2 +- 3 files changed, 16 insertions(+), 75 deletions(-) diff --git a/chrome/player/utils/FastStreamArchiveUtils.mjs b/chrome/player/utils/FastStreamArchiveUtils.mjs index 9795f03e..b6bf17a8 100644 --- a/chrome/player/utils/FastStreamArchiveUtils.mjs +++ b/chrome/player/utils/FastStreamArchiveUtils.mjs @@ -1,7 +1,7 @@ // @ts-check import {DownloadStatus} from '../enums/DownloadStatus.mjs'; import {DownloadEntry} from '../network/DownloadEntry.mjs'; -import {RequestUtils} from './RequestUtils.mjs'; +import {LargeBuffer} from '../modules/LargeBuffer.mjs'; import {Utils} from './Utils.mjs'; import {withoutCredentials} from '../VideoSource.mjs'; @@ -131,24 +131,27 @@ export class FastStreamArchiveUtils { } /** - * Reads an archive from a local file, such as one dropped on the player. - * - * httpGetLarge fetches the file in chunks as parseFSA reads it, so the object URL has to - * live until parseFSA is done. Revoked any earlier, every chunk past the first two failed, - * and any archive over 500 MB could not be opened. + * Reads an archive from a local file, such as one dropped on the player: a chunk at a time + * (fileChunkSize), each read from the file as parseFSA gets to it, the next one ahead. + * Until 2026-11-15 the chunks were fetched from an object URL of the file with Range + * requests (RequestUtils.httpGetLarge, its only use), which held the URL for the whole read. * @param {File|Blob} file - The .fsa file. * @param {(share: number) => void} [progressCallback] - Called with the share of entries read. * @param {import('../network/DownloadManager.mjs').DownloadManager} [downloadManager] - Receives the entries' data. * @return {Promise} What parseFSA returns. */ static async parseFSAFile(file, progressCallback, downloadManager) { - const url = URL.createObjectURL(file); - try { - const buffer = await RequestUtils.httpGetLarge(url, this.fileChunkSize); - return await this.parseFSA(buffer, progressCallback, downloadManager); - } finally { - URL.revokeObjectURL(url); + if (!file.size) { + throw new Error('The archive file is empty'); } + const chunkSize = this.fileChunkSize; + const chunkCount = Math.ceil(file.size / chunkSize); + const buffer = new LargeBuffer(file.size, chunkCount); + await buffer.initialize(async (i) => { + const start = i * chunkSize; + return new Uint8Array(await file.slice(start, Math.min(start + chunkSize, file.size)).arrayBuffer()); + }); + return await this.parseFSA(buffer, progressCallback, downloadManager); } /** diff --git a/chrome/player/utils/RequestUtils.mjs b/chrome/player/utils/RequestUtils.mjs index d9783534..15e43694 100644 --- a/chrome/player/utils/RequestUtils.mjs +++ b/chrome/player/utils/RequestUtils.mjs @@ -1,8 +1,6 @@ // @ts-check import {MessageTypes} from '../enums/MessageTypes.mjs'; -import {LargeBuffer} from '../modules/LargeBuffer.mjs'; import {EnvUtils} from './EnvUtils.mjs'; -import {URLUtils} from './URLUtils.mjs'; /** @typedef {Object} QueryParams */ @@ -256,64 +254,4 @@ export class RequestUtils { } return xhr; } - - /** - * Downloads a large file in fragments using range requests and returns a LargeBuffer. - * The fragments are fetched as the buffer is read, not up front, so `source` has to stay - * reachable until the last byte has been read. - * @param {string} source - The URL of the large file. - * @param {number} [fragSize] - Bytes per range request. - * @return {Promise} Resolves with a LargeBuffer containing the file data. - * @throws {Error} If the request fails or headers are missing. - */ - static async httpGetLarge(source, fragSize = 1e9 / 4) { - const headersXHR = await this.request({ - url: source, - responseType: 'arraybuffer', - range: { - start: 0, - end: 1, - }, - }); - - if (headersXHR.status !== 200 && headersXHR.status !== 206) { - throw new Error('Bad status code'); - } - - const headers = /** @type {Object} */ (URLUtils.headersStringToObj(headersXHR.getAllResponseHeaders())); - const range = headers['content-range']; - if (!range) { - throw new Error('No content-range header'); - } - - const s = range.split('/'); - const contentLength = parseInt(s[1]); - if (!contentLength) { - throw new Error('No content length'); - } - - const fragCount = Math.ceil(contentLength / fragSize); - const buffer = new LargeBuffer(contentLength, fragCount); - await buffer.initialize(/** @type {(i: number) => Promise>} */ (async (i) => { - if (i >= fragCount) { - throw new Error('Fragment index ' + i + ' out of range'); - } - const start = i * fragSize; - const end = Math.min(i * fragSize + fragSize - 1, contentLength - 1); - const xhr = await this.request({ - url: source, - responseType: 'arraybuffer', - range: { - start, - end, - }, - }); - if (xhr.status !== 200 && xhr.status !== 206) { - throw new Error('Bad status code'); - } - return new Uint8Array(xhr.response); - })); - - return buffer; - } } diff --git a/tests/unit/RequestUtils.test.mjs b/tests/unit/RequestUtils.test.mjs index 8d6b895c..dfc71543 100644 --- a/tests/unit/RequestUtils.test.mjs +++ b/tests/unit/RequestUtils.test.mjs @@ -24,7 +24,7 @@ describe('RequestUtils.request', () => { expect([...new Uint8Array(result.response)]).toEqual([1, 2, 3]); expect(result.getResponseHeader('content-range')).toBe('bytes 0-2/10'); expect(result.getResponseHeader('X-None')).toBe(null); - // httpGetLarge parses the whole list as XMLHttpRequest gave it. + // The whole list, as XMLHttpRequest gave it. expect(URLUtils.headersStringToObj(result.getAllResponseHeaders())['content-range']).toBe('bytes 0-2/10'); }); From 9aac44ed942b2d37e1f31903bf324f29a4aa57c9 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:38:42 +0100 Subject: [PATCH 06/17] css: no transition or animation left under the zero-motion override; 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 --- chrome/_locales/de/messages.json | 3 - chrome/_locales/en/messages.json | 3 - chrome/_locales/es/messages.json | 3 - chrome/_locales/fr/messages.json | 3 - chrome/_locales/id/messages.json | 3 - chrome/_locales/it/messages.json | 3 - chrome/_locales/ja/messages.json | 3 - chrome/_locales/ko/messages.json | 3 - chrome/_locales/ms/messages.json | 3 - chrome/_locales/nl/messages.json | 3 - chrome/_locales/pl/messages.json | 3 - chrome/_locales/pt_BR/messages.json | 3 - chrome/_locales/ru/messages.json | 3 - chrome/_locales/tr/messages.json | 3 - chrome/_locales/zh_CN/messages.json | 3 - chrome/_locales/zh_TW/messages.json | 3 - .../assets/fluidplayer/css/fluidplayer.css | 127 +----------------- .../options/defaults/VisChangeActions.mjs | 1 - chrome/player/options/options.css | 24 ---- combined-locales.json | 18 --- tests/unit/playerA11y.test.mjs | 9 +- 21 files changed, 13 insertions(+), 214 deletions(-) diff --git a/chrome/_locales/de/messages.json b/chrome/_locales/de/messages.json index ea99bfd2..c0ada10e 100644 --- a/chrome/_locales/de/messages.json +++ b/chrome/_locales/de/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "OLED-Schwarz" }, - "options_general_vischangeaction_pip": { - "message": "Bild-in-Bild umschalten" - }, "options_export_header": { "message": "Import/Export der Einstellungen" }, diff --git a/chrome/_locales/en/messages.json b/chrome/_locales/en/messages.json index 5cc4b81c..b793e4b1 100644 --- a/chrome/_locales/en/messages.json +++ b/chrome/_locales/en/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "OLED Black" }, - "options_general_vischangeaction_pip": { - "message": "Toggle picture-in-picture" - }, "options_export_header": { "message": "Import/Export Settings" }, diff --git a/chrome/_locales/es/messages.json b/chrome/_locales/es/messages.json index 01eb4107..7853bbae 100644 --- a/chrome/_locales/es/messages.json +++ b/chrome/_locales/es/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "Negro OLED" }, - "options_general_vischangeaction_pip": { - "message": "Alternar imagen en imagen" - }, "options_export_header": { "message": "Importar/Exportar Configuraciones" }, diff --git a/chrome/_locales/fr/messages.json b/chrome/_locales/fr/messages.json index 054f7846..d442bdd7 100644 --- a/chrome/_locales/fr/messages.json +++ b/chrome/_locales/fr/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "Noir OLED" }, - "options_general_vischangeaction_pip": { - "message": "Basculer l'image dans l'image" - }, "options_export_header": { "message": "Importer/Exporter les paramètres" }, diff --git a/chrome/_locales/id/messages.json b/chrome/_locales/id/messages.json index b98a6359..163e60da 100644 --- a/chrome/_locales/id/messages.json +++ b/chrome/_locales/id/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "Hitam OLED" }, - "options_general_vischangeaction_pip": { - "message": "Beralih gambar-dalam-gambar" - }, "options_export_header": { "message": "Impor/Ekspor Pengaturan" }, diff --git a/chrome/_locales/it/messages.json b/chrome/_locales/it/messages.json index 42e65109..111b937d 100644 --- a/chrome/_locales/it/messages.json +++ b/chrome/_locales/it/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "Nero OLED" }, - "options_general_vischangeaction_pip": { - "message": "Attiva/disattiva il picture-in-picture" - }, "options_export_header": { "message": "Importa/Esporta le Impostazioni" }, diff --git a/chrome/_locales/ja/messages.json b/chrome/_locales/ja/messages.json index 672cff0e..5bc3186b 100644 --- a/chrome/_locales/ja/messages.json +++ b/chrome/_locales/ja/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "OLEDブラック" }, - "options_general_vischangeaction_pip": { - "message": "ピクチャー イン ピクチャー (PiP) を切り替え" - }, "options_export_header": { "message": "設定のインポート/エクスポート" }, diff --git a/chrome/_locales/ko/messages.json b/chrome/_locales/ko/messages.json index 5ded207f..4092f302 100644 --- a/chrome/_locales/ko/messages.json +++ b/chrome/_locales/ko/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "OLED 블랙" }, - "options_general_vischangeaction_pip": { - "message": "화면 속 화면 전환" - }, "options_export_header": { "message": "설정 가져오기/내보내기" }, diff --git a/chrome/_locales/ms/messages.json b/chrome/_locales/ms/messages.json index bd2284f3..b4ecc71a 100644 --- a/chrome/_locales/ms/messages.json +++ b/chrome/_locales/ms/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "Hitam OLED" }, - "options_general_vischangeaction_pip": { - "message": "Togol gambar dalam gambar" - }, "options_export_header": { "message": "Import/Eksport Tetapan" }, diff --git a/chrome/_locales/nl/messages.json b/chrome/_locales/nl/messages.json index 60293f1f..2ac9417f 100644 --- a/chrome/_locales/nl/messages.json +++ b/chrome/_locales/nl/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "OLED-zwart" }, - "options_general_vischangeaction_pip": { - "message": "Beeld-in-beeld in-/uitschakelen" - }, "options_export_header": { "message": "Instellingen importeren/exporteren" }, diff --git a/chrome/_locales/pl/messages.json b/chrome/_locales/pl/messages.json index 56967917..e12c604d 100644 --- a/chrome/_locales/pl/messages.json +++ b/chrome/_locales/pl/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "Czerń OLED" }, - "options_general_vischangeaction_pip": { - "message": "Przełącz obraz w obrazie" - }, "options_export_header": { "message": "Import/Eksport ustawień" }, diff --git a/chrome/_locales/pt_BR/messages.json b/chrome/_locales/pt_BR/messages.json index 2768c124..69fda029 100644 --- a/chrome/_locales/pt_BR/messages.json +++ b/chrome/_locales/pt_BR/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "Preto OLED" }, - "options_general_vischangeaction_pip": { - "message": "Alternar imagem sobreposta (pip)" - }, "options_export_header": { "message": "Configurações de importação/exportação" }, diff --git a/chrome/_locales/ru/messages.json b/chrome/_locales/ru/messages.json index 95dc400f..abc896c1 100644 --- a/chrome/_locales/ru/messages.json +++ b/chrome/_locales/ru/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "Чёрный OLED" }, - "options_general_vischangeaction_pip": { - "message": "Картинка в картинке" - }, "options_export_header": { "message": "Настройки импорта/экспорта" }, diff --git a/chrome/_locales/tr/messages.json b/chrome/_locales/tr/messages.json index dbf13008..a0f10d92 100644 --- a/chrome/_locales/tr/messages.json +++ b/chrome/_locales/tr/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "OLED Siyah" }, - "options_general_vischangeaction_pip": { - "message": "Resim içinde resim aç/kapa" - }, "options_export_header": { "message": "Ayarları İçe/Dışa Aktar" }, diff --git a/chrome/_locales/zh_CN/messages.json b/chrome/_locales/zh_CN/messages.json index f66055cd..8569ef5c 100644 --- a/chrome/_locales/zh_CN/messages.json +++ b/chrome/_locales/zh_CN/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "OLED 纯黑" }, - "options_general_vischangeaction_pip": { - "message": "切换画中画" - }, "options_export_header": { "message": "导入/导出设置" }, diff --git a/chrome/_locales/zh_TW/messages.json b/chrome/_locales/zh_TW/messages.json index a23b6f72..87b4abdb 100644 --- a/chrome/_locales/zh_TW/messages.json +++ b/chrome/_locales/zh_TW/messages.json @@ -959,9 +959,6 @@ "options_general_color_theme_oled": { "message": "OLED 純黑" }, - "options_general_vischangeaction_pip": { - "message": "切換畫中畫" - }, "options_export_header": { "message": "匯入/匯出設定" }, diff --git a/chrome/player/assets/fluidplayer/css/fluidplayer.css b/chrome/player/assets/fluidplayer/css/fluidplayer.css index 535651b9..190dae07 100644 --- a/chrome/player/assets/fluidplayer/css/fluidplayer.css +++ b/chrome/player/assets/fluidplayer/css/fluidplayer.css @@ -182,8 +182,7 @@ body { display: inline-block; } -.fluid_html_on_pause, -.fluid_pseudo_poster { +.fluid_html_on_pause { position: absolute; top: 0; right: 0; @@ -253,7 +252,6 @@ body { background: var(--controls-background); height: 53px; z-index: 3; - transition: all 0.5s !important; } .fluid_controls_bottom_container { @@ -278,7 +276,6 @@ body { z-index: 1; text-align: center; max-width: 80%; - transition: bottom 0.5s; } .controls_visible.expanded .fluid_subtitles_container { @@ -301,7 +298,6 @@ body { pointer-events: none; z-index: 2; opacity: 0; - transition: opacity 0.3s; } .osd.visible { @@ -325,12 +321,10 @@ body { .fluid_video_wrapper.fluid_player_layout_default .fade_out { opacity: 0; - transition: visibility 0.5s, opacity 0.5s; } .fluid_video_wrapper.fluid_player_layout_default .fade_in { opacity: 1; - transition: visibility 0.5s, opacity 0.5s; } .fluid_video_wrapper.fluid_player_layout_default.pseudo_fullscreen { @@ -417,8 +411,7 @@ body { height: 5px; } -.fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container .fluid_controls_progress_container.freeze .fluid_controls_progress, -.fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container .fluid_controls_progress_container.freeze .fluid_controls_buffered { +.fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container .fluid_controls_progress_container.freeze .fluid_controls_progress { margin-top: -1px; height: 5px; } @@ -439,15 +432,10 @@ body { opacity: 1; } -.fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container .fluid_controls_progress_container.fluid_slider .fluid_controls_progress .fluid_controls_currentprogress .fluid_controls_currentpos { - transition: opacity 0.3s; -} - .fluid_control_volume_container { width: 0px; z-index: 2; opacity: 0.8; - transition: width 0.2s ease-in-out, visibility 0.2s; display: inline-block; visibility: hidden; } @@ -458,10 +446,6 @@ body { visibility: visible; } -.fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container .fluid_control_volume_container .fluid_control_volume .fluid_control_currentvolume .fluid_control_volume_currentpos { - transition: 0.2s ease-in-out; -} - .fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container div:not(.reordering)>.volume_block:focus .fluid_control_volume_container .fluid_control_volume .fluid_control_currentvolume .fluid_control_volume_currentpos, .fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container div:not(.reordering)>.volume_block:hover .fluid_control_volume_container .fluid_control_volume .fluid_control_currentvolume .fluid_control_volume_currentpos { width: 11px; @@ -526,10 +510,6 @@ body { border-radius: 50%; } -.fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container.no_volume_bar .fluid_control_volume_container { - display: none; -} - .fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container .fluid_button, .fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container .fluid_slider { cursor: pointer; @@ -597,7 +577,6 @@ body { border-top: none; } - .fluid_button_language .fluid_language_list .language_table { display: table; } @@ -655,7 +634,6 @@ body { width: 100%; height: 100%; opacity: 0.85; - transition: opacity 0.2s; display: inline-block; } @@ -770,7 +748,6 @@ body { height: 12px; border-radius: 6px; background-color: color-mix(in srgb, var(--menu-text-color) 35%, transparent); - transition: background-color 0.2s; } .fluid_autoplay_switch.on .fluid_autoplay_track { @@ -790,7 +767,6 @@ body { background-color: var(--menu-text-color); color: var(--menu-background-color); box-shadow: 0 1px 3px rgba(0, 0, 0, 0.5); - transition: transform 0.2s; } .fluid_autoplay_switch.on .fluid_autoplay_knob { @@ -832,18 +808,11 @@ body { .fluid_button_video_source .fluid_video_sources_list, .fluid_button_soundwave .fluid_audio_sources_list, -.fluid_button_language .fluid_language_list, -.fluid_video_playback_rates { +.fluid_button_language .fluid_language_list { z-index: 888888 !important; opacity: 0.9 !important; } -.fluid_video_playback_rates_item { - padding: 9px 25px 9px 25px; - line-height: 15px; - text-align: center; -} - .video-container { position: absolute; inset: 0px; @@ -939,7 +908,6 @@ body { .fluid_controls_progress_container .seek_preview .seek_preview_video video { height: 100px; - transition: opacity 0.15s; } .progress_loaded_container { @@ -998,7 +966,6 @@ body { display: none; } - .fluid_button_video_source .fluid_video_sources_list, .fluid_button_soundwave .fluid_audio_sources_list, .fluid_button_language .fluid_language_list { @@ -1116,8 +1083,6 @@ body { visibility: visible; } - - .buffering_spinner { display: inline-block; vertical-align: top; @@ -1143,7 +1108,6 @@ body { opacity: 0; width: 0px; height: 0px; - transition: bottom 0.5s; overflow: hidden; } @@ -1167,7 +1131,6 @@ body { padding: 4px 10px; font-size: 18px; cursor: pointer; - transition: bottom 0.5s; } /* VpnPrompt.mjs: above everything at the top, where a failed load's message leaves room. */ @@ -1212,7 +1175,6 @@ body { padding: 4px 0; font-size: 18px; cursor: pointer; - transition: bottom 0.5s; } .next_video_cancel:hover, @@ -1382,7 +1344,6 @@ body { text-align: center; } - .window_header .close_button { right: 5px; top: 50%; @@ -1402,7 +1363,6 @@ body { background-color: var(--popwindow-background-color); color: var(--popwindow-text-color); z-index: 2; - transition: bottom 0.5s; cursor: auto; outline: none; } @@ -1572,7 +1532,6 @@ body { color: var(--popwindow-button-text-color); } - .linkui-addnew-button { left: 20px; right: auto; @@ -1674,7 +1633,6 @@ body { left: 13px; right: 13px; overflow: hidden; - transition: visibility 0.5s; visibility: hidden; } @@ -1749,18 +1707,15 @@ body { height: 54px; left: 0px; right: 0px; - transition: height 0.2s, top 0.2s; overflow: hidden; } - .timeline_audio_canvas_container { position: absolute; top: 0px; left: 0px; right: 0px; height: 100%; - transition: height 0.2s, top 0.2s; } .threshold_bar { @@ -1981,7 +1936,6 @@ body { .subtitle-shiftl-tool, .subtitle-shiftr-tool { opacity: 0.85; - transition: opacity 0.2s; } .subtitle-resync-tool, @@ -2091,7 +2045,6 @@ body { background-color: var(--menu-list-item-active-background-color); } - .subtitle-menu-option { padding: 8px 5px; color: var(--menu-text-color); @@ -2116,12 +2069,10 @@ body { z-index: 2; } - .tooltip { position: relative; } - .tooltip .tooltiptext { visibility: hidden; background-color: var(--tooltip-background-color); @@ -2177,7 +2128,6 @@ body { right: 5px; } - .tooltip .tooltiptext.down { bottom: auto; top: 100%; @@ -2191,7 +2141,6 @@ body { border-color: transparent transparent var(--tooltip-background-color) transparent; } - div:not(.reordering)>.volume_block:focus .tooltip .tooltiptext { visibility: visible; } @@ -2230,82 +2179,20 @@ div:not(.reordering)>.volume_block:focus .tooltip .tooltiptext { opacity: 0.5; } -/* Make shake */ -.reordering>div>svg, -.reordering>div.volume_block>div>svg, -.reordering>.fluid_autoplay_switch>.fluid_autoplay_track { - animation: tilt-shaking 0.25s infinite; -} - -.reordering>.fluid_control_duration { - animation: tilt-shaking-small 0.25s infinite; -} - -.reordering>div.sortable-chosen>svg, -.reordering>div.volume_block.sortable-chosen>div>svg, -.reordering>.fluid_control_duration.sortable-chosen { - animation: none; -} - /* Everything at once, for everyone (his wish, 2026-10-13: "I do not want delays or animations"): the controls and menus appear and disappear at once, and the buttons being rearranged do not shake. It was only for those who asked their system for less motion; for everyone else the controls faded in over half a second, nearly invisible at first, on - a dark video above all. The controls bar needs its own selector to outrank its - `transition: all 0.5s !important`. The loading spinner is an animated SVG image, which - this leaves turning. */ -.fluid_video_wrapper.fluid_player_layout_default *, -.fluid_video_wrapper.fluid_player_layout_default .fluid_controls_container { + a dark video above all. The transitions and the shaking this made no-ops were taken out on + 2026-11-15; it stays, for any added later and for the libraries' own styles. The loading + spinner is an animated SVG image, which this leaves turning. */ +.fluid_video_wrapper.fluid_player_layout_default * { transition-duration: 0s !important; animation-duration: 0s !important; transition-delay: 0s !important; animation-delay: 0s !important; } -@keyframes tilt-shaking { - 0% { - transform: rotate(0deg); - } - - 25% { - transform: rotate(5deg); - } - - 50% { - transform: rotate(0deg); - } - - 75% { - transform: rotate(-5deg); - } - - 100% { - transform: rotate(0deg); - } -} - -@keyframes tilt-shaking-small { - 0% { - transform: rotate(0deg); - } - - 25% { - transform: rotate(1deg); - } - - 50% { - transform: rotate(0deg); - } - - 75% { - transform: rotate(-1deg); - } - - 100% { - transform: rotate(0deg); - } -} - .sortable-ghost { opacity: 0 !important; } diff --git a/chrome/player/options/defaults/VisChangeActions.mjs b/chrome/player/options/defaults/VisChangeActions.mjs index d818e15c..17c35927 100644 --- a/chrome/player/options/defaults/VisChangeActions.mjs +++ b/chrome/player/options/defaults/VisChangeActions.mjs @@ -6,7 +6,6 @@ export const VisChangeActions = { NOTHING: 'nothing', PLAY_PAUSE: 'playpause', MINI_PLAYER: 'miniplayer', - // PIP: 'pip', }; if (!EnvUtils.isExtension()) { diff --git a/chrome/player/options/options.css b/chrome/player/options/options.css index bbb5ab38..b65673d6 100644 --- a/chrome/player/options/options.css +++ b/chrome/player/options/options.css @@ -135,11 +135,6 @@ body.frame button:hover { } /* Primary submit button inside frame */ -body.frame #submitter { - background-color: var(--popwindow-button-active-background-color); - color: var(--popwindow-button-text-color); -} - /* Link styling when embedded in player */ body.frame a, body.frame a:visited { @@ -198,25 +193,6 @@ small, } /* Legacy submitter button kept for compatibility */ -#submitter { - background-color: var(--color-primary); - color: var(--color-primary-contrast); - float: right; - width: 120px; - margin: var(--space-3) 0; - border-radius: var(--radius-1); - text-align: center; - padding: var(--space-2) var(--space-3); - cursor: pointer; - font-size: 1rem; - font-weight: 700; - border: none; -} - -#submitter:hover { - filter: brightness(0.95); -} - h2 { clear: both; } diff --git a/combined-locales.json b/combined-locales.json index fb4886ae..b1b6ec65 100644 --- a/combined-locales.json +++ b/combined-locales.json @@ -5759,24 +5759,6 @@ "zh_CN": "OLED 纯黑", "zh_TW": "OLED 純黑" }, - "options_general_vischangeaction_pip": { - "en": "Toggle picture-in-picture", - "de": "Bild-in-Bild umschalten", - "es": "Alternar imagen en imagen", - "fr": "Basculer l'image dans l'image", - "id": "Beralih gambar-dalam-gambar", - "it": "Attiva/disattiva il picture-in-picture", - "ja": "ピクチャー イン ピクチャー (PiP) を切り替え", - "ko": "화면 속 화면 전환", - "ms": "Togol gambar dalam gambar", - "nl": "Beeld-in-beeld in-/uitschakelen", - "pl": "Przełącz obraz w obrazie", - "pt_BR": "Alternar imagem sobreposta (pip)", - "ru": "Картинка в картинке", - "tr": "Resim içinde resim aç/kapa", - "zh_CN": "切换画中画", - "zh_TW": "切換畫中畫" - }, "options_export_header": { "en": "Import/Export Settings", "de": "Import/Export der Einstellungen", diff --git a/tests/unit/playerA11y.test.mjs b/tests/unit/playerA11y.test.mjs index e702e9d5..0ef98553 100644 --- a/tests/unit/playerA11y.test.mjs +++ b/tests/unit/playerA11y.test.mjs @@ -297,10 +297,13 @@ describe('the extension\'s pages', () => { // less motion; for everyone else the controls faded in over half a second. const css = fs.readFileSync(new URL('../../chrome/player/assets/fluidplayer/css/fluidplayer.css', import.meta.url), 'utf8'); expect(css).not.toContain('prefers-reduced-motion'); - // At the top level, and with the controls bar's own selector: its `transition: all 0.5s - // !important` needs one as specific. - const still = /\n\.fluid_video_wrapper\.fluid_player_layout_default \*,\n\.fluid_video_wrapper\.fluid_player_layout_default \.fluid_controls_container \{([^}]*)\}/.exec(css); + // At the top level, for everything in the player: a net for anything added later. + const still = /\n\.fluid_video_wrapper\.fluid_player_layout_default \* \{([^}]*)\}/.exec(css); expect(still).not.toBe(null); expect(still[1]).toMatch(/transition-duration: 0s !important;\s*animation-duration: 0s !important;/); + // And no motion declared at all (2026-11-15: the declarations the net made no-ops are gone). + const declared = css.replace(/\/\*[\s\S]*?\*\//g, '').match(/^\s*(transition|animation)\s*:[^;]*;/gm) || []; + expect(declared.filter((line) => !/:\s*none\s*;/.test(line))).toEqual([]); + expect(css).not.toMatch(/@keyframes/); }); }); From 6c03422d7fe15a0e03c10ee39d0fdd69fd8013aa Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:38:48 +0100 Subject: [PATCH 07/17] tools: same-code.mjs, whether a change to the code is comments and types only node tools/same-code.mjs [path ...] parses each changed .js/.mjs file (untracked new ones too) as it was at 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 --- docs/notes/type-checking.md | 6 +- tests/unit/typeChecked.test.mjs | 1 + tools/same-code.mjs | 102 ++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 tools/same-code.mjs diff --git a/docs/notes/type-checking.md b/docs/notes/type-checking.md index acdabac5..18b7f580 100644 --- a/docs/notes/type-checking.md +++ b/docs/notes/type-checking.md @@ -166,8 +166,10 @@ errors under `strict`, the worklet 8), and an editor showed them in a project of `strict` or the extension's types. `include` is now `chrome/**/*.mjs` and `chrome/**/*.js`, the libraries' folder still excluded; a script in that folder that nothing imports goes into `files` (the HLS decrypter's worker, the worklet). `types/audio-worklet.d.ts` declares the worklet's two -globals. The three files were typed with comments only (`handoff`'s same-code check: the minified -code is the same). `typeChecked.test.mjs` asks tsc for its file list (`--listFilesOnly`) and fails +globals. The three files were typed with comments only: `node tools/same-code.mjs ` parses +each changed script as it was and as it is, and finds the same program (positions, comments, the +parentheses a JSDoc cast needs and a literal's quotes aside); it names each file whose code +differs, with the line. `typeChecked.test.mjs` asks tsc for its file list (`--listFilesOnly`) and fails on an opted-in script that is not in it; with the old `include` it names the three. An error an editor shows on a `tsconfig.json` of a worktree that no longer exists ("File ... diff --git a/tests/unit/typeChecked.test.mjs b/tests/unit/typeChecked.test.mjs index 7145b23e..f1ade858 100644 --- a/tests/unit/typeChecked.test.mjs +++ b/tests/unit/typeChecked.test.mjs @@ -226,6 +226,7 @@ const CHECKED = [ 'tools/privacy-check.mjs', 'tools/rebuild.mjs', 'tools/recut-patch.mjs', + 'tools/same-code.mjs', 'tools/security-overrides.mjs', 'tools/setup-dev-profile.mjs', 'tools/sign-amo.mjs', diff --git a/tools/same-code.mjs b/tools/same-code.mjs new file mode 100644 index 00000000..389c3a16 --- /dev/null +++ b/tools/same-code.mjs @@ -0,0 +1,102 @@ +// @ts-check +// Whether a change is comments only - JSDoc types, casts, notes - as a typing pass should be. +// Each .js/.mjs file that differs from is parsed as it was there and as it is now, and +// the two programs are compared without positions, comments, parentheses (a JSDoc cast needs +// them, acorn drops them) and a literal's raw text (its quotes). Lists every file whose code +// differs, with the line where it first does; exits 1 when one does. +// +// node tools/same-code.mjs [path ...] (paths default to chrome) +// +// Run from the repository's root. A file that is new (untracked ones too), or gone, counts as +// differing. + +import {execFileSync} from 'node:child_process'; +import fs from 'node:fs'; +import {parse} from './ast-compare.mjs'; + +const [base, ...paths] = process.argv.slice(2); +if (!base) { + console.error('usage: node tools/same-code.mjs [path ...]'); + process.exit(2); +} + +/** @param {string[]} args */ +const git = (...args) => execFileSync('git', args, { + encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'ignore'], +}); + +/** + * The program without what carries no behaviour. + * @param {string} src + * @return {any} + */ +function program(src) { + return JSON.parse(JSON.stringify(parse(src), (key, value) => { + if (key === 'start' || key === 'end' || key === 'raw') return undefined; + return typeof value === 'bigint' ? `${value}n` : value; + })); +} + +/** + * The first node of `b` that differs from `a`, with its start in the source. + * @param {any} a + * @param {any} b + * @param {any} rawB the same node of `b` before program() dropped its positions + * @return {number | null} the start of the differing node, -1 when no position is known + */ +function firstDifference(a, b, rawB) { + if (a === b) return null; + if (!a || !b || typeof a !== 'object' || typeof b !== 'object' || Array.isArray(a) !== Array.isArray(b)) { + return typeof rawB?.start === 'number' ? rawB.start : -1; + } + const keys = new Set([...Object.keys(a), ...Object.keys(b)]); + if (Array.isArray(a) && a.length !== b.length) keys.add('length'); + for (const key of keys) { + if (key === 'length' && Array.isArray(a)) { + if (a.length !== b.length) { + const at = b[Math.min(a.length, b.length)] ?? b[b.length - 1]; + return typeof at?.start === 'number' ? at.start : rawStart(rawB); + } + continue; + } + const found = firstDifference(a[key], b[key], rawB?.[key]); + if (found !== null) return found === -1 ? rawStart(rawB) : found; + } + return null; +} + +/** @param {any} node */ +const rawStart = (node) => (typeof node?.start === 'number' ? node.start : -1); + +/** @param {string} src @param {number} at */ +const lineOf = (src, at) => (at < 0 ? '?' : String(src.slice(0, at).split('\n').length)); + +const pathspec = paths.length ? paths : ['chrome']; +const changed = [...new Set([ + ...git('diff', '--name-only', '--diff-filter=ACMRD', base, '--', ...pathspec).split('\n'), + ...git('ls-files', '--others', '--exclude-standard', '--', ...pathspec).split('\n'), +])].filter((file) => /\.m?js$/.test(file)); + +let differ = 0; +for (const file of changed) { + let before = null; + try { + before = git('show', `${base}:${file}`); + } catch { + // new since + } + const after = fs.existsSync(file) ? fs.readFileSync(file, 'utf8') : null; + if (before === null || after === null) { + console.log(`${before === null ? 'NEW ' : 'GONE'} ${file}`); + differ++; + continue; + } + const rawAfter = parse(after); + const at = firstDifference(program(before), program(after), rawAfter); + if (at !== null) { + differ++; + console.log(`CODE ${file}:${lineOf(after, at)}`); + } +} +console.log(`${changed.length} changed file(s): ${changed.length - differ} with the same code, ${differ} differ`); +process.exitCode = differ ? 1 : 0; From 098e9ef20815b623a397cc21afea91d3e6b67cf7 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:43:10 +0100 Subject: [PATCH 08/17] docs: the polish round of 2026-11-15, with its A/B numbers and what was 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 --- docs/notes/player.md | 77 +++++++++++++++++++++++++++++++++++++++ docs/review-2026-11-14.md | 52 ++++++++++++++++++-------- 2 files changed, 113 insertions(+), 16 deletions(-) diff --git a/docs/notes/player.md b/docs/notes/player.md index 5e6ecd64..3b8fc533 100644 --- a/docs/notes/player.md +++ b/docs/notes/player.md @@ -1642,3 +1642,80 @@ Looked at and left: - Loading `sortable.mjs`, the subtitle search and `gif.mjs` only when first used (176 KB of 1.4 MB the player loads at its start): not measured and not built. Each is a way for a menu to open late, and what it could save is the parsing of an eighth of the player's code. + +## The polish round (2026-11-15) + +What the review of 2026-11-14 left open, done or measured on main, before the review round of +2026-11-17. + +- **An MP4 start that gets nowhere no longer runs its loop every millisecond.** 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 it took, and for good when the element could not decode the + data. It runs so only for 2 s after the start last got somewhere - an append (`updateend`), a + seek, a play - and then at the steady 50 ms; `loadeddata` and each append wake it at once. + `LoopPace.mjs` holds the choice (`LoopPace.test.mjs`). A/B against main + (`tools/bench-ab.mjs --scenarios mp4`, 7 rounds), with the files local and on a 12 Mbit/s + line with 150 ms to the first byte: every measure "same" (first frame 562/561 ms local, + 887/879 ms slow; seek to 80 s and playing past it 349/350 and 1839/1851 ms; wake-ups while + playing, paused and ended 25.0/9.5/9.4 a second on both). The change is for a start that + never gets its first frame, which the benchmark does not play. +- **A fragmented MP4's keyframe index takes in only the new samples.** `refreshSampleIndex` + filtered and sorted every sample of every track again after each range: 7.7 ms a range on a + 2-hour file (Node, synthetic samples: 216,000 video, 338,000 audio, every audio sample a + keyframe), on the player's thread. `addKeyframes` (`SampleIndex.mjs`) sorts in only what + mp4box added since: appended when it all comes later (a file played on: about nothing), + merged in otherwise (an earlier part read after a seek back: 0.75 ms). mp4box builds a + track's list once, at the moov, and only appends to it, so the result is the full sort's, + ties included (`SampleIndex.test.mjs`, a property test). `setFragmentTimes`, run with it, + stays linear in the samples (a binary search per range, then the range's samples). +- **One fragment status for the three requesters.** DASH's, HLS's and MP4's fragment requesters + kept a fragment's status in three copies of the same 40 lines; `players/FragmentStatus.mjs` + holds it (`FragmentStatus.test.mjs`). DASH's `preProcessFragment`/`postProcessFragment` + hooks, which no player had, are gone from `DashFragmentRequester` and `DashLoader`. +- **A dropped `.fsa` archive is read from the File.** `parseFSAFile` fetched the file's own + object URL in Range requests (`RequestUtils.httpGetLarge`, its only use, now gone) and read + the length back from `Content-Range`; it reads `file.slice()` chunks now, the next one + ahead as before. +- **PC-4, measured and not built.** A body read in 64 KiB pieces and joined at the end holds + its bytes twice at the peak; one buffer the size of the `Content-Length`, written in place, + was to hold them once. Measured in Node with a 64 MB body: the peak was 128 MB as it is and + 123.8 MB written in place, the time the same. The pieces are freed only at the next garbage + collection, which comes after the copy either way, so the body is in memory twice all the + same. +- **CS-7, measured and left.** The content script walks the page and its shadow roots on a + click or key (at most every 2 s, in the frame that got it). In the page's own realm the walk + takes under 1 ms for 10,000 elements, 2 ms for 50,000 and 15 ms for 200,000; from the + content script (through Xray wrappers) 3, 17 and 72 ms. Asking only for open-or-closed shadow + roots, or a live `getElementsByTagName` list, changes nothing (4/18/80 and 3/17/69 ms): the + cost is the iteration. A walk spread over idle time could miss a play that comes right + after the click, which is what the walk is for. +- **Zero motion, in the stylesheet itself.** `fluidplayer.css` declared 20 transitions and 2 + animations (and the "shaking" keyframes) that the 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 `options.css`'s `#submitter`. + `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 the 16 locales. +- **SEC-3**: `patched-libraries.yml` in two jobs (`docs/notes/workflows.md`, "Patched libraries: + two jobs"). +- **R-1** was done already: `update-prs.yml`'s concurrency is per job. + +Looked at and left: + +- `SaveFile.add` copies a view of a fragment (`slice()`) before handing it to the OPFS worker: + the worker takes the buffer (transferred), and the fragment's buffer holds more than the + mdat. Writing a view in the worker would spare one copy of each fragment, a memcpy of a few + MB, for a change to the worker's protocol. +- The progress bar reads every fragment of the level per fragment event (about two a + fragment) and writes the DOM only where something changed. +- Skipping a `saveTabState` write whose state is the one written last: most writes come with + an address change, and the address is part of the state. +- The settings sliders save once a frame while dragged (OP-6) - not at every input event, as + the review of 2026-11-14 had it. +- `content.js` keeps a copy of `SubtitleUtils.decodeSubtitleBytes` and `MpvTarget.mjs` builds + what the mpv helper built until host 14, byte for byte: a classic script cannot import a + module, and the helper is a program of its own. Each copy has a test that keeps it the same + (`decodeSubtitleBytes.test.mjs`, `MpvTarget.test.mjs`). +- The site scripts in `chrome/custom/` (bilibili, Facebook, Instagram) are still in the + manifest. Retiring them once the general detection is shown to play those sites (the owner's + rule of 2026-10-28) needs those sites tested, signed in: for the owner. diff --git a/docs/review-2026-11-14.md b/docs/review-2026-11-14.md index 8bade372..9b4d7004 100644 --- a/docs/review-2026-11-14.md +++ b/docs/review-2026-11-14.md @@ -38,6 +38,15 @@ The detailed account is elsewhere and is not repeated here: To list it: `git log --oneline 4813fbee..^2 --first-parent` (the merges and the commits of 2026-11-14), `git diff 4813fbee -- ` per area. +**After #679, on main** (the polish round of 2026-11-15, one pull request from +`polish/handoff`; `docs/notes/player.md`, "The polish round", says why and what was measured): +`patched-libraries.yml` in two jobs (SEC-3); the MP4 start that gets nowhere no longer loops +every millisecond; a fragmented MP4's keyframe index takes in only the new samples; one +fragment status for the DASH, HLS and MP4 requesters, and DASH's dead hooks out; a dropped +`.fsa` archive read from the File (`RequestUtils.httpGetLarge` gone); the stylesheets without +the motion the global override cancelled, and unused rules and a locale key out; +`tools/same-code.mjs`. Review it as its own diff: `git diff 6321c71c `. + ## 2. What was run, and what came out All on the branch's last code commits (the documents changed after), on a Windows 11 PC with @@ -166,9 +175,11 @@ Security - Not fixed, the owner's decision: PE-7, a crafted MP4 that claims billions of samples makes mp4box allocate one object per sample on the add-ons' shared process (4 million samples, 2.2 GB, measured). Recommended: a per-file sample budget, beyond it Firefox plays the file itself. -- Hardening recommended, not built (report section 8): a tag ruleset; `patched-libraries.yml` - installs a new npm release in a job with a write token; the mpv helper starts any `mpv*.exe` - path and passes `fs-*` tags unchecked; the fail-safe attaches AMO's signed file without +- Built on 2026-11-15 (SEC-3): `patched-libraries.yml` installs a new npm release only in a job + with a read-only token; a second job, which installs nothing, checks what the first hands on and + pushes (`docs/notes/workflows.md`, "Patched libraries: two jobs"). +- Hardening recommended, not built (report section 8): a tag ruleset; the mpv helper starts any + `mpv*.exe` path and passes `fs-*` tags unchecked; the fail-safe attaches AMO's signed file without comparing it with the built zip; `fsaunpack` answers `Access-Control-Allow-Origin: *`; the signing job also runs the tests. @@ -189,7 +200,10 @@ Unverified, worth a reviewer's look The owner's open decisions are in the report's section 8, PE-7 first; six were added on 2026-11-14 (the next episode through a page in between, the silence skipper against a held speed, the volume keys after a mouse click, the toast's countdown bar, remembering a host's -connection verdict, more than six connections on HTTP/2 and 3). +connection verdict, more than six connections on HTTP/2 and 3). One more on 2026-11-15: the +site scripts in `chrome/custom/` (bilibili, Facebook, Instagram; two of them read the page's +address every second for as long as it is open) are still installed; retiring them, by the +owner's rule of 2026-10-28, needs those sites tested while signed in. ## 5. Where to look next @@ -227,21 +241,27 @@ difference exists; the CI runs Windows and Linux. **Refactors and rewrites, with what each would buy.** - `MP4Player.refreshSampleIndex` is quadratic on fragmented files (10 ms a call at 2 h). -- `MP4Player`'s steady loop runs every 50 ms while playing (could be 250 ms at `readyState` 3); - its `needsInit` wait loops every millisecond (could wake on `updateend` and `loadedmetadata`). -- PC-4: a download with a `Content-Length`, no encoding and no cut could be one buffer written - in place instead of a list of chunks joined at the end. +- `MP4Player`'s steady loop runs every 50 ms while playing (could be 250 ms at `readyState` 3). + Its `needsInit` wait looped every millisecond for good; since 2026-11-15 only for 2 s after + the last step forward, then every 50 ms, woken at once by `loadeddata` and the duration + (`docs/notes/player.md`, "The polish round"). +- PC-4 (one buffer written in place for a download with a `Content-Length`) was measured on + 2026-11-15 and not built: the peak stays the same, as the pieces are freed only at the next + collection (same section). - `SaveFile.add` copies each fragment once more than it needs (`subarray` + `slice`). - Startup: `sortable.mjs` (119 KB), the subtitle search (43 KB) and `gif.mjs` (14 KB) load with the player and could load at first use (not measured). -- The UI: the status line is rescanned and rewritten per fragment event; a paused video's - controls are checked every 2 s; the settings sliders save on every input event, which makes - every player and every settings frame apply the options again. -- The background: `deleteHeaderCache` could join the first `onHeadersReceived` listener; the tab - state is written at every address change of every tab (could skip equal JSON); the tab icon is - updated on every `onUpdated`. -- CSS: `fluidplayer.css` has 16 transition declarations and a keyframes block under the global - zero-motion override, and five selectors nothing uses; `options.css` one. +- The UI: the progress bar reads every fragment per fragment event (it writes the DOM only + where something changed; about two events a fragment: looked at on 2026-11-15 and left); a + paused video's controls are checked every 2 s. The settings sliders save once a frame while + dragged (OP-6), not at every input event. +- The background: `deleteHeaderCache` could join the first `onHeadersReceived` listener. The + tab state is written at every address change of every tab; skipping a write of equal state + would save nothing there, as the address is part of it (looked at on 2026-11-15). The tab + icon is set on every `onUpdated` of an address or a load, as Firefox resets the button on + every new document. +- CSS: done on 2026-11-15. The transitions, animations and keyframes under the zero-motion + override, and the selectors nothing uses, are gone (`playerA11y.test.mjs` keeps them out). ## 6. How to review From 2cae48f5faac439a0bc5caacf86c45aaf84979d0 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:47:51 +0100 Subject: [PATCH 09/17] tools: same-code points a removed statement at the right line; tests 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 --- tests/unit/sameCode.test.mjs | 112 +++++++++++++++++++++++++++++++++++ tools/same-code.mjs | 26 ++++---- 2 files changed, 125 insertions(+), 13 deletions(-) create mode 100644 tests/unit/sameCode.test.mjs diff --git a/tests/unit/sameCode.test.mjs b/tests/unit/sameCode.test.mjs new file mode 100644 index 00000000..196be872 --- /dev/null +++ b/tests/unit/sameCode.test.mjs @@ -0,0 +1,112 @@ +import {execFileSync} from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import {afterEach, describe, expect, it} from 'vitest'; + +// tools/same-code.mjs answers whether a typing pass changed only comments. A false "same" lets +// a code change through under that name; a false "differs" makes the check useless. + +describe('same-code in a clone', () => { + const script = path.resolve('tools/same-code.mjs'); + /** @type {string[]} */ + const dirs = []; + afterEach(() => { + for (const dir of dirs.splice(0)) fs.rmSync(dir, {recursive: true, force: true}); + }); + + /** A repository whose one commit holds chrome/a.mjs and chrome/b.js. */ + const clone = () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'same-code-')); + dirs.push(dir); + const git = (...args) => execFileSync('git', args, {cwd: dir, encoding: 'utf8'}); + git('init', '-q'); + git('config', 'user.name', 'test'); + git('config', 'user.email', 'test@example.com'); + git('config', 'core.autocrlf', 'false'); + fs.mkdirSync(path.join(dir, 'chrome')); + fs.writeFileSync(path.join(dir, 'chrome', 'a.mjs'), 'export function f(x) {\n return x + 1;\n}\n'); + fs.writeFileSync(path.join(dir, 'chrome', 'b.js'), 'const s = "text";\n'); + git('add', '.'); + git('commit', '-q', '-m', 'base'); + return dir; + }; + const write = (dir, file, text) => fs.writeFileSync(path.join(dir, file), text); + const run = (dir, ...args) => { + try { + return {code: 0, out: execFileSync(process.execPath, [script, 'HEAD', ...args], {cwd: dir, encoding: 'utf8', stdio: 'pipe'})}; + } catch (error) { + return {code: error.status, out: String(error.stdout)}; + } + }; + + it('calls comments, JSDoc casts and quote style the same code', () => { + const dir = clone(); + write(dir, 'chrome/a.mjs', [ + '// @ts-check', + '/** @param {number} x @return {number} */', + 'export function f(x) {', + ' return /** @type {number} */ (x) + 1; // one more', + '}', + '', + ].join('\n')); + write(dir, 'chrome/b.js', 'const s = \'text\';\n'); + const result = run(dir); + expect(result).toEqual({code: 0, out: '2 changed file(s): 2 with the same code, 0 differ\n'}); + }); + + it('names the line where the code first differs', () => { + const dir = clone(); + write(dir, 'chrome/a.mjs', '// note\nexport function f(x) {\n return x + 2;\n}\n'); + const result = run(dir); + expect(result.code).toBe(1); + expect(result.out).toContain('CODE chrome/a.mjs:3'); + expect(result.out).toContain('1 changed file(s): 0 with the same code, 1 differ'); + }); + + it('notices a statement added at the end, and a changed string', () => { + const dir = clone(); + write(dir, 'chrome/a.mjs', 'export function f(x) {\n return x + 1;\n}\nf(1);\n'); + write(dir, 'chrome/b.js', 'const s = "text ";\n'); + const result = run(dir); + expect(result.code).toBe(1); + expect(result.out).toContain('CODE chrome/a.mjs:4'); + expect(result.out).toContain('CODE chrome/b.js:1'); + }); + + it('points a removed statement at the last one left', () => { + const dir = clone(); + write(dir, 'chrome/b.js', 'const a = 1;\nconst b = 2;\nconst c = 3;\n'); + execFileSync('git', ['commit', '-q', '-am', 'three'], {cwd: dir}); + write(dir, 'chrome/b.js', 'const a = 1;\nconst b = 2;\n'); + const result = run(dir); + expect(result.code).toBe(1); + expect(result.out).toContain('CODE chrome/b.js:2'); + }); + + it('counts new (untracked too) and deleted files as differing, and keeps to the paths given', () => { + const dir = clone(); + write(dir, 'chrome/new.mjs', 'export const n = 1;\n'); + fs.rmSync(path.join(dir, 'chrome', 'b.js')); + fs.mkdirSync(path.join(dir, 'tools')); + write(dir, 'tools/other.mjs', 'export const o = 1;\n'); + const result = run(dir); + expect(result.code).toBe(1); + expect(result.out).toContain('NEW chrome/new.mjs'); + expect(result.out).toContain('GONE chrome/b.js'); + expect(result.out).not.toContain('tools/other.mjs'); + expect(run(dir, 'tools').out).toContain('NEW tools/other.mjs'); + }); + + it('wants a base commit', () => { + const dir = clone(); + let status = 0; + try { + execFileSync(process.execPath, [script], {cwd: dir, stdio: 'pipe'}); + } catch (error) { + status = error.status; + } + expect(status).toBe(2); + }); +}); diff --git a/tools/same-code.mjs b/tools/same-code.mjs index 389c3a16..4a326376 100644 --- a/tools/same-code.mjs +++ b/tools/same-code.mjs @@ -37,6 +37,9 @@ function program(src) { })); } +/** @param {any} node */ +const rawStart = (node) => (typeof node?.start === 'number' ? node.start : -1); + /** * The first node of `b` that differs from `a`, with its start in the source. * @param {any} a @@ -47,27 +50,24 @@ function program(src) { function firstDifference(a, b, rawB) { if (a === b) return null; if (!a || !b || typeof a !== 'object' || typeof b !== 'object' || Array.isArray(a) !== Array.isArray(b)) { - return typeof rawB?.start === 'number' ? rawB.start : -1; + return rawStart(rawB); } - const keys = new Set([...Object.keys(a), ...Object.keys(b)]); - if (Array.isArray(a) && a.length !== b.length) keys.add('length'); + const lists = Array.isArray(a); + // A list is compared over the part both have, before a missing element is looked at: that + // has no position of its own. + const keys = lists ? Array.from({length: Math.min(a.length, b.length)}, (_, i) => i) : + new Set([...Object.keys(a), ...Object.keys(b)]); for (const key of keys) { - if (key === 'length' && Array.isArray(a)) { - if (a.length !== b.length) { - const at = b[Math.min(a.length, b.length)] ?? b[b.length - 1]; - return typeof at?.start === 'number' ? at.start : rawStart(rawB); - } - continue; - } const found = firstDifference(a[key], b[key], rawB?.[key]); if (found !== null) return found === -1 ? rawStart(rawB) : found; } + if (lists && a.length !== b.length) { + // the first element added, or the last one left when some were removed + return rawStart(rawB?.[a.length < b.length ? a.length : b.length - 1]); + } return null; } -/** @param {any} node */ -const rawStart = (node) => (typeof node?.start === 'number' ? node.start : -1); - /** @param {string} src @param {number} at */ const lineOf = (src, at) => (at < 0 ? '?' : String(src.slice(0, at).split('\n').length)); From 5b75aa3e6688e2ef54c4d2a38460a844ce382d72 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:56:00 +0100 Subject: [PATCH 10/17] Fixes from the review of the branch's diff - 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 --- .github/workflows/patched-libraries.yml | 32 ++++++++++----- chrome/player/players/dash/DashLoader.mjs | 4 +- tests/unit/DashLoader.test.mjs | 18 +++++++++ tests/unit/playerA11y.test.mjs | 36 ++++++++++++++++- tests/unit/sameCode.test.mjs | 21 ++++++++++ .../patched-libraries-publish.test.sh | 40 ++++++++++++++++--- tools/same-code.mjs | 10 +++-- 7 files changed, 138 insertions(+), 23 deletions(-) diff --git a/.github/workflows/patched-libraries.yml b/.github/workflows/patched-libraries.yml index b2703808..b9bac547 100644 --- a/.github/workflows/patched-libraries.yml +++ b/.github/workflows/patched-libraries.yml @@ -25,12 +25,14 @@ name: Patched libraries # # Two jobs, so that no code from a new library release runs where the write token is: "recut" -# installs the release (its install scripts are its authors' code) and re-cuts the patch, with -# a read-only token, and hands each result on as an artifact - the change as a git patch, or the -# report of why it stopped. "publish" runs on a runner of its own with the write token, installs -# nothing, and pushes, opens and closes with git, gh and jq alone (SEC-3 of the review of -# 2026-11-13: one job did both, and a release's install script could read the token from its -# step's environment, or plant something in the job's later steps through $GITHUB_ENV). +# installs the release and re-cuts the patch, with a read-only token, and hands each result on +# as an artifact - the change as a git patch, or the report of why it stopped. pnpm runs the +# install scripts of the packages allowBuilds lists only, but a release can depend on one of +# those in a new version. "publish" runs on a runner of its own with the write token, installs +# nothing, checks what it was handed, and pushes, opens and closes with git, gh and jq alone +# (SEC-3 of the review of 2026-11-13: one job did both, and an install script could read the +# token from its step's environment, or plant something in the job's later steps through +# $GITHUB_ENV). recut keeps no cache, which would carry what it installed to main's workflows. on: schedule: # Daily, 06:30 UTC. @@ -60,8 +62,9 @@ jobs: # A dash.js re-cut lints a 3 MB bundle four times; the rest takes seconds. timeout-minutes: 30 permissions: - contents: read # the repository - issues: read # the titles raised before (the issues endpoint lists pull requests too) + contents: read # the repository + issues: read # the titles raised before; the issues endpoint lists pull requests + pull-requests: read # too, but only to a token that may read them outputs: count: ${{ steps.recut.outputs.count }} steps: @@ -73,11 +76,12 @@ jobs: - uses: pnpm/action-setup@73e46992e05b19abd31f2f3c20e0e7370d762115 # v6.1.1 + # No pnpm cache: its post step would save the store a new release was installed into, + # under main's key, for CI and every other workflow on main to restore. - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 with: node-version-file: .nvmrc check-latest: true - cache: pnpm # tools/recut-patch.mjs lints with the project's ESLint, and --apply installs. - name: Install dependencies @@ -289,6 +293,10 @@ jobs: if ! [[ $name =~ ^(@[a-z0-9._-]+/)?[a-zA-Z0-9._-]+$ && $latest =~ ^[0-9A-Za-z.+-]+$ && $current =~ ^[0-9A-Za-z.+-]+$ ]]; then echo "Not a library and version: '$name' '$current' '$latest'."; exit 1 fi + # The link in the pull request or issue: a release on npm or GitHub, nothing else. + if ! [[ $url =~ ^https://(www\.npmjs\.com|github\.com)/[A-Za-z0-9._~%@+/-]+$ ]]; then + echo "Not a release page on npm or GitHub: '$url'."; exit 1 + fi title="$PREFIX$name $latest" cp "$dir/report.txt" "$RUNNER_TEMP/report.txt" @@ -296,8 +304,12 @@ jobs: branch="patched/$name-$latest" git checkout -q -f -B "$branch" "$start" git am -q "$dir/change.patch" - stray=$(git diff --name-only "$start" HEAD | + # --no-renames: a rename is listed as its new name only, and would carry a file + # off the list away unseen. Each file stays a plain one (a deleted one: 000000): + # no link, no submodule, no executable. + stray=$(git diff --no-renames --name-only "$start" HEAD | grep -vxE 'package\.json|pnpm-lock\.yaml|pnpm-workspace\.yaml|patches/[^/]+\.patch|tools/sync-vendor\.mjs|\.github/dependabot\.yml' || true) + stray+=$(git diff --no-renames --raw "$start" HEAD | awk '$2 != "100644" && $2 != "000000"') if [ -n "$stray" ]; then echo "The re-cut of $name $latest changed more than a re-cut may:"; echo "$stray"; exit 1 fi diff --git a/chrome/player/players/dash/DashLoader.mjs b/chrome/player/players/dash/DashLoader.mjs index 549c1868..7bd228bd 100644 --- a/chrome/player/players/dash/DashLoader.mjs +++ b/chrome/player/players/dash/DashLoader.mjs @@ -254,7 +254,9 @@ export function DASHLoaderFactory(player) { if (player.loadedManifests.has(key)) downloadManager.forgetCompletedFile(context); player.loadedManifests.add(key); } - const loader = downloadManager.getFile(context, { + // No watcher of a DASH download reads its pieces (onChunk): a postProcessor, even one that + // changes nothing, keeps the entry from holding a second copy of them while it loads. + const loader = downloadManager.getFile({...context, postProcessor: async (entry, response) => response}, { /** * @param {DownloadEntry} entry * @param {null} [xhr] - FetchLoader has none; none at all for a stored copy. diff --git a/tests/unit/DashLoader.test.mjs b/tests/unit/DashLoader.test.mjs index f241e684..7fd1c8cc 100644 --- a/tests/unit/DashLoader.test.mjs +++ b/tests/unit/DashLoader.test.mjs @@ -2,6 +2,8 @@ import fs from 'node:fs'; import {afterEach, describe, expect, it, vi} from 'vitest'; import {DASHLoaderFactory as dashLoaderFactory} from '../../chrome/player/players/dash/DashLoader.mjs'; import {DashFragment} from '../../chrome/player/players/dash/DashFragment.mjs'; +import {DownloadEntry} from '../../chrome/player/network/DownloadEntry.mjs'; +import {DownloadStatus} from '../../chrome/player/enums/DownloadStatus.mjs'; // DashLoader hands dash.js's requests to FastStream's download manager, passing each URL // through decodeURI (since the dash.js 5 upgrade). decodeURI throws on a `%` that is not @@ -70,6 +72,22 @@ describe('DashLoader URLs', () => { }); }); +describe('DashLoader downloads', () => { + // Nothing reads a DASH download's pieces as they come (onChunk): an entry with no + // postProcessor keeps a copy of them, for a watcher that might join on the way, until the + // download is over - a second copy of every segment while it loads (2026-11-15: the + // pre/post-processing hooks went, and with them the postProcessor that had kept it from it). + it('keeps no pieces of what it loads, and passes the answer on as it came', async () => { + const details = loadManifest('http://127.0.0.1/a.mpd'); + const entry = new DownloadEntry(details); + entry.status = DownloadStatus.DOWNLOAD_INITIATED; + entry.onChunk(new Uint8Array([1, 2]), 0, {status: 200}); + expect(entry.pieces).toBe(null); + const response = {url: details.url, status: 200, headers: {}, data: 'mpd'}; + expect(await details.postProcessor(entry, response)).toBe(response); + }); +}); + describe('DashLoader and DashFragment, one segment through both paths', () => { // dash.js can ask for a segment before the store has it: the "Fragment not found" // fallback downloads it, keyed by the URL it decoded, while every later request for it diff --git a/tests/unit/playerA11y.test.mjs b/tests/unit/playerA11y.test.mjs index 0ef98553..6816b7b2 100644 --- a/tests/unit/playerA11y.test.mjs +++ b/tests/unit/playerA11y.test.mjs @@ -42,6 +42,24 @@ const {AudioQualityChanger} = await import('../../chrome/player/ui/menus/AudioQu const {VideoQualityChanger} = await import('../../chrome/player/ui/menus/VideoQualityChanger.mjs'); const {SubtitlesManager} = await import('../../chrome/player/ui/subtitles/SubtitlesManager.mjs'); +/** + * The declarations in a style sheet that could make something move, written however: a + * transition or an animation, or its name or properties, other than none, or a duration + * other than 0. Its delay, timing and the like move nothing alone. + * @param {string} css + * @return {string[]} each as written, without its semicolon + */ +function movingDeclarations(css) { + const declarations = css.replace(/\/\*[\s\S]*?\*\//g, '') + .matchAll(/(? { + const v = value.replace(/!important/, '').trim(); + if (property.endsWith('-duration')) return !/^0s?$/.test(v); + if (['transition', 'animation', 'transition-property', 'animation-name'].includes(property)) return v !== 'none'; + return false; + }).map(([declaration]) => declaration.trim()); +} + describe('a control the keyboard went to', () => { /** * A control set up as the player's are; :focus-visible as Firefox has it (keyboard focus, not a click's). @@ -302,8 +320,22 @@ describe('the extension\'s pages', () => { expect(still).not.toBe(null); expect(still[1]).toMatch(/transition-duration: 0s !important;\s*animation-duration: 0s !important;/); // And no motion declared at all (2026-11-15: the declarations the net made no-ops are gone). - const declared = css.replace(/\/\*[\s\S]*?\*\//g, '').match(/^\s*(transition|animation)\s*:[^;]*;/gm) || []; - expect(declared.filter((line) => !/:\s*none\s*;/.test(line))).toEqual([]); + expect(movingDeclarations(css)).toEqual([]); expect(css).not.toMatch(/@keyframes/); }); + + it('finds motion however it is written', () => { + expect(movingDeclarations('.a { transition: opacity .5s; }\n.b{animation:spin 1s}')).toEqual([ + 'transition: opacity .5s', 'animation:spin 1s', + ]); + expect(movingDeclarations('.a {\n transition-property: opacity; transition-duration: .5s;\n animation-name: x;\n}')).toEqual([ + 'transition-property: opacity', 'transition-duration: .5s', 'animation-name: x', + ]); + // Still: none, a duration of 0, the parts that move nothing alone, a class named so, a comment. + expect(movingDeclarations([ + '.a { transition: none; animation-duration: 0s !important; transition-duration: 0;', + ' animation-delay: 1s; animation-timing-function: ease; }', + '.fluid_transition:hover {} /* transition: all 1s; */', + ].join('\n'))).toEqual([]); + }); }); diff --git a/tests/unit/sameCode.test.mjs b/tests/unit/sameCode.test.mjs index 196be872..b7b9793b 100644 --- a/tests/unit/sameCode.test.mjs +++ b/tests/unit/sameCode.test.mjs @@ -75,6 +75,27 @@ describe('same-code in a clone', () => { expect(result.out).toContain('CODE chrome/b.js:1'); }); + it('sees a template\'s raw text change, which a tag gets', () => { + const dir = clone(); + write(dir, 'chrome/b.js', 'const s = String.raw`\\n`;\n'); + execFileSync('git', ['commit', '-q', '-am', 'tag'], {cwd: dir}); + write(dir, 'chrome/b.js', 'const s = String.raw`\\x0a`;\n'); + const result = run(dir); + expect(result.code).toBe(1); + expect(result.out).toContain('CODE chrome/b.js:1'); + }); + + it('reads a file whose name is not ASCII', () => { + const dir = clone(); + write(dir, 'chrome/é.mjs', 'export const e = 1;\n'); + execFileSync('git', ['add', '.'], {cwd: dir}); + execFileSync('git', ['commit', '-q', '-m', 'e'], {cwd: dir}); + write(dir, 'chrome/é.mjs', 'export const e = 2;\n'); + const result = run(dir); + expect(result.code).toBe(1); + expect(result.out).toContain('CODE chrome/é.mjs:1'); + }); + it('points a removed statement at the last one left', () => { const dir = clone(); write(dir, 'chrome/b.js', 'const a = 1;\nconst b = 2;\nconst c = 3;\n'); diff --git a/tests/workflows/patched-libraries-publish.test.sh b/tests/workflows/patched-libraries-publish.test.sh index 2469c95b..8b012288 100644 --- a/tests/workflows/patched-libraries-publish.test.sh +++ b/tests/workflows/patched-libraries-publish.test.sh @@ -55,7 +55,8 @@ make_repo() { ) } -# recut [files to change...]: what the re-cut job hands on. +# recut [change...]: what the re-cut job hands on. A change is a +# file to write, mv:: to move one as it is, or x: to write one executable. recut() { local n=$1 library=$2 outcome=$3 shift 3 @@ -68,7 +69,14 @@ recut() { ( cd "$here/repo" || exit 1 git checkout -q -b tmp - for file in "$@"; do echo "changed $n" > "$file"; git add -A -- "$file"; done + local change from to + for change in "$@"; do + case "$change" in + mv:*) IFS=: read -r _ from to <<< "$change"; git mv -- "$from" "$to" ;; + x:*) echo "changed $n" > "${change#x:}"; chmod +x "${change#x:}"; git add -- "${change#x:}" ;; + *) echo "changed $n" > "$change"; git add -A -- "$change" ;; + esac + done git commit -q -m "deps: re-cut $n" git format-patch -1 --stdout > "$dir/change.patch" git checkout -q main @@ -94,8 +102,8 @@ run() { pushed() { git --git-dir="$here/origin.git" rev-parse -q --verify "refs/heads/$1" > /dev/null; } not_pushed() { ! pushed "$1"; } -HLS='{"name":"hls.js","current":"1.7.4","latest":"1.7.5","url":"https://example.invalid/hls","behind":true}' -DASH='{"name":"dashjs","current":"5.3.0","latest":"5.4.0","url":"https://example.invalid/dash","behind":true}' +HLS='{"name":"hls.js","current":"1.7.4","latest":"1.7.5","url":"https://www.npmjs.com/package/hls.js/v/1.7.5","behind":true}' +DASH='{"name":"dashjs","current":"5.3.0","latest":"5.4.0","url":"https://github.com/Dash-Industry-Forum/dash.js/releases/tag/v5.4.0","behind":true}' reset recut 1 "$HLS" cut pnpm-lock.yaml patches/hls.js@1.7.4.patch @@ -118,13 +126,33 @@ check 'pushes nothing' not_pushed 'patched/hls.js-1.7.5' check 'opens nothing' lacks "$FIX/calls" 'create' reset -recut 1 '{"name":"hls.js;touch x","current":"1.7.4","latest":"1.7.5","url":"u"}' stopped +recut 1 "$HLS" cut pnpm-lock.yaml mv:.github/workflows/ci.yml:patches/x.patch +run 'a re-cut that moves a workflow into patches/ (a rename lists only its new name)' +check 'fails' test "$status" -ne 0 +check 'says what it changed' contains "$FIX/out" '.github/workflows/ci.yml' +check 'pushes nothing' not_pushed 'patched/hls.js-1.7.5' + +reset +recut 1 "$HLS" cut pnpm-lock.yaml x:patches/hls.js@1.7.4.patch +run 'a re-cut that makes a listed file executable' +check 'fails' test "$status" -ne 0 +check 'says which' contains "$FIX/out" 'patches/hls.js@1.7.4.patch' +check 'pushes nothing' not_pushed 'patched/hls.js-1.7.5' + +reset +recut 1 '{"name":"hls.js;touch x","current":"1.7.4","latest":"1.7.5","url":"https://www.npmjs.com/package/x"}' stopped run 'a library name that is no npm name' check 'fails' test "$status" -ne 0 check 'opens nothing' lacks "$FIX/calls" 'create' reset -recut 1 '{"name":"@scope/lib","current":"1.0.0","latest":"1.0.1-beta.2","url":"u"}' stopped +recut 1 '{"name":"hls.js","current":"1.7.4","latest":"1.7.5","url":"https://evil.example/hls.js"}' stopped +run 'a link to anywhere but npm or GitHub' +check 'fails' test "$status" -ne 0 +check 'opens nothing' lacks "$FIX/calls" 'create' + +reset +recut 1 '{"name":"@scope/lib","current":"1.0.0","latest":"1.0.1-beta.2","url":"https://www.npmjs.com/package/%40scope%2Flib/v/1.0.1-beta.2"}' stopped run 'a scoped name and a pre-release' check 'succeeds' test "$status" -eq 0 check 'opens the issue' contains "$FIX/calls" 'issue create --title Patched library update: @scope/lib 1.0.1-beta.2' diff --git a/tools/same-code.mjs b/tools/same-code.mjs index 4a326376..32405d90 100644 --- a/tools/same-code.mjs +++ b/tools/same-code.mjs @@ -20,8 +20,8 @@ if (!base) { process.exit(2); } -/** @param {string[]} args */ -const git = (...args) => execFileSync('git', args, { +/** @param {string[]} args - quotePath off: a name with other than ASCII in it comes as it is. */ +const git = (...args) => execFileSync('git', ['-c', 'core.quotePath=false', ...args], { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'ignore'], }); @@ -31,8 +31,10 @@ const git = (...args) => execFileSync('git', args, { * @return {any} */ function program(src) { - return JSON.parse(JSON.stringify(parse(src), (key, value) => { - if (key === 'start' || key === 'end' || key === 'raw') return undefined; + return JSON.parse(JSON.stringify(parse(src), /** @this {any} @param {string} key @param {any} value */ function(key, value) { + if (key === 'start' || key === 'end') return undefined; + // A literal's raw text is its quotes; a template's is what a tag gets (String.raw). + if (key === 'raw' && this.type === 'Literal') return undefined; return typeof value === 'bigint' ? `${value}n` : value; })); } From dfa33a8de3950013c1b522e51ebac658a40b30ae Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 01:56:36 +0100 Subject: [PATCH 11/17] docs: the review's fixes in the polish round's notes Co-Authored-By: Claude Opus 5.5 --- docs/notes/player.md | 11 ++++++++++- docs/notes/workflows.md | 27 ++++++++++++++++++--------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/docs/notes/player.md b/docs/notes/player.md index 3b8fc533..31438b66 100644 --- a/docs/notes/player.md +++ b/docs/notes/player.md @@ -1671,7 +1671,10 @@ What the review of 2026-11-14 left open, done or measured on main, before the re - **One fragment status for the three requesters.** DASH's, HLS's and MP4's fragment requesters kept a fragment's status in three copies of the same 40 lines; `players/FragmentStatus.mjs` holds it (`FragmentStatus.test.mjs`). DASH's `preProcessFragment`/`postProcessFragment` - hooks, which no player had, are gone from `DashFragmentRequester` and `DashLoader`. + hooks, which no player had, are gone from `DashFragmentRequester` and `DashLoader`. Their + postProcessor stays as one that changes nothing: a download with none keeps its pieces for a + watcher that might join on the way (`DownloadEntry.onChunk`), and nothing reads a DASH + download's, so it held a second copy of each while it loaded (`DashLoader.test.mjs`). - **A dropped `.fsa` archive is read from the File.** `parseFSAFile` fetched the file's own object URL in Range requests (`RequestUtils.httpGetLarge`, its only use, now gone) and read the length back from `Content-Range`; it reads `file.slice()` chunks now, the next one @@ -1699,6 +1702,12 @@ What the review of 2026-11-14 left open, done or measured on main, before the re - **SEC-3**: `patched-libraries.yml` in two jobs (`docs/notes/workflows.md`, "Patched libraries: two jobs"). - **R-1** was done already: `update-prs.yml`'s concurrency is per job. +- **A second review of the round's diff** (the Claude API, with the changed files): seven + claims, each checked against the code and each real - the rename, cache and pull-request read + of SEC-3 above, the DASH pieces above, two in `tools/same-code.mjs` (a template's raw text, a + name not in ASCII) and a motion check that saw shorthands only. All fixed, with a test each + that fails without its fix. Writing `same-code`'s own test found one more: a removed + statement was reported at the enclosing node's line. Looked at and left: diff --git a/docs/notes/workflows.md b/docs/notes/workflows.md index efe54388..fb1e3a30 100644 --- a/docs/notes/workflows.md +++ b/docs/notes/workflows.md @@ -767,22 +767,31 @@ `tests/unit/checkPatchedUpdates.test.mjs` fails if Dependabot's ignore list and `patchedDependencies` drift apart; `tests/unit/recutPatch.test.mjs` covers the tool. - **Patched libraries: two jobs** (2026-11-15, SEC-3 of the review of 2026-11-13). The re-cut - installs the new release, and its install scripts are its authors' code: in one job with the - write token they could read the token from the step's environment, or plant something in the - later steps through `$GITHUB_ENV`. Now `recut` (contents and issues read) installs, re-cuts and + installs the new release. pnpm runs install scripts only for the packages `allowBuilds` lists, + but a release can depend on a new version of one of those: in one job with the write token, + such a script could read the token from the step's environment, or plant something in the + later steps through `$GITHUB_ENV`. Now `recut` (contents, issues and pull requests read - the + issues endpoint lists pull requests only to a token that may read them; no pnpm cache, whose + post step would save the store the release went into under main's key) installs, re-cuts and hands each result on as the artifact `recuts` (one folder per library: `library.json`, `report.txt`, `outcome` - `cut` with `change.patch` from `git format-patch`, or `stopped`); `publish` (the write permissions) installs nothing, closes and supersedes as before, and publishes what it was handed in order. As that came from a job that ran foreign code, it is - checked first: the name and versions as npm has them (no shell or path characters), and the - change, applied with `git am` onto main, may touch `package.json`, `pnpm-lock.yaml`, - `pnpm-workspace.yaml`, `patches/*.patch`, `tools/sync-vendor.mjs` and `.github/dependabot.yml` - only; anything else stops the run before a push. A push to main skips `recut` and only closes. + checked first: the name and versions as npm has them (no shell or path characters), the link + a release page on npm or GitHub, and the change, applied with `git am` onto main, may touch + `package.json`, `pnpm-lock.yaml`, `pnpm-workspace.yaml`, `patches/*.patch`, + `tools/sync-vendor.mjs` and `.github/dependabot.yml` only, as plain files (no link, submodule + or executable bit); anything else stops the run before a push. The list is taken with + `--no-renames`: a rename shows only its new name, so a file off the list could be moved into + `patches/` unseen. A push to main skips `recut` and only closes. `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`: a clean re-cut becomes a branch, a pull request and two dispatches, a stopped one an issue; a change to a - workflow and a name with a `;` fail with nothing pushed or opened (each check, broken by hand, - turns it red). + workflow, a workflow moved into `patches/`, an executable patch, a name with a `;` and a link + elsewhere fail with nothing pushed or opened (each check, broken by hand, turns it red). + What stays: code that runs in `recut` runs on main's ref, and a runner's user may use sudo, so + it could take the job's cache token from the runner and write a cache entry other workflows + restore. Only a job that runs no foreign code at all would close that. - All of it was dry-run with a stub `gh` against the real upstream history (sync: adopted -> closed, push -> no rebuild, new release named, comment only on upstream movement; patched libraries: open once, no duplicates, close when caught up or unpatched). From eb9926745f3cc5c67bd577488705b98b26b4317b Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 02:09:14 +0100 Subject: [PATCH 12/17] player: fixes from a review of FastStreamClient - 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 --- chrome/player/FastStreamClient.mjs | 79 ++++++++++++----- tests/unit/FastStreamClient.test.mjs | 128 ++++++++++++++++++++++++++- 2 files changed, 182 insertions(+), 25 deletions(-) diff --git a/chrome/player/FastStreamClient.mjs b/chrome/player/FastStreamClient.mjs index 3ae1304c..e1cd3ea4 100644 --- a/chrome/player/FastStreamClient.mjs +++ b/chrome/player/FastStreamClient.mjs @@ -871,12 +871,15 @@ export class FastStreamClient extends EventEmitter { const ramOnly = EnvUtils.isIncognito() || this.options.bufferToDisk === false || diskGone; const ramRoom = ramOnly ? this.memoryShare().share * 8 * LOW : Infinity; + // Where the room does not decide it below, "Buffer ahead" does, on every tick: the fit in + // the RAM share at the end only lowers it, and one share that shrank (another player + // watched) held it there until the next video, as it held a "Buffer ahead" raised meanwhile. if (EnvUtils.isIncognito()) { + this.state.bufferAhead = this.options.bufferAhead; // Only for a predownload asked for: without it there is nothing to fit, and turned on // later, it warns then. if (this.hasDownloadSpace && this.options.downloadAll) { this.state.bufferBehind = this.options.bufferBehind; - this.state.bufferAhead = this.options.bufferAhead; // Without a limit on either, the whole video is buffered all the same: nothing to say. const windowSeconds = this.state.bufferBehind + this.state.bufferAhead; if (Number.isFinite(windowSeconds)) { @@ -889,6 +892,7 @@ export class FastStreamClient extends EventEmitter { // A live stream has no size to fit, and is freed behind playback as it plays. Its // infinite duration made it "too big" on its first tick: a storage warning on every // live stream, and everything downloaded so far pinned for the session. + this.state.bufferAhead = this.options.bufferAhead; this.hasDownloadSpace = false; } else if (bitrate && this.duration) { // With nothing on disk, the room is RAM: the disk's free space does not count. @@ -939,6 +943,7 @@ export class FastStreamClient extends EventEmitter { } this.hasDownloadSpace = newHasDownloadSpace || !this.options.downloadAll; } else { + this.state.bufferAhead = this.options.bufferAhead; this.hasDownloadSpace = true; } } @@ -1022,14 +1027,20 @@ export class FastStreamClient extends EventEmitter { isPreview: true, })); + // Stored with the preview only once it is the one shown: a build for a video since + // replaced, still running beside the next one's, took the field over, and the shown + // preview's context was never let go of. + /** @type {?ReturnType} */ + let context = null; try { await previewPlayer.setup(); - this.bindPreviewPlayer(previewPlayer); + context = this.bindPreviewPlayer(previewPlayer); await previewPlayer.setSource(source); } catch (e) { // Nothing else holds it: its video and its loader, which may go on fetching, lived on // for the page's life, one for each failed build (glm's review, 2026-10-13). + context?.destroy(); try { previewPlayer.destroy(); } catch (destroyError) { @@ -1043,10 +1054,12 @@ export class FastStreamClient extends EventEmitter { // in the page. Nor does one that was switched off while it was being built, which // setOptions() could not destroy because it was not stored yet. if (this.previewPlayer || !this.options.previewEnabled || this.player?.getSource() !== source) { + context.destroy(); previewPlayer.destroy(); return; } + this.previewContext = context; this.previewPlayer = previewPlayer; this.interfaceController.addPreviewVideo(previewPlayer.getVideo()); this.updateVideoTransform(); @@ -1353,7 +1366,15 @@ export class FastStreamClient extends EventEmitter { } catch (destroyError) { console.warn('Could not destroy a player that failed to set up', destroyError); } - if (this.player === built) this.player = null; + if (this.player === built) { + this.player = null; + // Its listeners and its wait for a picture, as resetPlayer lets go of them: after the + // load error no reset follows, and the wait polled once a second for good. + this.context?.destroy(); + this.context = null; + this.initPromise = null; + clearInterval(this.initHookInterval); + } } // Its player could not be built or given the source: that stream failed as surely // as one whose player reports an error, and the next of those picked from plays @@ -1755,11 +1776,15 @@ export class FastStreamClient extends EventEmitter { if (manager.activeCount() > 0) return true; const next = this.getNextToDownload(); if (!next) return false; - // The window predownloadFragments keeps to when the whole video is not wanted. - if (next.canFree() && !this.shouldDownloadAll() && - (next.start > this.state.currentTime + this.state.bufferAhead || - next.end < this.state.currentTime - this.state.bufferBehind)) { - return false; + // The window predownloadFragments keeps to when the whole video is not wanted, its + // fragments placed as it places them: one without times read as -1 counted as behind the + // playhead, and a background player said it wanted nothing it would have downloaded. + if (next.canFree() && !this.shouldDownloadAll()) { + const span = this.windowSpan(next, new Map()); + if (span && (span.start > this.state.currentTime + this.state.bufferAhead || + span.end < this.state.currentTime - this.state.bufferBehind)) { + return false; + } } return true; } @@ -2417,7 +2442,6 @@ export class FastStreamClient extends EventEmitter { saveManager?.resetMpvContentType(); const promises = []; - this.lastTime = 0; // The next video starts at 0. Until its first time update, the previous one's time // stayed on the progress bar, and the downloads were planned around it. this.state.currentTime = 0; @@ -2520,8 +2544,6 @@ export class FastStreamClient extends EventEmitter { this.storageAvailable = 0; /** @type {boolean|undefined} Whether the whole video fits in the room kept for it (updateHasDownloadSpace); none before the first reset. */ this.hasDownloadSpace = true; - this.previousLevel = -1; - this.previousAudioLevel = -1; this.getLevelManager().reset(); await Promise.all(promises); @@ -2818,35 +2840,44 @@ export class FastStreamClient extends EventEmitter { /** * Binds event listeners to the preview player. * @param {Player} player + * @return {ReturnType} + * Their context, for the build to store once the preview is the one shown. */ bindPreviewPlayer(player) { - this.previewContext = player.createContext(); + const context = player.createContext(); - this.previewContext.on(DefaultPlayerEvents.MANIFEST_PARSED, () => { + context.on(DefaultPlayerEvents.MANIFEST_PARSED, () => { player.setCurrentVideoLevelID(this.getCurrentVideoLevelID()); player.setCurrentAudioLevelID(this.getCurrentAudioLevelID()); }); - this.previewContext.on(DefaultPlayerEvents.FRAGMENT_UPDATE, (/** @type {Fragment} */ fragment) => { + context.on(DefaultPlayerEvents.FRAGMENT_UPDATE, (/** @type {Fragment} */ fragment) => { this.interfaceController.updateFragmentsLoaded(); }); - this.previewContext.on(DefaultPlayerEvents.SEEKED, (/** @type {undefined} */ event) => { + context.on(DefaultPlayerEvents.SEEKED, (/** @type {undefined} */ event) => { this.updatePreview(); }); - this.previewContext.on(DefaultPlayerEvents.SEEKING, (/** @type {undefined} */ event) => { + context.on(DefaultPlayerEvents.SEEKING, (/** @type {undefined} */ event) => { this.updatePreview(); }); - this.previewContext.on(DefaultPlayerEvents.ERROR, (/** @type {unknown} */ e) => { + context.on(DefaultPlayerEvents.ERROR, (/** @type {unknown} */ e) => { console.log('Preview player error', e); + // Another preview's: one built for a video since replaced, beside the one shown, which + // it destroyed. Its own build lets go of it. + if (this.previewPlayer && this.previewPlayer !== player) return; // Still null while the preview player is being built: it is only stored once its // source has loaded, and a source that fails to load is exactly what gets here. this.previewPlayer?.destroy(); this.previewPlayer = null; + if (this.previewContext === context) { + context.destroy(); + this.previewContext = null; + } this.interfaceController.resetPreviewVideo(); if (!this.interfaceController.failed) { @@ -2863,9 +2894,10 @@ export class FastStreamClient extends EventEmitter { this.previewReloads.push(now); this.lastPreviewReload = now; console.error('Reloading preview player'); - this.setupPreviewPlayer(); + this.setupPreviewPlayer().catch((error) => console.warn('The preview player could not be built again', error)); } }); + return context; } /** @@ -3595,16 +3627,15 @@ export class FastStreamClient extends EventEmitter { */ setVolume(volume) { this.state.volume = volume; - if (SET_VOLUME_USING_NODE || (volume > 1 && EnvUtils.isWebAudioSupported())) { + // Through the audio graph wherever there is one (above 100 % too); the element's own + // volume only without Web Audio, which cannot go above it. + if (SET_VOLUME_USING_NODE) { if (this.player && (!this.syncedAudioPlayer || !this.syncedAudioPlayer.setVolume(1))) { this.player.volume = 1; } this.audioOutput?.updateVolume(volume); - } else { - if (this.player && (!this.syncedAudioPlayer || !this.syncedAudioPlayer.setVolume(volume))) { - this.player.volume = volume; - } - if (EnvUtils.isWebAudioSupported()) this.audioOutput.updateVolume(1); + } else if (this.player && (!this.syncedAudioPlayer || !this.syncedAudioPlayer.setVolume(volume))) { + this.player.volume = volume; } } diff --git a/tests/unit/FastStreamClient.test.mjs b/tests/unit/FastStreamClient.test.mjs index 125e3d33..36b9d686 100644 --- a/tests/unit/FastStreamClient.test.mjs +++ b/tests/unit/FastStreamClient.test.mjs @@ -1142,6 +1142,39 @@ describe('FastStreamClient, the RAM budget with nothing on disk', () => { expect(client.hasDownloadSpace).toBe(false); expect(client.state.bufferAhead).toBe(fits(100 * MB)); }); + + it('gives a private window\'s buffer back when its share grows again, and takes a new "Buffer ahead"', () => { + // Fitted to a share that shrank while another player was watched, it stayed there for + // the rest of the video: the fit only ever lowered it. + vi.spyOn(EnvUtils, 'isIncognito').mockReturnValue(true); + const client = clientWithVideo({ramBudget: 2 * GB, bufferAhead: 1200}); + client.state.playing = true; + client.peers = {visible: () => true, livePeers: () => [{ramBytes: 1.9 * GB, visible: true, playing: true}]}; + client.updateHasDownloadSpace(); + const shared = client.state.bufferAhead; + expect(shared).toBe(fits(1 * GB)); + client.peers = {visible: () => true, livePeers: () => []}; + client.updateHasDownloadSpace(); + // Alone, all of "Buffer ahead" fits again. + expect(fits(2 * GB)).toBeGreaterThan(1200); + expect(client.state.bufferAhead).toBe(1200); + client.options.bufferAhead = 600; + client.updateHasDownloadSpace(); + expect(client.state.bufferAhead).toBe(600); + }); + + it('takes a new "Buffer ahead" during a live stream with nothing on disk', () => { + const client = clientWithVideo({bufferToDisk: false, ramBudget: 2 * GB}); + client.player.duration = Infinity; + client.updateHasDownloadSpace(); + expect(client.state.bufferAhead).toBe(300); + client.options.bufferAhead = 60; + client.updateHasDownloadSpace(); + expect(client.state.bufferAhead).toBe(60); + client.options.bufferAhead = 120; + client.updateHasDownloadSpace(); + expect(client.state.bufferAhead).toBe(120); + }); }); describe('FastStreamClient, limits of 0 and no limit', () => { @@ -1233,6 +1266,28 @@ describe('FastStreamClient, limits of 0 and no limit', () => { expect(predownloadOnce({maxSpeed: 125000, speed: 200000})).toBe(0); }); + it('wants the network for a fragment without times that it would download', () => { + // A fragmented MP4's ranges not parsed yet have times of -1. predownloadFragments places + // them (windowSpan); wantsNetwork read -1 as behind the playhead, and a player in a + // background tab told its peers it wanted nothing past 20 s in. + vi.stubGlobal('navigator', {onLine: true}); + const client = makeClient({maxSpeed: -1}); + const fragment = (sn, start, end) => ({level: 'video-1', sn, start, end, canFree: () => true}); + const fragments = [fragment(0, 0, 10), fragment(1, 10, 20), fragment(2, 20, 600), fragment(3, -1, -1)]; + client.player = {}; + client.getFragments = () => fragments; + client.getNextToDownload = () => fragments[3]; + client.needsUserInteraction = () => false; + client.isLive = () => false; + client.downloadManager = {downloaders: [], paused: false, memoryFull: false, activeCount: () => 0}; + client.state.currentTime = 35; + expect(client.wantsNetwork()).toBe(true); + // Placed far ahead of what it keeps ahead: not wanted, as predownloadFragments would not start it. + client.state.bufferAhead = 2; + client.state.currentTime = 1; + expect(client.wantsNetwork()).toBe(false); + }); + /** Whether the RAM budget counts this player full, holding this many bytes in RAM. */ function ramFull(ramBudget, held, toDisk = true) { const client = makeClient({ramBudget}); @@ -1560,6 +1615,57 @@ describe('FastStreamClient, the seek preview\'s player', () => { expect(preview.destroy).toHaveBeenCalledTimes(1); expect(client.previewPlayer).toBeNull(); }); + + /** A preview player: an emitter whose contexts can be counted. */ + const previewOf = (setSource) => Object.assign(new EventEmitter(), { + setup: async () => {}, setSource, destroy: vi.fn(), getVideo: () => ({}), + }); + + it('keeps its listeners only on a build that is shown', async () => { + // A build for a video replaced meanwhile, beside the next one's, took the context field + // over: the shown preview's was never let go of, and the stale one's stayed. + const client = makeClient({previewEnabled: true}); + let playing = {mode: 'hls', url: 'https://cdn.test/a.m3u8'}; + client.player = {getSource: () => playing}; + const stale = previewOf(async () => { + playing = {mode: 'hls', url: 'https://cdn.test/b.m3u8'}; + }); + client.playerLoader.createPlayer = vi.fn(async () => stale); + await client.buildPreviewPlayer(); + expect(stale.destroy).toHaveBeenCalledTimes(1); + expect(stale.contexts).toHaveLength(1); + expect(client.previewContext).toBeNull(); + + const shown = previewOf(async () => {}); + client.playerLoader.createPlayer = vi.fn(async () => shown); + client.interfaceController.addPreviewVideo = vi.fn(); + client.updateVideoTransform = vi.fn(); + await client.buildPreviewPlayer(); + expect(client.previewPlayer).toBe(shown); + expect(shown.contexts).toContain(client.previewContext); + }); + + it('lets the shown preview be when another one fails', () => { + const client = makeClient({previewEnabled: true}); + client.interfaceController.resetPreviewVideo = vi.fn(); + client.setupPreviewPlayer = vi.fn(async () => {}); + const shown = previewOf(async () => {}); + client.previewPlayer = shown; + client.previewContext = client.bindPreviewPlayer(shown); + const stale = previewOf(async () => {}); + client.bindPreviewPlayer(stale); + stale.emit(DefaultPlayerEvents.ERROR, new Error('the old video\'s level failed')); + expect(shown.destroy).not.toHaveBeenCalled(); + expect(client.previewPlayer).toBe(shown); + expect(client.setupPreviewPlayer).not.toHaveBeenCalled(); + + // Its own error lets go of it and of its listeners, and builds it again. + shown.emit(DefaultPlayerEvents.ERROR, new Error('segment expired')); + expect(shown.destroy).toHaveBeenCalledTimes(1); + expect(shown.contexts).toHaveLength(1); + expect(client.previewContext).toBeNull(); + expect(client.setupPreviewPlayer).toHaveBeenCalledTimes(1); + }); }); describe('FastStreamClient, a player that fails to set up', () => { @@ -1581,6 +1687,26 @@ describe('FastStreamClient, a player that fails to set up', () => { expect(client.player).toBeNull(); }); + it('leaves no listeners and no wait for a picture when it could not take the source', async () => { + vi.useFakeTimers(); + try { + const client = makeClient(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + const made = await setSource(client, makeSource('https://cdn.test/z.m3u8'), (player) => { + player.setSource = vi.fn(async () => { + throw new Error('no level'); + }); + }); + expect(client.player).toBeNull(); + expect(client.context).toBeNull(); + expect(made.contexts).toHaveLength(1); + expect(client.initPromise).toBeNull(); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + it('is not destroyed once it has taken the source', async () => { const client = makeClient(); const player = await setSource(client, makeSource('https://cdn.test/y.m3u8'), (made) => { @@ -1600,7 +1726,7 @@ describe('FastStreamClient, a seek preview that keeps failing', () => { const client = makeClient({previewEnabled: true}); client.interfaceController.resetPreviewVideo = vi.fn(); const context = new EventEmitter(); - client.setupPreviewPlayer = vi.fn(); + client.setupPreviewPlayer = vi.fn(async () => {}); client.bindPreviewPlayer({createContext: () => context}); return {client, context}; } From b2b83f58072b54c42493abfe48a126ff25c67b05 Mon Sep 17 00:00:00 2001 From: Nawid3333 <102778966+Nawid3333@users.noreply.github.com> Date: Sun, 15 Nov 2026 02:17:16 +0100 Subject: [PATCH 13/17] content: fixes from a review of content.js - 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 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 --- chrome/content.js | 124 +++++++++++++------ tests/unit/contentDom.mjs | 62 +++++++--- tests/unit/contentEpisodeNavigation.test.mjs | 17 +++ tests/unit/contentPagePlayer.test.mjs | 45 +++++++ tests/unit/contentScript.test.mjs | 108 ++++++++++++++++ 5 files changed, 300 insertions(+), 56 deletions(-) diff --git a/chrome/content.js b/chrome/content.js index a7a1f98b..e1fe2655 100644 --- a/chrome/content.js +++ b/chrome/content.js @@ -291,10 +291,7 @@ iframeMap.set(request.frameId, newFrameObj); // Watched while it is in the page too: taken out, it gives back what it took // (checkPendingPlayers). - if (!pendingPlayersWatch) { - pendingPlayersWatch = new MutationObserver(checkPendingPlayers); - pendingPlayersWatch.observe(document.documentElement, {childList: true, subtree: true}); - } + watchRemovalOf(iframeElement); checkPendingPlayers(); updateReplacedPlayers(replacedData && replacedData.convertDue ? replacedData : null); @@ -321,9 +318,32 @@ */ function watchPendingPlayer(iframe, attempt) { pendingPlayers.set(iframe, attempt); + watchRemovalOf(iframe); + } + + /** @type {WeakSet} The shadow roots pendingPlayersWatch observes besides the page. */ + let watchedRoots = new WeakSet(); + + /** + * Has pendingPlayersWatch see a player iframe go: the page, and each shadow root it is in + * (a player put where the page's video was, in a web component): an observer of the page + * sees nothing of what changes inside one, and a component that re-rendered its own tree + * took the player out unseen. + * @param {Element} iframe + */ + function watchRemovalOf(iframe) { if (!pendingPlayersWatch) { pendingPlayersWatch = new MutationObserver(checkPendingPlayers); pendingPlayersWatch.observe(document.documentElement, {childList: true, subtree: true}); + watchedRoots = new WeakSet(); + } + let root = iframe.getRootNode(); + while (root && root !== document && /** @type {ShadowRoot} */ (root).host) { + if (!watchedRoots.has(root)) { + watchedRoots.add(root); + pendingPlayersWatch.observe(root, {childList: true, subtree: true}); + } + root = /** @type {ShadowRoot} */ (root).host.getRootNode(); } } @@ -469,6 +489,15 @@ const address = episodeLinkAddress(button); if (address) { window.location.assign(address); + } else if (!button.isConnected) { + // The next season's link made from the address (getNextOrPreviousButton), where the + // page's sends links elsewhere: a link out of the page cannot navigate, + // and its click did nothing. In the page for the click, as the page's own would be. + const link = /** @type {HTMLAnchorElement} */ (button); + link.hidden = true; + document.body.appendChild(link); + link.click(); + link.remove(); } else { /** @type {HTMLElement} */ (button).click(); } @@ -558,7 +587,6 @@ * @property {PauseWatcher} watcher * @property {HTMLElement} old * @property {boolean} softReplace - * @property {boolean} fillScreen * @property {?HTMLVideoElement} [video] * @property {?PlayedVideo} [played] * @property {boolean} [convertDue] @@ -597,6 +625,11 @@ } RedirectingToPlayer = true; window.location = /** @type {string & Location} */ (newURL.href); + // A page still here a moment later did not go (a "Leave page?" the user said no to): + // its next leave is reported again. Set for good, every later one was skipped. + setTimeout(() => { + RedirectingToPlayer = false; + }, LeaveSettledMs); sendResponse('redirect'); } else { const iframe = document.createElement('iframe'); @@ -640,7 +673,6 @@ watcher, old: video.highest, softReplace, - fillScreen: playerFillsScreen, }; // The video itself, for the stream it played (GET_PLAYED_VIDEO). A site's own query // may have found its player's box: the one video in it, and none when there are @@ -720,7 +752,8 @@ const windowedFullscreenState = iframeObj.windowedFullscreenState; - if (request.force === undefined || request.force !== windowedFullscreenState.active) { + // A state not entered yet has no `active`: an exit asked of it (force false) entered. + if (request.force === undefined || request.force !== !!windowedFullscreenState.active) { windowedFullscreenToggle(iframeObj); } @@ -761,7 +794,8 @@ } try { - if ((request.force !== undefined && request.force === miniplayerState.active) || iframeObj.windowedFullscreenState.active || document.fullscreenElement) { + // As for windowed fullscreen: a state not entered yet is out, not unknown. + if ((request.force !== undefined && request.force === !!miniplayerState.active) || iframeObj.windowedFullscreenState.active || document.fullscreenElement) { updateMiniPlayer(iframeObj); } else { toggleMiniPlayer(iframeObj); @@ -784,7 +818,7 @@ value: false, }); - if (result[0] !== 'recieved') { + if (result !== 'recieved') { console.error('Failed to send miniplayer state to player, assuming its closed'); unmakeMiniPlayer(iframeObj); updateReplacedPlayers(); @@ -807,32 +841,24 @@ } /** - * @param {number|number[]|'all'} frameId + * Tells a player something of its page (through the background, SEND_TO_PLAYER). + * @param {number|undefined} frameId - The player's frame: none for a state never entered. * @param {*} data + * @return {Promise<*>} Its answer; undefined when there is none, or no player to ask. */ - async function sendToPlayer(frameId, data) { - const frameIds = []; - if (typeof frameId === 'number') { - frameIds.push(frameId); - } else if (Array.isArray(frameId)) { - frameIds.push(...frameId); - } else if (frameId === 'all') { - iframeMap.forEach((iframeObj) => { - frameIds.push(iframeObj.frameId); - }); - } - - return Promise.all(frameIds.map((id) => { - return new Promise((resolve) => { - chrome.runtime.sendMessage({ - type: MessageTypes.SEND_TO_PLAYER, - frameId: id, - data, - }, (response) => { - resolve(response); - }); + function sendToPlayer(frameId, data) { + if (typeof frameId !== 'number') return Promise.resolve(undefined); + return new Promise((resolve) => { + chrome.runtime.sendMessage({ + type: MessageTypes.SEND_TO_PLAYER, + frameId, + data, + }, (response) => { + // A player gone meanwhile: no answer, which the caller reads as such. + void chrome.runtime.lastError; + resolve(response); }); - })); + }); } /** @@ -1621,8 +1647,8 @@ }; // Its sounds too, as for the ones added later: an