Skip to content

fix(engine): harden DOM mask visibility restore #1953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
miguel-heygen merged 1 commit into from
Jul 5, 2026
Merged

fix(engine): harden DOM mask visibility restore #1953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 92 additions & 18 deletions packages/engine/src/services/screenshotService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,29 @@ describe("video-frame injection respects ancestor visibility", () => {
} as unknown as Page;
}

function installDomMaskGlobals(setup: { window: Window; document: Document }): () => void {
const globals = globalThis as unknown as {
window?: Window;
document?: Document;
HTMLElement?: typeof HTMLElement;
CSS?: typeof CSS;
};
const previousWindow = globals.window;
const previousDocument = globals.document;
const previousHTMLElement = globals.HTMLElement;
const previousCSS = globals.CSS;
globals.window = setup.window;
globals.document = setup.document;
globals.HTMLElement = setup.window.HTMLElement;
globals.CSS = { escape: (value: string) => value.replace(/[^a-zA-Z0-9_-]/g, "\\$&") } as CSS;
return () => {
globals.window = previousWindow;
globals.document = previousDocument;
globals.HTMLElement = previousHTMLElement;
globals.CSS = previousCSS;
};
}

it("skips replacement-frame creation when the video's host has visibility:hidden", async () => {
const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
try {
Expand Down Expand Up @@ -569,20 +592,7 @@ describe("video-frame injection respects ancestor visibility", () => {
}),
});

const globals = globalThis as unknown as {
window?: Window;
document?: Document;
HTMLElement?: typeof HTMLElement;
CSS?: typeof CSS;
};
const previousWindow = globals.window;
const previousDocument = globals.document;
const previousHTMLElement = globals.HTMLElement;
const previousCSS = globals.CSS;
globals.window = window;
globals.document = document;
globals.HTMLElement = window.HTMLElement;
globals.CSS = { escape: (value: string) => value.replace(/[^a-zA-Z0-9_-]/g, "\\$&") } as CSS;
const teardown = installDomMaskGlobals({ window, document });
try {
await applyDomLayerMask(passthroughPage(), ["scene"], []);
expect(scene.style.visibility || "").toBe("");
Expand All @@ -592,10 +602,74 @@ describe("video-frame injection respects ancestor visibility", () => {
expect(label.style.visibility).toBe("hidden");
expect(label.hasAttribute("data-hf-dom-layer-mask-hidden")).toBe(false);
} finally {
globals.window = previousWindow;
globals.document = previousDocument;
globals.HTMLElement = previousHTMLElement;
globals.CSS = previousCSS;
teardown();
}
});

it("removeDomLayerMask keeps hidden timed descendants hidden when they are also extraHideIds", async () => {
const { window, document } = parseHTML(
`<html><head></head><body>
<div id="scene" data-start="0" data-duration="6">
<div id="caption" data-start="4.5" data-duration="1.5">late label</div>
</div>
</body></html>`,
);
const caption = document.getElementById("caption") as HTMLElement;
caption.style.visibility = "hidden";

Object.defineProperty(window, "getComputedStyle", {
configurable: true,
value: (el: Element) => ({
display: (el as HTMLElement).style.display || "block",
visibility: (el as HTMLElement).style.visibility || "visible",
}),
});

const teardown = installDomMaskGlobals({ window, document });
try {
await applyDomLayerMask(passthroughPage(), ["scene"], ["caption"]);
await removeDomLayerMask(passthroughPage(), ["caption"]);

expect(caption.style.visibility).toBe("hidden");
expect(caption.hasAttribute("data-hf-dom-layer-mask-hidden")).toBe(false);
} finally {
teardown();
}
});

it("removeDomLayerMask restores extraHideIds and render frames to previous visibility", async () => {
const { window, document } = parseHTML(
`<html><head></head><body>
<div id="visible-caption" data-start="0" data-duration="1">current</div>
<video id="clip" data-start="0" data-duration="1"></video>
<img id="__render_frame_clip__" />
</body></html>`,
);
const visibleCaption = document.getElementById("visible-caption") as HTMLElement;
const clip = document.getElementById("clip") as HTMLElement;
const renderFrame = document.getElementById("__render_frame_clip__") as HTMLElement;
visibleCaption.style.visibility = "visible";
clip.style.visibility = "hidden";
renderFrame.style.setProperty("visibility", "hidden", "important");

Object.defineProperty(window, "getComputedStyle", {
configurable: true,
value: (el: Element) => ({
display: (el as HTMLElement).style.display || "block",
visibility: (el as HTMLElement).style.visibility || "visible",
}),
});

const teardown = installDomMaskGlobals({ window, document });
try {
await applyDomLayerMask(passthroughPage(), [], ["visible-caption", "clip"]);
await removeDomLayerMask(passthroughPage(), ["visible-caption", "clip"]);

expect(visibleCaption.style.visibility).toBe("visible");
expect(clip.style.visibility).toBe("hidden");
expect(renderFrame.style.visibility).toBe("hidden");
} finally {
teardown();
}
});
});
78 changes: 37 additions & 41 deletions packages/engine/src/services/screenshotService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,12 @@ const DOM_LAYER_MASK_PREV_PRIORITY_ATTR = "data-hf-dom-layer-mask-prev-priority"
* layer elements remain visible even though intermediate parents are
* hidden by the mass-hide rule.
* 2. Inline-hide each `extraHideId` (and its `__render_frame_*` sibling) with
* `visibility: hidden !important`. Inline `!important` beats stylesheet
* `!important`, so this overrides the show rule for elements that fall
* under a show selector but should NOT paint — typically other-layer
* elements that are descendants of a container layer (for example HDR
* videos and other-layer SDR videos are descendants of `#root` when we
* capture the root DOM layer).
* `visibility: hidden !important`, while first recording its previous
* inline visibility. Inline `!important` beats stylesheet `!important`,
* so this overrides the show rule for elements that fall under a show
* selector but should NOT paint — typically other-layer elements that are
* descendants of a container layer (for example HDR videos and other-layer
* SDR videos are descendants of `#root` when we capture the root DOM layer).
* 3. Inline-hide timed descendants of shown elements that were hidden before
* the mask was installed. This covers idless child clips and same-layer
* descendants that the `extraHideIds` id list cannot represent.
Expand Down Expand Up @@ -332,7 +332,7 @@ export async function applyDomLayerMask(
const existing = document.getElementById(args.styleId);
if (existing) existing.remove();

const restoreMaskedTimedDescendants = () => {
const restoreMaskedElements = () => {
const masked = document.querySelectorAll(`[${args.hiddenAttr}="1"]`);
for (const node of masked) {
if (!(node instanceof HTMLElement)) continue;
Expand All @@ -348,7 +348,29 @@ export async function applyDomLayerMask(
node.removeAttribute(args.prevPriorityAttr);
}
};
restoreMaskedTimedDescendants();
restoreMaskedElements();

const rememberAndHideElement = (el: HTMLElement) => {
if (el.getAttribute(args.hiddenAttr) !== "1") {
const prevVisibility = el.style.getPropertyValue("visibility");
const prevPriority =
typeof el.style.getPropertyPriority === "function"
? el.style.getPropertyPriority("visibility")
: "";
if (prevVisibility) {
el.setAttribute(args.prevVisibilityAttr, prevVisibility);
} else {
el.removeAttribute(args.prevVisibilityAttr);
}
if (prevPriority) {
el.setAttribute(args.prevPriorityAttr, prevPriority);
} else {
el.removeAttribute(args.prevPriorityAttr);
}
el.setAttribute(args.hiddenAttr, "1");
}
el.style.setProperty("visibility", "hidden", "important");
};

const hiddenTimedDescendants: HTMLElement[] = [];
const rememberHiddenTimedDescendants = (root: Element) => {
Expand Down Expand Up @@ -382,34 +404,17 @@ export async function applyDomLayerMask(
document.head.appendChild(style);

for (const el of hiddenTimedDescendants) {
if (el.getAttribute(args.hiddenAttr) === "1") continue;
const prevVisibility = el.style.getPropertyValue("visibility");
const prevPriority =
typeof el.style.getPropertyPriority === "function"
? el.style.getPropertyPriority("visibility")
: "";
if (prevVisibility) {
el.setAttribute(args.prevVisibilityAttr, prevVisibility);
} else {
el.removeAttribute(args.prevVisibilityAttr);
}
if (prevPriority) {
el.setAttribute(args.prevPriorityAttr, prevPriority);
} else {
el.removeAttribute(args.prevPriorityAttr);
}
el.setAttribute(args.hiddenAttr, "1");
el.style.setProperty("visibility", "hidden", "important");
rememberAndHideElement(el);
}

for (const id of args.hide) {
const el = document.getElementById(id);
if (el) {
el.style.setProperty("visibility", "hidden", "important");
rememberAndHideElement(el);
}
const img = document.getElementById(`__render_frame_${id}__`);
if (img) {
img.style.setProperty("visibility", "hidden", "important");
rememberAndHideElement(img);
}
}
},
Expand All @@ -427,8 +432,9 @@ export async function applyDomLayerMask(
/**
* Tear down the mask installed by applyDomLayerMask.
*
* Removes the mask stylesheet and clears the inline `visibility` properties
* set on `extraHideIds` (and their `__render_frame_*` siblings).
* Removes the mask stylesheet and restores the inline `visibility` values
* temporarily overwritten for hidden timed descendants, `extraHideIds`, and
* their `__render_frame_*` siblings.
*
* IMPORTANT: We do NOT strip inline `opacity` here. applyDomLayerMask only
* ever sets `visibility` (never `opacity`), so any inline opacity present on
Expand All @@ -438,10 +444,9 @@ export async function applyDomLayerMask(
* we strip opacity here and then seek to the same time for the next layer,
* GSAP won't put it back and the wrapper will render fully opaque.
*/
export async function removeDomLayerMask(page: Page, extraHideIds: string[]): Promise<void> {
export async function removeDomLayerMask(page: Page, _extraHideIds: string[]): Promise<void> {
await page.evaluate(
(args: {
hide: string[];
styleId: string;
hiddenAttr: string;
prevVisibilityAttr: string;
Expand All @@ -463,17 +468,8 @@ export async function removeDomLayerMask(page: Page, extraHideIds: string[]): Pr
node.removeAttribute(args.prevVisibilityAttr);
node.removeAttribute(args.prevPriorityAttr);
}
for (const id of args.hide) {
const el = document.getElementById(id);
if (el) {
el.style.removeProperty("visibility");
}
const img = document.getElementById(`__render_frame_${id}__`);
if (img) img.style.removeProperty("visibility");
}
},
{
hide: extraHideIds,
styleId: DOM_LAYER_MASK_STYLE_ID,
hiddenAttr: DOM_LAYER_MASK_HIDDEN_ATTR,
prevVisibilityAttr: DOM_LAYER_MASK_PREV_VISIBILITY_ATTR,
Expand Down
Loading