From 5f00d0644c501c128b30f6fe6f3a840bac2472f2 Mon Sep 17 00:00:00 2001 From: Aleksandr Shevchuk Date: Wed, 14 Oct 2026 11:04:54 +0300 Subject: [PATCH] fix: boot early only on ready, not on an answer from the whole archive On a host without Range the element indexes on every progress report and boots early when the answer allows. Only a partial index carries `ready`, decided by the dependency walk; once the download is whole the worker answers from the central directory with no `ready` and no walk, and the element booted on that answer because it checked `ready !== false`. The runtime then raced the library bundle's attachment for the missing library.json: 1 or 2 plays in 30 failed in WebKit. Any answer but `ready: true` now waits for the final index. Closes #32. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 10 ++++++++++ packages/player/src/h5p-offline-player.ts | 6 +++++- .../player/tests/browser/libraries.test.ts | 20 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 54afa4a..c7ae866 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -520,6 +520,16 @@ element, and the element acts. That relay is why `frame-document.ts` has a `mess its library folder, which left the race a few milliseconds wide; paced through `?throttle=` with its media last, it failed every time in Chromium. While a dependency is absent, every progress report walks the tree again, a few local reads each. + The element boots early only on `ready: true`, never on its absence. The last progress + report's `index` can reach the worker after the download is whole, and the worker then answers + from the central directory: no `ready`, since only a partial index has one, and no walk, since + that is the final `deep` index's. Up to 0.9.3 the element booted unless `ready` was `false`, so + on an unpaced download it booted on that answer every time, measured, and the runtime raced + the bundle's attachment for the missing `library.json`; WebKit lost the race in 1 or 2 plays of + 30, Chromium in none, and CI caught it once more on 2026-10-14. A content-only export was never + exposed: the plain index refuses a package missing a library `h5p.json` declares, so there is no + answer to boot on. `libraries.test.ts` pins the states such a load goes through, `indexing` + between the package's download and the bundle's. - **What a load asked for is the load's, not the package's.** The `packages` row is keyed by `pkgId`, a hash of the source, and up to 2026-10-13 it also held what the last load asked for: `untrusted`, the language, the display options, the frame assets, `allow-origins` and the diff --git a/packages/player/src/h5p-offline-player.ts b/packages/player/src/h5p-offline-player.ts index d781569..aca601c 100644 --- a/packages/player/src/h5p-offline-player.ts +++ b/packages/player/src/h5p-offline-player.ts @@ -748,7 +748,11 @@ export class H5PPlayerElement extends HTMLElement { attempt = this.send({ type: 'index', ...load }) .then((reply) => { if (signal.aborted || booted) return - if (reply.ok && reply.type === 'indexed' && reply.ready !== false) { + // Only a partial index says `ready`, and only after the dependency walk. Once the + // download is whole the worker answers from the central directory with no `ready` and + // no walk, which is the final index's job below: booting on that answer let the runtime + // ask for a library before the bundle that supplies it was attached. + if (reply.ok && reply.type === 'indexed' && reply.ready === true) { booted = true this.iframe.title = frameTitle(reply.title) this.navigateFrame(frameUrl) diff --git a/packages/player/tests/browser/libraries.test.ts b/packages/player/tests/browser/libraries.test.ts index 41aa7f6..e6aa687 100644 --- a/packages/player/tests/browser/libraries.test.ts +++ b/packages/player/tests/browser/libraries.test.ts @@ -152,6 +152,26 @@ describe('supplying missing libraries', () => { } }) + // Unpaced, the download is whole before the frame could boot from the forward index, and the + // last progress report's index is answered from the central directory: no `ready`, and no + // dependency walk, which is the final index's. The element once booted on that answer, every + // time, so the runtime raced the bundle's attachment for the missing library and in WebKit + // sometimes lost: a 404 read as JSON. Now it stops when the download ends (`indexing`), the + // bundle is downloaded (`downloading` again) and attached, and only then does the frame boot. + // Booted early, the bundle's `downloading` ran on from the package's, with no `indexing` between. + it('boots a whole download only after the final index has supplied what its libraries need', async () => { + const player = createPlayer({ libraries: FIXTURES.libraries }) + const states: string[] = [] + player.addEventListener('statechange', (event) => states.push((event as CustomEvent<{ state: string }>).detail.state)) + const settled = waitForSettled(player) + player.setAttribute('src', `${FIXTURES.needsDependencyNoRange}?case=whole-${Date.now()}`) + + const result = await settled + if (!result.ok) expect.fail(`${result.detail.code} — ${result.detail.message}`) + expect(states).toEqual(['probing', 'downloading', 'indexing', 'downloading', 'indexing', 'ready']) + expect((frameWindow(player) as unknown as { h5pOfflineExtraLoaded?: boolean }).h5pOfflineExtraLoaded).toBe(true) + }) + it('names the bundle in the revision, after the package', async () => { const player = await playWithLibraries(FIXTURES.needsLibraries, FIXTURES.libraries) expect(player.revision).toMatch(/^sha256:[0-9a-f]{64}; libraries sha256:[0-9a-f]{64}$/)