feat: CORS iframe + closed shadow DOM parity#436
Open
aryanku-dev wants to merge 10 commits into
Open
Conversation
…ore, and recovery controls Replace the single-level processFrame helper with a recursive ProcessFrameTree pipeline that mirrors the canonical Percy CORS iframe spec from percy/percy-nightwatch#869. Adds: - DEFAULT_MAX_FRAME_DEPTH/MAX_ALLOWED_FRAME_DEPTH, ClampFrameDepth, NormalizeIgnoreSelectors, ResolveMaxFrameDepth, ResolveIgnoreSelectors inlined since .NET has no @percy/sdk-utils equivalent. - ENUMERATE_IFRAMES_SCRIPT + IframeInfo to collect iframe metadata in one round-trip per frame context. - ShouldSkipIframe centralizes filtering: data-percy-ignore attribute, ignoreIframeSelectors matches, unsupported / srcdoc / same-origin frames, and frames without data-percy-element-id are dropped early. - ProcessFrameTree recurses cross-origin descendants up to MaxFrameDepth, with a HashSet ancestor-URL chain to break cyclic A->B->A graphs. - Post-switch URL re-check: after SwitchTo().Frame, the loaded document.URL is rechecked against IsUnsupportedIframeSrc to handle late navigations. - PercyContextLostException carries the partial capture so a failed SwitchTo().ParentFrame() unwind still surfaces every frame serialized up to the failure. CaptureCorsIframes wires the helpers into getSerializedDom, replacing the prior ad-hoc top-level iframe loop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors percy/percy-playwright#609. Walks the CDP DOM tree (DOM.getDocument with depth=-1 and pierce=true), collects backendNodeId pairs for every closed shadow root, resolves both sides via DOM.resolveNode, then uses Runtime.callFunctionOn to register each shadow root in a window-bound WeakMap (window.__percyClosedShadowRoots) that PercyDOM.serialize() reads during cloning. Nodes inside child frame documents are skipped because their execution contexts don't share the WeakMap. Wired into Snapshot() before serialization and into the responsive capture reload path so the WeakMap survives page.reload(). No-op on non-Chrome drivers and when ExecuteCdpCommand isn't available, so existing Firefox / non-Chrome test paths are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Covers the inlined helpers (GetOrigin, IsUnsupportedIframeSrc,
ClampFrameDepth, NormalizeIgnoreSelectors), the ShouldSkipIframe skip
matrix (data-percy-ignore, ignoreIframeSelectors, unsupported src,
srcdoc, same-origin, missing percyElementId, and immediate-parent
origin comparison), and the PercyContextLostException carrier.
Tests rely on the existing InternalsVisibleTo("Percy.Test") in
AssemblyInfo.cs and use reflection for the private IframeInfo /
ShouldSkipIframe symbols so the production API stays sealed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add volatile to the _http field so the unlocked outer read in getHttpClient sees a fully-published HttpClient (Timeout set) on weak memory models. Without it the DCL pattern provides no happens-before guarantee on ARM/Apple Silicon. - Pair DOM.enable with DOM.disable in a finally block so the CDP session doesn't keep emitting DOM events after every snapshot + resize/reload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two reflection-based unit tests confirm the invariant the newly-volatile _http field exists to protect: - First getHttpClient() call returns a client whose Timeout is already TimeSpan.FromMinutes(10) (the bug we fixed: setting Timeout AFTER assigning the field could let concurrent callers observe a default 100s timeout). - Subsequent calls return the same instance — DCL must not rebuild. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CE re-review flagged ClampFrameDepth silently mapping user-supplied 0 to DEFAULT_MAX_FRAME_DEPTH (5) as a hidden behavior. The intent matches @percy/sdk-utils clampFrameDepth and the JS SDKs: 0 means "use default" (unset / config placeholder), not "disable iframe capture". To disable nested capture, users should omit the option or use data-percy-ignore / ignoreIframeSelectors. No behavior change — pure documentation so future readers don't re-flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CE re-review flagged that CollectClosedShadowRoots early-returned on any
node carrying contentDocument, so closed shadow roots living inside
same-origin iframes were silently skipped — even though those frames
share the parent's JS realm and therefore the same
window.__percyClosedShadowRoots WeakMap that PercyDOM.serialize reads.
Walker now decides per-frame:
* Same origin (compared via GetOrigin against the page URL) -> recurse
INTO contentDocument; found closed roots use the parent realm's map.
* Cross origin -> still skipped (different JS realm, the resolveNode
objectIds don't belong to our execution context anyway).
* Missing documentURL / unknown origin -> defensive skip (pre-fix
behavior for the unknown case).
Also factored the CDP flow inside ExposeClosedShadowRoots into a pure
internal RunClosedShadowRootExposure that takes a CDP invoker delegate,
a script-runner delegate, and a page-URL getter. The WebDriver-bound
entrypoint now adapts the reflected ExecuteCdpCommand to those delegates,
which makes the DOM.enable / DOM.disable lifecycle directly unit-testable
without a real browser.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses CE re-review MINORs and MAJOR coverage gaps:
* Same-origin closed-shadow-in-iframe recursion: three new tests drive
CollectClosedShadowRoots with synthesized CDP DOM payloads — one
same-origin iframe (closed root inside contentDocument is captured),
one cross-origin iframe (skipped), one with missing documentURL
(defensive skip). Plus a sanity test that a top-level closed root
still works.
* DOM.disable invocation regression coverage: a FakeCdp recorder
exercises RunClosedShadowRootExposure end-to-end and asserts:
- DOM.disable is sent after a successful run, as the final command.
- DOM.disable is still sent if DOM.getDocument throws.
- DOM.disable is NOT sent when DOM.enable itself throws (domEnabled
stays false in the finally block — avoids spurious disable on a
session that never enabled the domain).
* HttpClient state serialization: the GetHttpClient_* tests mutate the
static volatile _http field. Tag the suite with
[Collection("HttpClientStateSerial")] (DisableParallelization = true)
so a future flip of parallelizeTestCollections in xunit.runner.json
can't race them against Percy.Test.cs / PercyDriver.Test.cs, which
also touch setHttpClient.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings percy-selenium-dotnet to parity with the canonical Percy CORS iframe + closed shadow DOM feature set.
Implemented
data-percy-ignoreattribute opt-outignoreIframeSelectorsoptionIsUnsupportedIframeSrcPercyContextLostExceptionrecovery mergesPartialCaptureExposeClosedShadowRoots)Skipped
Reference
Mirrored from percy/percy-nightwatch#869 (PER-7292-add-cors-iframe-support); CDP from percy/percy-playwright#609.
Test plan
dotnet test)🤖 Generated with Claude Code via /percy-sdk-sync