fix(compiler): don't scan media tags inside comments/scripts; DOM-check auto-start (#1938) (#1940)

The timing compiler scanned raw HTML with tag regexes that weren't
comment-aware, so a comment or script merely mentioning `<video>`/`<audio>`
was rewritten as a real element — injecting id/data-start/data-hf-auto-start
into the comment text. That phantom attribute then tripped the probe stage's
substring check (`html.includes("data-hf-auto-start")`), launching an
unnecessary browser probe on every render with an unexplained empty reasons list.

- Mask comments, <script>, and <style> regions before the tag scan, then
  restore them verbatim (compileTimingAttrs, extractResolvedMedia).
- Replace the probe's substring match with a DOM query
  (video[data-hf-auto-start]) and add "auto-start video(s)" to the reasons list.
This commit is contained in:
Miguel Ángel
2026-07-06 18:19:35 -04:00
committed by GitHub
parent afa7f292fb
commit 1a3330a972
4 changed files with 64 additions and 3 deletions
@@ -128,6 +128,35 @@ describe("compileTimingAttrs", () => {
expect(unresolved).toHaveLength(0); expect(unresolved).toHaveLength(0);
}); });
it("ignores media tags mentioned inside comments (issue #1938)", () => {
const html =
"<!-- this comment mentions a <video> and an <audio> tag -->\n<p>no media here</p>";
const { html: compiled, unresolved } = compileTimingAttrs(html);
// Comment text is preserved verbatim — no id/data-start/data-hf-auto-start injected.
expect(compiled).toBe(html);
expect(compiled).not.toContain("data-hf-auto-start");
expect(unresolved).toHaveLength(0);
});
it("ignores media tags inside <script> string literals", () => {
const html = '<script>const x = "<video src=\\"a.mp4\\">";</script>';
const { html: compiled, unresolved } = compileTimingAttrs(html);
expect(compiled).toBe(html);
expect(unresolved).toHaveLength(0);
});
it("still compiles real media tags alongside a comment that mentions them", () => {
const html =
'<!-- a <video> in prose -->\n<video src="a.mp4" data-start="0" data-duration="2">';
const { html: compiled } = compileTimingAttrs(html);
expect(compiled).toContain("<!-- a <video> in prose -->");
expect(compiled).toContain('id="hf-video-0"');
expect(compiled).toContain('data-end="2"');
});
}); });
describe("injectDurations", () => { describe("injectDurations", () => {
Binary file not shown.
@@ -1,5 +1,5 @@
import { describe, expect, it, mock } from "bun:test"; import { describe, expect, it, mock } from "bun:test";
import { hasScriptedAudioVolumeAutomation } from "./probeStage.js"; import { hasAutoStartVideos, hasScriptedAudioVolumeAutomation } from "./probeStage.js";
// ── Mocks for runProbeStage tests ──────────────────────────────────────────── // ── Mocks for runProbeStage tests ────────────────────────────────────────────
// Capture the cfg passed to createCaptureSession so we can assert it carries // Capture the cfg passed to createCaptureSession so we can assert it carries
@@ -224,6 +224,26 @@ describe("hasScriptedAudioVolumeAutomation", () => {
}); });
}); });
describe("hasAutoStartVideos", () => {
it("detects a real auto-start video element", () => {
expect(hasAutoStartVideos(`<video src="a.mp4" data-hf-auto-start="">`)).toBe(true);
});
it("ignores the attribute mentioned in a comment (issue #1938)", () => {
expect(hasAutoStartVideos(`<!-- videos get data-hf-auto-start injected --><p>hi</p>`)).toBe(
false,
);
});
it("ignores the attribute in prose text", () => {
expect(hasAutoStartVideos(`<p>the data-hf-auto-start sentinel</p>`)).toBe(false);
});
it("returns false when there is no media", () => {
expect(hasAutoStartVideos(`<div class="clip"></div>`)).toBe(false);
});
});
describe("runProbeStage — forceScreenshot threading", () => { describe("runProbeStage — forceScreenshot threading", () => {
it("passes forceScreenshot:true to createCaptureSession when stage input carries it but cfg does not (low-memory mode fix #1236)", async () => { it("passes forceScreenshot:true to createCaptureSession when stage input carries it but cfg does not (low-memory mode fix #1236)", async () => {
capturedCfgs.length = 0; capturedCfgs.length = 0;
@@ -114,6 +114,17 @@ export function hasScriptedAudioVolumeAutomation(html: string, audioCount: numbe
); );
} }
/**
* True when the compiled HTML has at least one `<video>` carrying the
* auto-injected `data-hf-auto-start` sentinel. Uses a DOM query, not a
* substring scan — `html.includes("data-hf-auto-start")` false-fires on any
* comment or prose that merely mentions the attribute (issue #1938).
*/
export function hasAutoStartVideos(html: string): boolean {
const { document } = parseHTML(html);
return document.querySelector("video[data-hf-auto-start]") !== null;
}
export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageResult> { export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageResult> {
const { const {
projectDir, projectDir,
@@ -139,7 +150,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
let lastBrowserConsole: string[] = []; let lastBrowserConsole: string[] = [];
const probeStart = Date.now(); const probeStart = Date.now();
const hasAutoStartVideos = compiled.html.includes("data-hf-auto-start"); const hasAutoStart = hasAutoStartVideos(compiled.html);
const hasScriptedAudio = hasScriptedAudioVolumeAutomation( const hasScriptedAudio = hasScriptedAudioVolumeAutomation(
compiled.html, compiled.html,
composition.audios.length, composition.audios.length,
@@ -147,7 +158,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
const needsBrowser = const needsBrowser =
composition.duration <= 0 || composition.duration <= 0 ||
compiled.unresolvedCompositions.length > 0 || compiled.unresolvedCompositions.length > 0 ||
hasAutoStartVideos || hasAutoStart ||
hasScriptedAudio; hasScriptedAudio;
if (needsBrowser) { if (needsBrowser) {
@@ -155,6 +166,7 @@ export async function runProbeStage(input: ProbeStageInput): Promise<ProbeStageR
if (composition.duration <= 0) reasons.push("root duration unknown"); if (composition.duration <= 0) reasons.push("root duration unknown");
if (compiled.unresolvedCompositions.length > 0) if (compiled.unresolvedCompositions.length > 0)
reasons.push(`${compiled.unresolvedCompositions.length} unresolved composition(s)`); reasons.push(`${compiled.unresolvedCompositions.length} unresolved composition(s)`);
if (hasAutoStart) reasons.push("auto-start video(s)");
if (hasScriptedAudio) reasons.push("scripted audio volume"); if (hasScriptedAudio) reasons.push("scripted audio volume");
log.info("Launching browser for composition probe...", { log.info("Launching browser for composition probe...", {