Skip to content

feat: CORS iframe + closed shadow DOM parity#436

Open
aryanku-dev wants to merge 10 commits into
masterfrom
feat/cors-iframes-and-shadow-dom
Open

feat: CORS iframe + closed shadow DOM parity#436
aryanku-dev wants to merge 10 commits into
masterfrom
feat/cors-iframes-and-shadow-dom

Conversation

@aryanku-dev
Copy link
Copy Markdown

Summary

Brings percy-selenium-dotnet to parity with the canonical Percy CORS iframe + closed shadow DOM feature set.

Implemented

  • Nested cross-origin iframe capture (depth-capped, cycle-guarded)
  • data-percy-ignore attribute opt-out
  • ignoreIframeSelectors option
  • Post-switch URL re-check via IsUnsupportedIframeSrc
  • PercyContextLostException recovery merges PartialCapture
  • Closed shadow DOM via CDP (ExposeClosedShadowRoots)
  • Inlined C# helpers

Skipped

  • ElementInternals preflight (Feature 8): N/A — selenium-dotnet has no before-page-load hook.
  • @percy/sdk-utils bump (Feature 9): not applicable to .NET; helpers inlined.

Reference

Mirrored from percy/percy-nightwatch#869 (PER-7292-add-cors-iframe-support); CDP from percy/percy-playwright#609.

Test plan

  • Full repo test suite passed locally (dotnet test)
  • Manual smoke: cross-origin iframes
  • Manual smoke: closed shadow roots in Chrome

🤖 Generated with Claude Code via /percy-sdk-sync

aryanku-dev and others added 10 commits May 11, 2026 15:15
…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>
@aryanku-dev aryanku-dev marked this pull request as ready for review May 24, 2026 11:45
@aryanku-dev aryanku-dev requested a review from a team as a code owner May 24, 2026 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant