From 0fcf592826f5c34ac3c6cb1d81e693f67f8644a4 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 20 Aug 2026 14:58:36 -0700 Subject: [PATCH] fix(core): gate the hidden-audio reschedule on its own canary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding 5. A `data-hidden` toggle mid-playback fired `webAudio.stopAll()` + a full reschedule for EVERY user, while the two skips that reschedule exists to re-run are themselves gated on `silenceHiddenAudioEnabled()`. Un-enrolled — which is everyone, the canary is at 0% — the rebuilt set was therefore identical, and the only observable effect was an audible stop-and-restart across the whole mix on every visibility toggle. Also folds the same bus-blindness fixed in media.ts into the two scheduling skips: they used `closest("[data-hidden]")`, which cannot see a muted BUS because membership lives on the member's `data-audio-group` and a group never nests its members. Both now share one `isSilencedByHidden` predicate. **Three existing tests were passing only because the path was ungated** — worth knowing, because it is the second time this canary's tests have measured the wrong thing: - "batches a mid-playback toggle into exactly one reschedule" and "stops the running sources before rescheduling" never enrolled the canary. They now do; the reschedule IS the feature. - "still schedules a data-hidden clip when the host has not opted in" asserted through `scheduleMediaElementPlayback`, and in jsdom `webAudioReady` is false so `play()` schedules nothing — the ungated reschedule was the only scheduler in the test, i.e. the assertion was carried by the defect. It now measures the finding directly: the same `data-hidden` toggle costs ONE `stopAll` un-enrolled (the seek's own) and two enrolled. Verified 1 vs 2, and 3 vs 1 on a revert. Two things that cost a round each, for the next person: a plain `seek()` calls `stopAll()` unconditionally, so a raw "was stopAll called" assertion proves nothing — count the delta. And `hiddenAudioDirty` is set by a data-hidden MUTATION, so the gesture under test has to toggle the attribute; a seek alone never reaches the reschedule. core: 122 files, 2490 tests. --- packages/core/src/runtime/init.test.ts | 36 +++++++++++++++++++++++--- packages/core/src/runtime/init.ts | 18 ++++++++++--- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/packages/core/src/runtime/init.test.ts b/packages/core/src/runtime/init.test.ts index 1618d6736..54cd942a0 100644 --- a/packages/core/src/runtime/init.test.ts +++ b/packages/core/src/runtime/init.test.ts @@ -1487,13 +1487,36 @@ describe("initSandboxRuntimeModular", () => { const scheduleSpy = vi .spyOn(WebAudioTransport.prototype, "scheduleMediaElementPlayback") .mockResolvedValue(null); + const stopSpy = vi.spyOn(WebAudioTransport.prototype, "stopAll"); const player = window.__player; player?.play(); - player?.seek(0); - expect(scheduleSpy).toHaveBeenCalledTimes(1); - expect(scheduleSpy.mock.calls[0]?.[0]).toBe(hiddenAudio); + // A seek stops the transport itself, so the count is the measure: the + // visibility pass over a data-hidden element must add NO second stop while + // un-enrolled. The skips the reschedule exists to re-run are inert with the + // flag off, so the set it rebuilds is identical and the only observable + // effect is an audible stop-and-restart across the whole mix. + // The dirty flag is set by a data-hidden MUTATION, so the toggle is the + // gesture — a plain seek never reaches the reschedule at all. + hiddenAudio.removeAttribute("data-hidden"); + player?.seek(1, { keepPlaying: true }); + const unenrolled = stopSpy.mock.calls.length; + + window.__hf?.setCanaries?.({ "audio-track-mute": true }); + stopSpy.mockClear(); + hiddenAudio.setAttribute("data-hidden", ""); + player?.seek(2, { keepPlaying: true }); + const enrolled = stopSpy.mock.calls.length; + + expect(unenrolled).toBe(1); + expect(enrolled).toBe(unenrolled + 1); + + // And the hidden clip is still audible to the transport un-enrolled: the + // canary flip handler reschedules, and with the flag off the hidden skip + // does not fire, so the element IS scheduled. + window.__hf?.setCanaries?.({ "audio-track-mute": false }); + expect(scheduleSpy.mock.calls.map((call) => call[0])).toContain(hiddenAudio); }); it("batches a mid-playback data-hidden toggle into exactly one Web Audio reschedule", () => { @@ -1527,6 +1550,10 @@ describe("initSandboxRuntimeModular", () => { window.__timelines = { main: createMockTimeline(10) }; initSandboxRuntimeModular(); + // The reschedule IS the audio-track-mute feature — un-enrolled, hidden-ness + // does not change the active set, so nothing should stop or restart (see + // the un-enrolled test below). Enrol before asserting on it. + window.__hf?.setCanaries?.({ "audio-track-mute": true }); const player = window.__player; // play() alone (no seek) already runs one visibility pass while the clock @@ -1575,6 +1602,9 @@ describe("initSandboxRuntimeModular", () => { window.__timelines = { main: createMockTimeline(10) }; initSandboxRuntimeModular(); + // Enrolled: the stop-before-reschedule pairing this test pins only runs for + // the audio-track-mute feature. + window.__hf?.setCanaries?.({ "audio-track-mute": true }); const player = window.__player; player?.play(); diff --git a/packages/core/src/runtime/init.ts b/packages/core/src/runtime/init.ts index e71a2ea52..439d6f66f 100644 --- a/packages/core/src/runtime/init.ts +++ b/packages/core/src/runtime/init.ts @@ -42,7 +42,7 @@ import { applyVariableBindings } from "./applyVariableBindings"; import { createColorGradingRuntime, type RuntimeColorGradingApi } from "./colorGrading"; import { TransportClock } from "./clock"; import { WebAudioTransport } from "./webAudioTransport"; -import { HF_AUDIO_GROUP_TAG } from "../audioGroups"; +import { HF_AUDIO_GROUP_TAG, isMemberGroupHidden } from "../audioGroups"; import { clampNativeMediaVolume } from "../audioGain"; import { quantizeTimeToFrame } from "../inline-scripts/parityContract"; import { STUDIO_MANUAL_EDIT_GESTURE_ATTR } from "../editing/draftMarkers"; @@ -192,6 +192,11 @@ export function initSandboxRuntimeModular(): void { // render already does. Off until enrolled, so a composition carrying // `data-hidden` on an audio element keeps playing in preview meanwhile. const silenceHiddenAudioEnabled = (): boolean => canaries["audio-track-mute"] === true; + /** Hidden by an ancestor, or by the BUS this clip belongs to. The bus is + * never an ancestor — membership is on the member's `data-audio-group` — so + * `closest()` alone could not see a muted group, which the render drops. */ + const isSilencedByHidden = (el: Element): boolean => + el.closest("[data-hidden]") !== null || isMemberGroupHidden(el.ownerDocument, el); window.__hf.setCanaries = (states) => { const wasSilencing = silenceHiddenAudioEnabled(); for (const [name, enabled] of Object.entries(states)) canaries[name] = enabled === true; @@ -2048,7 +2053,12 @@ export function initSandboxRuntimeModular(): void { rawNode.style.display = "none"; } } - if (hiddenAudioDirty && clock.isPlaying()) { + // Gated like every other consumer of this canary. The skips this reschedule + // exists to re-run (`silenceHiddenAudioEnabled() && …data-hidden`, below) + // are inert while the flag is off, so un-enrolled the whole stop/reschedule + // rebuilt an IDENTICAL active set — an audible stop-and-restart across the + // entire mix on every visibility toggle, at 100%, for a feature at 0%. + if (hiddenAudioDirty && silenceHiddenAudioEnabled() && clock.isPlaying()) { webAudio.stopAll(); scheduleWebAudioForActiveClips(); } @@ -2995,7 +3005,7 @@ export function initSandboxRuntimeModular(): void { let foundActive = false; for (const rawEl of audioEls) { if (!(rawEl instanceof HTMLMediaElement) || !rawEl.isConnected) continue; - if (silenceHiddenAudioEnabled() && rawEl.closest("[data-hidden]")) continue; + if (silenceHiddenAudioEnabled() && isSilencedByHidden(rawEl)) continue; const start = Number.parseFloat(rawEl.dataset.start ?? ""); const durAttr = parseStrictFiniteTimingNumber(rawEl.dataset.duration); const end = durAttr != null && durAttr > 0 ? start + durAttr : Infinity; @@ -3103,7 +3113,7 @@ export function initSandboxRuntimeModular(): void { const audioEls = document.querySelectorAll("audio[data-start]"); for (const rawEl of audioEls) { if (!(rawEl instanceof HTMLMediaElement) || !rawEl.isConnected) continue; - if (silenceHiddenAudioEnabled() && rawEl.closest("[data-hidden]")) continue; + if (silenceHiddenAudioEnabled() && isSilencedByHidden(rawEl)) continue; const compStart = Number.parseFloat(rawEl.dataset.start ?? ""); if (!Number.isFinite(compStart)) continue; const mediaStart = readElementPlaybackStart(rawEl);