From 16ea23411309d2c570cdac56b2a2bf5f59c9bfb5 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Thu, 25 Jun 2026 17:49:45 +0530 Subject: [PATCH 1/2] fix(dom): auto-detect open popovers nested in shadow DOM Open native popovers ([popover]:popover-open) are an unambiguous serialize-time state, but the only existing popover handling (markPopoverIfOpen, added in #2141) ran solely on configured pseudoClassEnabledElements resolved via top-document queries -- which require explicit config AND cannot pierce shadow DOM. A popover nested in an open shadow root (e.g. Q2/Tecton Stencil components, consumer_wellness_web) was therefore never stamped, so the renderer hit the UA rule [popover]:not(:popover-open){display:none} and dropped it. Add markOpenPopovers(), called from markPseudoClassElements (the pre-clone marking pass). It walks the live document and every shadow root via a marker-independent walk (eachScopeIncludingShadow, descending through live el.shadowRoot / getShadowRoot) -- NOT walkShadowDOM, whose data-percy-shadow-host markers aren't stamped until cloning, after this pass runs. Every [popover]:popover-open is stamped data-percy-popover-open automatically, with zero config. Stamps are recorded on _liveMutations so cleanup unstamps the live DOM. Warns once and stops querying if :popover-open is unsupported. The renderer already re-opens stamped popovers across shadow boundaries, so no renderer change is needed. Adds a regression test for an open popover inside an open shadow root captured with no config. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/dom/src/serialize-pseudo-classes.js | 38 +++++++++++ .../dom/test/serialize-pseudo-classes.test.js | 65 +++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/packages/dom/src/serialize-pseudo-classes.js b/packages/dom/src/serialize-pseudo-classes.js index 7fca97f74..aca4340b6 100644 --- a/packages/dom/src/serialize-pseudo-classes.js +++ b/packages/dom/src/serialize-pseudo-classes.js @@ -146,6 +146,43 @@ function markInteractiveStates(ctx) { }); } +// Walk the LIVE document and every shadow root (open, or closed via the CDP +// WeakMap) WITHOUT relying on data-percy-shadow-host markers. walkShadowDOM +// descends through those markers, but they are only stamped later during +// cloning — so during this pre-clone marking pass it cannot reach shadow +// content. We descend via the live shadowRoot directly instead. +function eachScopeIncludingShadow(root, visit) { + if (!root || typeof root.querySelectorAll !== 'function') return; + visit(root); + for (const el of root.querySelectorAll('*')) { + const shadow = getShadowRoot(el); + if (shadow) eachScopeIncludingShadow(shadow, visit); + } +} + +// Auto-detect open native popovers page-wide, INCLUDING inside shadow roots. +// `:popover-open` is an unambiguous serialize-time state (like :checked / +// :disabled), so it is stamped automatically rather than only on +// pseudoClassEnabledElements. The renderer's popover-element-helper already +// re-opens any [popover][data-percy-popover-open] across shadow boundaries; +// without this stamp a popover open at snapshot time renders hidden via the +// UA `[popover]:not(:popover-open){display:none}` rule. If `:popover-open` +// is unsupported the selector throws — we stop querying and warn once. +function markOpenPopovers(ctx) { + let supported = true; + eachScopeIncludingShadow(ctx.dom, scope => { + if (!supported) return; + try { + for (const el of scope.querySelectorAll('[popover]:popover-open')) { + stampOnce(ctx, el, POPOVER_OPEN_ATTR, 'true'); + } + } catch (e) { + supported = false; + ctx.warnings.add('Browser does not support :popover-open pseudo-class.'); + } + }); +} + function isPopoverOpen(ctx, element) { try { return element.matches(':popover-open'); @@ -258,6 +295,7 @@ export function getElementsToProcess(ctx, config, markWithId = false) { export function markPseudoClassElements(ctx, config) { ctx._liveMutations = []; markInteractiveStates(ctx); + markOpenPopovers(ctx); if (config) getElementsToProcess(ctx, config, true); } diff --git a/packages/dom/test/serialize-pseudo-classes.test.js b/packages/dom/test/serialize-pseudo-classes.test.js index 7bd0c28f9..f7186ddaf 100644 --- a/packages/dom/test/serialize-pseudo-classes.test.js +++ b/packages/dom/test/serialize-pseudo-classes.test.js @@ -366,6 +366,71 @@ describe('serialize-pseudo-classes', () => { }); }); + // Regression guard for the customer scenario: a native popover opened via + // showPopover() and living inside an open shadow root (e.g. Tecton q2-popover). + // The open/top-layer state is lost on serialization unless the pre-clone pass + // pierces shadow DOM and stamps data-percy-popover-open — and it must do so + // automatically, with no pseudoClassEnabledElements config. + describe('popover auto-detection inside shadow DOM', () => { + let host; + + function popoverSupported() { + return typeof document.createElement('div').showPopover === 'function'; + } + + function openShadowPopover({ mode = 'open', type = 'manual', open = true } = {}) { + host = document.createElement('div'); + host.id = 'shadow-popover-host'; + const root = host.attachShadow({ mode }); + // nosemgrep: javascript.browser.security.insecure-document-method.insecure-document-method + root.innerHTML = `
menu
`; + document.body.appendChild(host); + const popover = root.getElementById('sp'); + if (open) popover.showPopover(); + return popover; + } + + afterEach(() => { + if (!host) return; + try { + const p = host.shadowRoot && host.shadowRoot.querySelector('[popover]'); + if (p && typeof p.hidePopover === 'function' && p.matches(':popover-open')) p.hidePopover(); + } catch (e) { /* ignore */ } + host.remove(); + host = null; + }); + + it('auto-stamps an open popover inside an open shadow root with NO config', () => { + if (!popoverSupported()) { pending('Popover API not supported in this environment'); return; } + const popover = openShadowPopover(); + + // No pseudoClassEnabledElements passed — auto-detection must still reach it. + markPseudoClassElements(ctx); + + expect(popover.getAttribute('data-percy-popover-open')).toBe('true'); + }); + + it('removes the shadow-DOM popover marker from the live DOM on cleanup', () => { + if (!popoverSupported()) { pending('Popover API not supported in this environment'); return; } + const popover = openShadowPopover(); + + markPseudoClassElements(ctx); + expect(popover.hasAttribute('data-percy-popover-open')).toBe(true); + + cleanupInteractiveStateMarkers(ctx); + expect(popover.hasAttribute('data-percy-popover-open')).toBe(false); + }); + + it('does NOT stamp a closed popover inside a shadow root', () => { + if (!popoverSupported()) { pending('Popover API not supported in this environment'); return; } + const popover = openShadowPopover({ open: false }); + + markPseudoClassElements(ctx); + + expect(popover.hasAttribute('data-percy-popover-open')).toBe(false); + }); + }); + describe('rewriteCustomStateCSS', () => { it('rewrites :state() selectors in style elements', () => { withExample(''); From fd6934453f4a9ff2cb96bc3baa4380a216643b44 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Fri, 26 Jun 2026 18:36:05 +0530 Subject: [PATCH 2/2] refactor(dom): narrow popover support try/catch + cover closed-root case Address review on #2321: - markOpenPopovers: scope the try to the `:popover-open` querySelectorAll only (the one thing that can throw a SyntaxError on unsupported engines) and stamp outside it. Previously a throw from stampOnce mid-iteration would be misreported as "Browser does not support :popover-open" AND silently skip every remaining scope. - Add a regression test for an open popover inside a genuinely CLOSED shadow root, registered via the __percyClosedShadowRoots WeakMap (mirroring the CDP exposure path). Asserts host.shadowRoot is null so the test only passes via the getShadowRoot -> getClosedShadowRoot fallback, guarding the closed-root path from diverging from the open-root one. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/dom/src/serialize-pseudo-classes.js | 12 ++++-- .../dom/test/serialize-pseudo-classes.test.js | 37 ++++++++++++++++++- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/packages/dom/src/serialize-pseudo-classes.js b/packages/dom/src/serialize-pseudo-classes.js index aca4340b6..a7ca35069 100644 --- a/packages/dom/src/serialize-pseudo-classes.js +++ b/packages/dom/src/serialize-pseudo-classes.js @@ -172,14 +172,20 @@ function markOpenPopovers(ctx) { let supported = true; eachScopeIncludingShadow(ctx.dom, scope => { if (!supported) return; + // Only the `:popover-open` SELECTOR can legitimately throw a SyntaxError + // (engines without popover support). Scope the try to the query and + // materialize the matches; stamp OUTSIDE the try. Otherwise a throw from + // stampOnce mid-iteration would be misreported as "unsupported" AND would + // silently skip every remaining scope. + let matches; try { - for (const el of scope.querySelectorAll('[popover]:popover-open')) { - stampOnce(ctx, el, POPOVER_OPEN_ATTR, 'true'); - } + matches = scope.querySelectorAll('[popover]:popover-open'); } catch (e) { supported = false; ctx.warnings.add('Browser does not support :popover-open pseudo-class.'); + return; } + for (const el of matches) stampOnce(ctx, el, POPOVER_OPEN_ATTR, 'true'); }); } diff --git a/packages/dom/test/serialize-pseudo-classes.test.js b/packages/dom/test/serialize-pseudo-classes.test.js index f7186ddaf..febc30d23 100644 --- a/packages/dom/test/serialize-pseudo-classes.test.js +++ b/packages/dom/test/serialize-pseudo-classes.test.js @@ -373,6 +373,10 @@ describe('serialize-pseudo-classes', () => { // automatically, with no pseudoClassEnabledElements config. describe('popover auto-detection inside shadow DOM', () => { let host; + // For the closed-root case: keep a handle to the (otherwise unreachable) + // closed root, and remember the previous WeakMap so we can restore it. + let closedRoot; + let origClosedMap; function popoverSupported() { return typeof document.createElement('div').showPopover === 'function'; @@ -385,6 +389,17 @@ describe('serialize-pseudo-classes', () => { // nosemgrep: javascript.browser.security.insecure-document-method.insecure-document-method root.innerHTML = `
menu
`; document.body.appendChild(host); + if (mode === 'closed') { + // host.shadowRoot is null for closed roots. Mimic the CDP closed-shadow + // exposure path: register host -> root in the WeakMap that + // getShadowRoot()/getClosedShadowRoot() reads, so the marking walk can + // reach inside. + closedRoot = root; + origClosedMap = window.__percyClosedShadowRoots; + const map = new WeakMap(); + map.set(host, root); + window.__percyClosedShadowRoots = map; + } const popover = root.getElementById('sp'); if (open) popover.showPopover(); return popover; @@ -393,11 +408,17 @@ describe('serialize-pseudo-classes', () => { afterEach(() => { if (!host) return; try { - const p = host.shadowRoot && host.shadowRoot.querySelector('[popover]'); + const root = host.shadowRoot || closedRoot; + const p = root && root.querySelector('[popover]'); if (p && typeof p.hidePopover === 'function' && p.matches(':popover-open')) p.hidePopover(); } catch (e) { /* ignore */ } host.remove(); host = null; + if (closedRoot) { + window.__percyClosedShadowRoots = origClosedMap; + closedRoot = null; + origClosedMap = undefined; + } }); it('auto-stamps an open popover inside an open shadow root with NO config', () => { @@ -429,6 +450,20 @@ describe('serialize-pseudo-classes', () => { expect(popover.hasAttribute('data-percy-popover-open')).toBe(false); }); + + it('auto-stamps an open popover inside a CLOSED shadow root (via the CDP WeakMap)', () => { + if (!popoverSupported()) { pending('Popover API not supported in this environment'); return; } + const popover = openShadowPopover({ mode: 'closed' }); + + // host.shadowRoot is null for closed roots — reaching the popover relies + // ENTIRELY on getShadowRoot() falling back to __percyClosedShadowRoots. + // This guards the closed-root path from diverging from the open-root one. + expect(host.shadowRoot).toBe(null); + + markPseudoClassElements(ctx); + + expect(popover.getAttribute('data-percy-popover-open')).toBe('true'); + }); }); describe('rewriteCustomStateCSS', () => {