From db3b0e8e55aa8fa0fd6556272d06ea1c59e6a8d4 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 11 May 2026 12:23:30 +0530 Subject: [PATCH 01/22] chore: inline @percy/sdk-utils helpers for iframe + ignore-selector parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brings the same helper surface used by percy-nightwatch / percy-webdriverio into percy-selenium-python directly: DEFAULT_MAX_FRAME_DEPTH, clamp_frame_depth, normalize_ignore_selectors, is_unsupported_iframe_src, get_origin, resolve_max_frame_depth, resolve_ignore_selectors, and the PercyContextLost exception. No SDK version bump — Python doesn't share a utils package with the JS SDKs, so the helpers ship inline. Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 109 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 100 insertions(+), 9 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index 746bd3b..d9ba7a9 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -177,19 +177,110 @@ def process_frame(driver, frame_element, options, percy_dom_script): } -def _is_unsupported_iframe_src(frame_src): - return ( - not frame_src or - frame_src == "about:blank" or - frame_src.startswith("javascript:") or - frame_src.startswith("data:") or - frame_src.startswith("vbscript:") +# --------------------------------------------------------------------------- +# Inlined SDK helpers (mirrors @percy/sdk-utils used by Node SDKs). We do not +# bump a shared utils package — selenium-python ships these directly so that +# behavior stays in sync with percy-nightwatch / percy-webdriverio. +# --------------------------------------------------------------------------- + +DEFAULT_MAX_FRAME_DEPTH = 5 + + +def is_unsupported_iframe_src(frame_src): + """True if a frame's src cannot be navigated/loaded for serialization.""" + if not frame_src: + return True + unsupported_exact = ("about:blank", "about:srcdoc") + unsupported_prefixes = ( + "javascript:", "data:", "vbscript:", "blob:", + "chrome:", "chrome-extension:", "about:" ) + if frame_src in unsupported_exact: + return True + for prefix in unsupported_prefixes: + if frame_src.startswith(prefix): + return True + return False + + +# Backwards-compatible private alias kept for any external callers. +_is_unsupported_iframe_src = is_unsupported_iframe_src + + +def get_origin(url): + """Return scheme://netloc for a URL, or None when parsing fails.""" + try: + parsed = urlparse(url) + if not parsed.scheme or not parsed.netloc: + return None + return f"{parsed.scheme}://{parsed.netloc}" + except Exception: # pylint: disable=broad-except + return None def _get_origin(url): - parsed = urlparse(url) - return f"{parsed.scheme}://{parsed.netloc}" + """Compat shim: previous Feature 1 code expected a non-None string.""" + origin = get_origin(url) + return origin if origin is not None else "" + + +def clamp_frame_depth(value, default_max=DEFAULT_MAX_FRAME_DEPTH): + """Clamp a user-provided depth into [1, default_max].""" + try: + n = int(value) + except (TypeError, ValueError): + return default_max + if n < 1: + return 1 + if n > default_max: + return default_max + return n + + +def normalize_ignore_selectors(value): + """Accept str|list|None and return a clean list[str].""" + if value is None: + return [] + if isinstance(value, str): + return [value] if value.strip() else [] + if isinstance(value, (list, tuple)): + return [s for s in value if isinstance(s, str) and s.strip()] + return [] + + +def resolve_max_frame_depth(options, percy_config): + """Read maxIframeDepth from per-snapshot options or percy.config.snapshot.""" + options = options or {} + config = (percy_config or {}).get('snapshot', {}) if isinstance(percy_config, dict) else {} + raw = options.get('maxIframeDepth') + if raw is None: + raw = options.get('max_iframe_depth') + if raw is None: + raw = config.get('maxIframeDepth', DEFAULT_MAX_FRAME_DEPTH) + return clamp_frame_depth(raw) + + +def resolve_ignore_selectors(options, percy_config): + """Read ignoreIframeSelectors from per-snapshot options or percy.config.snapshot.""" + options = options or {} + config = (percy_config or {}).get('snapshot', {}) if isinstance(percy_config, dict) else {} + raw = options.get('ignoreIframeSelectors') + if raw is None: + raw = options.get('ignore_iframe_selectors') + if raw is None: + raw = config.get('ignoreIframeSelectors', []) + return normalize_ignore_selectors(raw) + + +class PercyContextLost(Exception): + """Raised when an iframe-context switch goes wrong mid-traversal. + + Carries any partial corsIframes capture already collected so the outer + caller can still emit a useful payload before bailing on the rest. + """ + def __init__(self, message, partial_capture=None): + super().__init__(message) + self.partial_capture = partial_capture or [] def get_serialized_dom(driver, cookies, percy_dom_script=None, **kwargs): # 1. Serialize the main page first (this adds the data-percy-element-ids) From ce948407f0c2ba6ff93edf4f77f743a9645f8656 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 11 May 2026 15:15:14 +0530 Subject: [PATCH 02/22] feat: nested CORS iframe capture with depth+cycle guard and recovery Implements feature parity with percy-nightwatch / percy-webdriverio for cross-origin iframe serialization: - Replaces the flat single-level iframe scan with a recursive ``process_frame_tree`` walk bounded by ``DEFAULT_MAX_FRAME_DEPTH`` (5, overridable via ``maxIframeDepth`` option or ``percy.config.snapshot.maxIframeDepth``). - Adds an ancestor-URL cycle guard so frames that link back to a previously-visited URL stop descending instead of recursing forever. - Adds an ``enumerate_iframes_script`` JS helper that runs inside the current frame context and returns metadata for every iframe (src, srcdoc, percyElementId, dataPercyIgnore, matchesIgnoreSelector, index). Nested-frame discovery now uses this script in the child context so nested-frame origin comparisons are against the *immediate* parent origin, not the page origin. - ``data-percy-ignore`` attribute opt-out: any iframe with this attribute is dropped before any switch. - ``ignoreIframeSelectors`` option (and ``ignore_iframe_selectors`` / ``percy.config.snapshot.ignoreIframeSelectors``): selectors are baked into the in-browser enumeration script so matching iframes are dropped before being processed. - Post-switch URL re-check via ``is_unsupported_iframe_src``: after switching into a frame we read ``document.URL`` and bail if the loaded document is about:blank, about:srcdoc, a net-error page, or another unsupported scheme. - ``PercyContextLost`` recovery: if ``switch_to.parent_frame()`` fails at depth > 1 we raise ``PercyContextLost`` carrying the ``partial_capture`` collected so far. The top-level walk merges that partial capture into the final ``corsIframes`` payload before aborting sibling iteration (whose enumeration was performed in a now-lost context). All per-frame serialize calls force ``enableJavaScript=True`` to bypass the standard iframe inlining path inside PercyDOM. Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 318 ++++++++++++++++++++++++++--- tests/test_snapshot.py | 453 ++++++++++++++++++++++++++++++----------- 2 files changed, 620 insertions(+), 151 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index d9ba7a9..b0d5390 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -4,7 +4,7 @@ from contextlib import contextmanager from functools import lru_cache from time import sleep -from urllib.parse import urlparse, urljoin +from urllib.parse import urlparse import requests from selenium.webdriver import __version__ as SELENIUM_VERSION @@ -150,7 +150,11 @@ def iframe_context(driver, frame_element): driver.switch_to.parent_frame() def process_frame(driver, frame_element, options, percy_dom_script): - """Processes a single cross-origin frame to capture its snapshot.""" + """Processes a single cross-origin frame to capture its snapshot. + + Kept for backwards compatibility with existing tests/callers. New code paths + (nested CORS-iframe capture) go through ``process_frame_tree``. + """ frame_url = frame_element.get_attribute('src') or "unknown-src" with iframe_context(driver, frame_element): try: @@ -177,6 +181,265 @@ def process_frame(driver, frame_element, options, percy_dom_script): } +# In-browser script that walks document.querySelectorAll('iframe') and returns +# metadata for each. Mirrors percy-nightwatch's enumerateIframesScript so the +# wire shape stays in sync. Selectors is a list[str] of CSS selectors that +# users want to opt out of CORS iframe capture for. +def enumerate_iframes_script(selectors): + selectors_json = json.dumps(list(selectors or [])) + return ( + "var __percySelectors = " + selectors_json + ";" + "var __percyIframes = document.querySelectorAll('iframe');" + "var __percyResult = [];" + "for (var i = 0; i < __percyIframes.length; i++) {" + " var f = __percyIframes[i];" + " var matchesIgnore = false;" + " if (__percySelectors && __percySelectors.length) {" + " for (var j = 0; j < __percySelectors.length; j++) {" + " try { if (f.matches(__percySelectors[j])) { matchesIgnore = true; break; } }" + " catch (e) {}" + " }" + " }" + " __percyResult.push({" + " src: f.src || ''," + " srcdoc: f.getAttribute('srcdoc')," + " percyElementId: f.getAttribute('data-percy-element-id')," + " dataPercyIgnore: f.hasAttribute('data-percy-ignore')," + " matchesIgnoreSelector: matchesIgnore," + " index: i" + " });" + "}" + "return __percyResult;" + ) + + +def _should_skip_iframe(iframe, current_origin): + """Mirror of nightwatch's shouldSkipIframe — pure on the enumerated metadata.""" + if iframe.get('dataPercyIgnore'): + log(f"Skipping iframe marked with data-percy-ignore: {iframe.get('src') or '(no src)'}", + "debug") + return True + if iframe.get('matchesIgnoreSelector'): + log(f"Skipping iframe matching ignoreIframeSelectors: " + f"{iframe.get('src') or '(no src)'}", "debug") + return True + src = iframe.get('src') or '' + if not src or is_unsupported_iframe_src(src): + if src: + log(f"Skipping unsupported iframe src: {src}", "debug") + return True + if iframe.get('srcdoc'): + log(f"Skipping srcdoc iframe at index {iframe.get('index')}", "debug") + return True + frame_origin = get_origin(src) + if not frame_origin: + log(f"Skipping iframe with invalid URL: {src}", "debug") + return True + if frame_origin == current_origin: + log(f"Skipping same-origin iframe: {src}", "debug") + return True + if not iframe.get('percyElementId'): + log(f"Skipping cross-origin iframe without data-percy-element-id: {src}", "debug") + return True + return False + + +def process_frame_tree(driver, iframe_meta, depth, ancestor_urls, ctx): + """Recursively capture a cross-origin iframe and any nested cross-origin + descendants. Bounded by ``ctx['max_frame_depth']`` to prevent runaway + recursion when pages link to each other in cycles. ``ancestor_urls`` is the + chain of frame URLs above this one — if the current frame's URL appears in + the chain we treat it as a cycle and stop descending. + """ + max_frame_depth = ctx['max_frame_depth'] + ignore_selectors = ctx['ignore_selectors'] + serialize_options = ctx['serialize_options'] + percy_dom_script = ctx['percy_dom_script'] + + if depth > max_frame_depth: + log(f"Reached max iframe nesting depth ({max_frame_depth}); " + f"stopping at {iframe_meta.get('src')}", "debug") + return [] + if ancestor_urls and iframe_meta.get('src') in ancestor_urls: + log(f"Skipping cyclic iframe ({iframe_meta.get('src')} appears in ancestor chain)", + "debug") + return [] + + collected = [] + switched_in = False + captured_error = None + + try: + log(f"Processing cross-origin iframe (depth {depth}): {iframe_meta.get('src')}", + "debug") + + # Find the iframe element by its data-percy-element-id rather than by + # numeric index, which avoids drift if the DOM mutated between + # enumeration and switch. + find_script = ( + "return document.querySelector(" + "'iframe[data-percy-element-id=\"' + arguments[0] + '\"]'" + ");" + ) + iframe_element = driver.execute_script( + find_script, iframe_meta['percyElementId'] + ) + if not iframe_element: + log(f"Could not find iframe element with data-percy-element-id: " + f"{iframe_meta['percyElementId']}", "debug") + return [] + + driver.switch_to.frame(iframe_element) + switched_in = True + + # Post-switch URL re-check: a frame's src attribute may have pointed + # somewhere reachable but the actual loaded document can be about:blank + # or a net-error page. Read document.URL inside the frame and bail if + # unsupported. + try: + inside_url = driver.execute_script("return document.URL;") + except Exception: # pylint: disable=broad-except + inside_url = None + if is_unsupported_iframe_src(inside_url): + log(f"Skipping iframe (post-switch URL unsupported): {inside_url}", "debug") + return [] + + # Inject PercyDOM and serialize. enableJavaScript is forced to True so + # that the standard iframe serialization path is bypassed — we handle + # CORS iframe serialization manually here. + driver.execute_script(percy_dom_script) + frame_options = {**serialize_options, 'enableJavaScript': True} + frame_result = driver.execute_script( + "return { snapshot: PercyDOM.serialize(" + json.dumps(frame_options) + ")," + " frameUrl: document.URL };" + ) + + if not frame_result or not frame_result.get('snapshot'): + log(f"Serialization returned empty result for frame: {iframe_meta.get('src')}", + "debug") + return [] + + frame_url = frame_result.get('frameUrl') or iframe_meta.get('src') or "unknown-src" + log(f"Captured cross-origin iframe (depth {depth}): {frame_url}", "debug") + + collected.append({ + "iframeData": {"percyElementId": iframe_meta['percyElementId']}, + "iframeSnapshot": frame_result['snapshot'], + "frameUrl": frame_url + }) + + # Look for cross-origin iframes nested inside this frame and recurse. + # Same-origin descendants are already inlined as srcdoc by + # PercyDOM.serialize above. Compare each nested-frame origin against + # this frame's origin (the immediate parent), not the page origin. + if depth < max_frame_depth: + current_origin = get_origin(frame_url) + try: + child_iframes_raw = driver.execute_script( + enumerate_iframes_script(ignore_selectors) + ) + except Exception as e: # pylint: disable=broad-except + log(f"Failed to enumerate nested iframes: {e}", "debug") + child_iframes_raw = [] + child_iframes = child_iframes_raw if isinstance(child_iframes_raw, list) else [] + next_ancestors = set(ancestor_urls or []) + next_ancestors.add(frame_url) + if iframe_meta.get('src'): + next_ancestors.add(iframe_meta['src']) + for child in child_iframes: + if _should_skip_iframe(child, current_origin): + continue + nested = process_frame_tree(driver, child, depth + 1, next_ancestors, ctx) + if nested: + collected.extend(nested) + + return collected + except PercyContextLost as err: + # Merge any partial capture from the inner level before propagating. + if err.partial_capture: + collected.extend(err.partial_capture) + err.partial_capture = collected + raise + except Exception as error: # pylint: disable=broad-except + log(f"Failed to process cross-origin iframe {iframe_meta.get('src')}: {error}", + "debug") + captured_error = error + return collected + finally: + if switched_in: + # Step up exactly one level so an outer recursion can continue from + # its own context. If parent_frame fails we have no reliable way to + # land in the correct parent — fall back to default_content and + # signal the caller to stop iterating siblings (whose enumeration + # was performed in a now-lost context). + try: + driver.switch_to.parent_frame() + except Exception as e: # pylint: disable=broad-except + log(f"Failed to switch back to parent frame: {e}", "debug") + try: + driver.switch_to.default_content() + except Exception: # pylint: disable=broad-except + pass + if depth > 1: + lost = PercyContextLost( + f"Lost parent frame context: {e}", + partial_capture=collected + ) + if captured_error is not None: + lost.__cause__ = captured_error + # pylint: disable=lost-exception + raise lost # noqa: B904 + + +def _capture_cors_iframes(driver, page_url, ctx): + """Top-level walk: enumerate page iframes, recurse into cross-origin ones.""" + try: + try: + iframe_info_raw = driver.execute_script( + enumerate_iframes_script(ctx['ignore_selectors']) + ) + except Exception as e: # pylint: disable=broad-except + log(f"Failed to enumerate top-level iframes: {e}", "debug") + return [] + iframe_info = iframe_info_raw if isinstance(iframe_info_raw, list) else [] + if not iframe_info: + return [] + + log(f"Found {len(iframe_info)} top-level iframe(s)", "debug") + page_origin = get_origin(page_url) + cors_iframes = [] + skipped = 0 + + for iframe in iframe_info: + if _should_skip_iframe(iframe, page_origin): + skipped += 1 + continue + try: + entries = process_frame_tree( + driver, iframe, 1, {page_url} if page_url else set(), ctx + ) + except PercyContextLost as err: + log("Aborting further nested CORS capture due to lost frame context", + "debug") + if err.partial_capture: + cors_iframes.extend(err.partial_capture) + break + if entries: + cors_iframes.extend(entries) + + log(f"Captured {len(cors_iframes)} cross-origin iframe(s) " + f"(top-level skipped: {skipped})", "debug") + return cors_iframes + except Exception as e: # pylint: disable=broad-except + log(f"Error capturing CORS iframes: {e}", "debug") + return [] + + +def expose_closed_shadow_roots(driver): # pylint: disable=unused-argument + """Stub — closed shadow DOM capture is added in a follow-up commit.""" + return + + # --------------------------------------------------------------------------- # Inlined SDK helpers (mirrors @percy/sdk-utils used by Node SDKs). We do not # bump a shared utils package — selenium-python ships these directly so that @@ -282,37 +545,24 @@ def __init__(self, message, partial_capture=None): super().__init__(message) self.partial_capture = partial_capture or [] -def get_serialized_dom(driver, cookies, percy_dom_script=None, **kwargs): +def get_serialized_dom(driver, cookies, percy_dom_script=None, percy_config=None, **kwargs): # 1. Serialize the main page first (this adds the data-percy-element-ids) dom_snapshot = driver.execute_script(f'return PercyDOM.serialize({json.dumps(kwargs)})') - # 2. Process CORS IFrames - try: - page_origin = _get_origin(driver.current_url) - iframes = driver.find_elements("tag name", "iframe") - if iframes and percy_dom_script: - processed_frames = [] - for frame in iframes: - frame_src = frame.get_attribute('src') - if _is_unsupported_iframe_src(frame_src): - continue - - try: - frame_origin = _get_origin(urljoin(driver.current_url, frame_src)) - except Exception as e: - log(f"Skipping iframe \"{frame_src}\": {e}", "debug") - continue - - if frame_origin == page_origin: - continue - - result = process_frame(driver, frame, kwargs, percy_dom_script) - if result: - processed_frames.append(result) - - if processed_frames: - dom_snapshot['corsIframes'] = processed_frames - except Exception as e: - log(f"Failed to process cross-origin iframes: {e}", "debug") + # 2. Process CORS iframes (nested, depth-capped, cycle-guarded, ignore-aware) + if percy_dom_script: + ctx = { + 'max_frame_depth': resolve_max_frame_depth(kwargs, percy_config), + 'ignore_selectors': resolve_ignore_selectors(kwargs, percy_config), + 'serialize_options': dict(kwargs), + 'percy_dom_script': percy_dom_script, + } + try: + page_url = driver.current_url + except Exception: # pylint: disable=broad-except + page_url = None + cors_iframes = _capture_cors_iframes(driver, page_url, ctx) + if cors_iframes: + dom_snapshot['corsIframes'] = cors_iframes dom_snapshot['cookies'] = cookies return dom_snapshot @@ -430,7 +680,8 @@ def capture_responsive_dom(driver, cookies, config, percy_dom_script=None, **kwa print(f'{width}x{height} ready, taking snapshot...') _responsive_sleep() dom_snapshot = get_serialized_dom( - driver, cookies, percy_dom_script=percy_dom_script, **kwargs) + driver, cookies, percy_dom_script=percy_dom_script, + percy_config=config, **kwargs) dom_snapshot['width'] = width print(f'Taken snapshot for width: {width}, height: {height}') dom_snapshots.append(dom_snapshot) @@ -474,7 +725,8 @@ def percy_snapshot(driver, name, **kwargs): ) else: dom_snapshot = get_serialized_dom( - driver, cookies, percy_dom_script=percy_dom_script, **kwargs) + driver, cookies, percy_dom_script=percy_dom_script, + percy_config=data['config'], **kwargs) # Post the DOM to the snapshot endpoint with snapshot options and other info response = requests.post(f'{PERCY_CLI_API}/percy/snapshot', json={**kwargs, **{ diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index 9419138..dbaa323 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -378,7 +378,12 @@ def test_posts_snapshots_to_the_local_percy_server_with_defer_and_responsive(sel self.assertEqual(httpretty.last_request().path, '/percy/snapshot') - s1 = httpretty.latest_requests()[2].parsed_body + snap_bodies = [ + r.parsed_body for r in httpretty.latest_requests() + if r.path == '/percy/snapshot' and r.method == 'POST' + and isinstance(r.parsed_body, dict) + ] + s1 = snap_bodies[0] self.assertEqual(s1['name'], 'Snapshot 1') self.assertEqual(s1['url'], 'http://localhost:8000/') self.assertEqual(s1['dom_snapshot'], expected_dom_snapshot) @@ -391,15 +396,20 @@ def test_posts_snapshots_to_the_local_percy_server_for_responsive_dom_chrome(sel driver = MockChrome.return_value # execute_script calls (reload=False): # [0] inject percy_dom [1] _setup_resize_listener [2] waitForResize - # [3] resize-check w375 [4] serialize w375 - # [5] resize-check w390 [6] serialize w390 [7] restore resize-check + # [3] resize-check w375 [4] serialize w375 [5] enumerate iframes w375 + # [6] resize-check w390 [7] serialize w390 [8] enumerate iframes w390 + # [9] restore resize-check driver.execute_script.side_effect = [ - '', '', None, 1, { 'html': 'some_dom' }, 2, { 'html': 'some_dom_1' }, 3 + '', '', None, + 1, { 'html': 'some_dom' }, [], + 2, { 'html': 'some_dom_1' }, [], + 3 ] driver.get_cookies.return_value = '' driver.execute_cdp_cmd.return_value = '' driver.get_window_size.return_value = { 'height': 400, 'width': 800 } - # Return empty iframe list so CORS-iframe code path is skipped + # find_elements is no longer used by the iframe path; left for any + # legacy callers. driver.find_elements.return_value = [] mock_logger() mock_healthcheck(widths = { "config": [375], "mobile": [390] }) @@ -425,12 +435,18 @@ def test_posts_snapshots_to_the_local_percy_server_for_responsive_dom_chrome(sel def test_has_a_backwards_compatible_function(self): mock_healthcheck() mock_snapshot() + mock_logger() percySnapshot(browser=self.driver, name='Snapshot') self.assertEqual(httpretty.last_request().path, '/percy/snapshot') - s1 = httpretty.latest_requests()[2].parsed_body + snap_bodies = [ + r.parsed_body for r in httpretty.latest_requests() + if r.path == '/percy/snapshot' and r.method == 'POST' + and isinstance(r.parsed_body, dict) + ] + s1 = snap_bodies[0] self.assertEqual(s1['name'], 'Snapshot') self.assertEqual(s1['url'], 'http://localhost:8000/') self.assertEqual(s1['dom_snapshot'], { @@ -992,29 +1008,45 @@ def test_process_frame_returns_none_on_script_injection_failure(self): self.assertIsNone(result) + # ------------------------------------------------------------------ + # Helpers for nested-frame-tree tests. The new flow drives iframe + # discovery via driver.execute_script(enumerate_iframes_script(...)) + # which returns a list of metadata dicts (no real WebElements). + # ------------------------------------------------------------------ + @staticmethod + def _meta(src, percy_id, *, srcdoc=None, ignore=False, matches=False, index=0): + return { + "src": src, + "srcdoc": srcdoc, + "percyElementId": percy_id, + "dataPercyIgnore": ignore, + "matchesIgnoreSelector": matches, + "index": index, + } + def test_get_serialized_dom_populates_cors_iframes(self): driver = Mock() + # execute_script call order: + # [0] main serialize [1] enumerate top-level iframes + # [2] querySelector (find iframe by id) + # [3] post-switch document.URL re-check + # [4] inject PercyDOM in frame + # [5] serialize the frame [6] enumerate nested iframes (empty) driver.execute_script.side_effect = [ - { - "html": '', - "resources": [{"url": "https://cdn/main.css", "content": "m"}]}, + {"html": '', + "resources": [{"url": "https://cdn/main.css", "content": "m"}]}, + [self._meta("http://main.example.com/inner", None), + self._meta("https://cross.example.com/page", "cid-1", index=1)], + Mock(name="iframe_element"), + "https://cross.example.com/page", None, - { - "html": "", - "resources": [{"url": "https://cdn/frame.css", "content": "f"}]}, + {"snapshot": {"html": "", + "resources": [{"url": "https://cdn/frame.css", "content": "f"}]}, + "frameUrl": "https://cross.example.com/page"}, + [], ] driver.current_url = "http://main.example.com/" - same_origin_frame = Mock() - same_origin_frame.get_attribute = lambda attr: ( - "http://main.example.com/inner" if attr == 'src' else None - ) - cross_origin_frame = Mock() - cross_origin_frame.get_attribute = lambda attr: ( - "https://cross.example.com/page" if attr == 'src' else "cid-1" - ) - driver.find_elements.return_value = [same_origin_frame, cross_origin_frame] - dom = local.get_serialized_dom(driver, [], percy_dom_script="some_script") self.assertIn("corsIframes", dom) @@ -1023,23 +1055,18 @@ def test_get_serialized_dom_populates_cors_iframes(self): self.assertEqual(entry["iframeData"]["percyElementId"], "cid-1") self.assertEqual(entry["iframeSnapshot"]["html"], "") self.assertEqual(entry["frameUrl"], "https://cross.example.com/page") - # HTML is left unchanged (no srcdoc injection here — core handles that) self.assertNotIn("srcdoc", dom["html"]) def test_get_serialized_dom_skips_blank_src_frames(self): """Frames with no src or src='about:blank' are not processed.""" driver = Mock() - driver.execute_script.return_value = { - "html": '' - } + driver.execute_script.side_effect = [ + {"html": ''}, + [self._meta("about:blank", None), + self._meta("", None, index=1)], + ] driver.current_url = "http://main.example.com/" - blank_frame = Mock() - blank_frame.get_attribute = lambda attr: ("about:blank" if attr == 'src' else None) - no_src_frame = Mock() - no_src_frame.get_attribute = lambda attr: (None if attr == 'src' else None) - driver.find_elements.return_value = [blank_frame, no_src_frame] - dom = local.get_serialized_dom(driver, [], percy_dom_script="some_script") self.assertNotIn("corsIframes", dom) @@ -1052,12 +1079,6 @@ def test_get_serialized_dom_no_cors_iframes_without_script(self): } driver.current_url = "http://main.example.com/" - cross_origin_frame = Mock() - cross_origin_frame.get_attribute = lambda attr: ( - "https://cross.example.com/page" if attr == 'src' else "cid-1" - ) - driver.find_elements.return_value = [cross_origin_frame] - dom = local.get_serialized_dom(driver, [], percy_dom_script=None) self.assertNotIn("corsIframes", dom) @@ -1065,71 +1086,62 @@ def test_get_serialized_dom_no_cors_iframes_without_script(self): def test_get_serialized_dom_cookies_always_attached(self): """Cookies are always added to the dom_snapshot regardless of iframes.""" driver = Mock() - driver.execute_script.return_value = {"html": ""} + driver.execute_script.side_effect = [{"html": ""}, []] driver.current_url = "http://main.example.com/" - driver.find_elements.return_value = [] cookies = [{"name": "session", "value": "abc"}] - dom = local.get_serialized_dom(driver, cookies) + dom = local.get_serialized_dom(driver, cookies, percy_dom_script="script") self.assertEqual(dom["cookies"], cookies) def test_get_serialized_dom_same_host_different_scheme_is_cross_origin(self): - """http://example.com and https://example.com differ in scheme → cross-origin. - Previously the netloc-only check would miss this; the origin-based check catches it.""" + """http://example.com vs https://example.com → cross-origin.""" driver = Mock() driver.execute_script.side_effect = [ {"html": ''}, - None, # percy_dom inject into frame - {"html": ""}, # frame serialize + [self._meta("https://main.example.com/widget", "percy-id-1")], + Mock(), + "https://main.example.com/widget", + None, + {"snapshot": {"html": ""}, "frameUrl": "https://main.example.com/widget"}, + [], ] driver.current_url = "http://main.example.com/" - # Same host, DIFFERENT scheme — should be treated as cross-origin - https_frame = Mock() - https_frame.get_attribute = lambda attr: ( - "https://main.example.com/widget" if attr == 'src' else "percy-id-1" - ) - driver.find_elements.return_value = [https_frame] - dom = local.get_serialized_dom(driver, [], percy_dom_script="script") self.assertIn("corsIframes", dom) self.assertEqual(dom["corsIframes"][0]["iframeSnapshot"]["html"], "") def test_get_serialized_dom_same_host_different_port_is_cross_origin(self): - """http://example.com:3000 and http://example.com:4000 differ in port → cross-origin.""" + """http://example.com:3000 vs http://example.com:4000 → cross-origin.""" driver = Mock() driver.execute_script.side_effect = [ {"html": ''}, + [self._meta("http://main.example.com:4000/widget", "percy-id-port")], + Mock(), + "http://main.example.com:4000/widget", None, - {"html": ""}, + {"snapshot": {"html": ""}, + "frameUrl": "http://main.example.com:4000/widget"}, + [], ] driver.current_url = "http://main.example.com:3000/" - diff_port_frame = Mock() - diff_port_frame.get_attribute = lambda attr: ( - "http://main.example.com:4000/widget" if attr == 'src' else "percy-id-port" - ) - driver.find_elements.return_value = [diff_port_frame] - dom = local.get_serialized_dom(driver, [], percy_dom_script="script") self.assertIn("corsIframes", dom) self.assertEqual(dom["corsIframes"][0]["iframeSnapshot"]["html"], "") def test_get_serialized_dom_same_origin_is_not_cross_origin(self): - """http://main.example.com/page1 and http://main.example.com/page2 share origin.""" + """Same-origin iframes are skipped before any frame switch.""" driver = Mock() - driver.execute_script.return_value = {"html": ""} + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("http://main.example.com/inner.html", "percy-id-same")], + ] driver.current_url = "http://main.example.com/" - same_origin_frame = Mock() - same_origin_frame.get_attribute = lambda attr: ( - "http://main.example.com/inner.html" if attr == 'src' else "percy-id-same" - ) - driver.find_elements.return_value = [same_origin_frame] - dom = local.get_serialized_dom(driver, [], percy_dom_script="script") self.assertNotIn("corsIframes", dom) @@ -1175,15 +1187,16 @@ def test_get_serialized_dom_corsIframes_entry_has_correct_structure(self): driver = Mock() driver.execute_script.side_effect = [ {"html": dom_html, "resources": []}, + [self._meta("https://cross.example.com/page", "cid-1")], + Mock(), + "https://cross.example.com/page", None, - {"html": '

Frame

', "resources": [frame_resource]}, + {"snapshot": {"html": '

Frame

', + "resources": [frame_resource]}, + "frameUrl": "https://cross.example.com/page"}, + [], ] driver.current_url = "http://main.example.com/" - frame = Mock() - frame.get_attribute = lambda attr: ( - "https://cross.example.com/page" if attr == 'src' else "cid-1" - ) - driver.find_elements.return_value = [frame] dom = local.get_serialized_dom(driver, [], percy_dom_script="some_script") @@ -1198,30 +1211,23 @@ def test_get_serialized_dom_multiple_cross_origin_frames(self): """All cross-origin frames are collected; same-origin frames are skipped.""" driver = Mock() driver.execute_script.side_effect = [ - { - "html": '' - '' - '' - }, - None, {"html": ""}, - None, {"html": ""}, + {"html": '' + '' + ''}, + [self._meta("https://a.other.com/w1", "pid-1"), + self._meta("http://main.example.com/inner", "pid-same", index=1), + self._meta("https://b.other.com/w2", "pid-2", index=2)], + # frame 1 + Mock(), "https://a.other.com/w1", None, + {"snapshot": {"html": ""}, "frameUrl": "https://a.other.com/w1"}, + [], + # frame 2 + Mock(), "https://b.other.com/w2", None, + {"snapshot": {"html": ""}, "frameUrl": "https://b.other.com/w2"}, + [], ] driver.current_url = "http://main.example.com/" - frame1 = Mock() - frame1.get_attribute = lambda attr: ( - "https://a.other.com/w1" if attr == 'src' else "pid-1" - ) - frame2 = Mock() - frame2.get_attribute = lambda attr: ( - "https://b.other.com/w2" if attr == 'src' else "pid-2" - ) - same_origin = Mock() - same_origin.get_attribute = lambda attr: ( - "http://main.example.com/inner" if attr == 'src' else "pid-same" - ) - driver.find_elements.return_value = [frame1, same_origin, frame2] - dom = local.get_serialized_dom(driver, [], percy_dom_script="script") self.assertIn("corsIframes", dom) @@ -1231,13 +1237,15 @@ def test_get_serialized_dom_multiple_cross_origin_frames(self): self.assertIn("pid-2", pids) self.assertNotIn("pid-same", pids) - def test_get_serialized_dom_handles_find_elements_exception(self): - """If find_elements raises, the error is swallowed, cookies are still attached, - and DOM stitching is skipped.""" + def test_get_serialized_dom_handles_enumerate_exception(self): + """If iframe enumeration raises, the error is swallowed, cookies are still + attached, and CORS iframe stitching is skipped.""" driver = Mock() - driver.execute_script.return_value = {"html": ""} + driver.execute_script.side_effect = [ + {"html": ""}, + Exception("enumerate error"), + ] driver.current_url = "http://main.example.com/" - driver.find_elements.side_effect = Exception("find error") dom = local.get_serialized_dom(driver, [{"name": "k", "value": "v"}], percy_dom_script="script") @@ -1248,34 +1256,243 @@ def test_get_serialized_dom_handles_find_elements_exception(self): def test_get_serialized_dom_process_frame_failure_is_skipped(self): """If a cross-origin frame fails to process, it is omitted and the rest succeed.""" driver = Mock() - # Calls: main serialize, fail-frame inject (raises), ok-frame inject, ok-frame serialize driver.execute_script.side_effect = [ - { - "html": '' - '' - }, - Exception("inject failed"), # fail_frame injection raises - None, # ok_frame inject - {"html": ""}, # ok_frame serialize + {"html": '' + ''}, + [self._meta("https://fail.example.com/page", "pid-fail"), + self._meta("https://ok.example.com/page", "pid-ok", index=1)], + # fail frame: querySelector returns, document.URL ok, inject raises + Mock(), "https://fail.example.com/page", Exception("inject failed"), + # ok frame + Mock(), "https://ok.example.com/page", None, + {"snapshot": {"html": ""}, "frameUrl": "https://ok.example.com/page"}, + [], + ] + driver.current_url = "http://main.example.com/" + # switch_to.frame succeeds for both; parent_frame called in finally + dom = local.get_serialized_dom(driver, [], percy_dom_script="script") + + self.assertIn("corsIframes", dom) + self.assertEqual(len(dom["corsIframes"]), 1) + self.assertEqual(dom["corsIframes"][0]["iframeData"]["percyElementId"], "pid-ok") + self.assertEqual(dom["corsIframes"][0]["iframeSnapshot"]["html"], "") + + +class TestIframeHelpers(unittest.TestCase): + """Unit tests for the inlined sdk-utils helpers.""" + + def test_is_unsupported_iframe_src_truthy_cases(self): + for src in [None, '', 'about:blank', 'about:srcdoc', 'javascript:alert(1)', + 'data:text/html;base64,', 'blob:http://foo', 'chrome://settings', + 'vbscript:msg']: + self.assertTrue(local.is_unsupported_iframe_src(src), msg=src) + + def test_is_unsupported_iframe_src_falsy_cases(self): + for src in ['http://x', 'https://x.example.com/p', 'http://x.example.com:8080/']: + self.assertFalse(local.is_unsupported_iframe_src(src), msg=src) + + def test_clamp_frame_depth_bounds(self): + self.assertEqual(local.clamp_frame_depth(0), 1) + self.assertEqual(local.clamp_frame_depth(-3), 1) + self.assertEqual(local.clamp_frame_depth(1), 1) + self.assertEqual(local.clamp_frame_depth(3), 3) + self.assertEqual(local.clamp_frame_depth(5), 5) + self.assertEqual(local.clamp_frame_depth(99), local.DEFAULT_MAX_FRAME_DEPTH) + self.assertEqual(local.clamp_frame_depth("not-a-number"), local.DEFAULT_MAX_FRAME_DEPTH) + self.assertEqual(local.clamp_frame_depth(None), local.DEFAULT_MAX_FRAME_DEPTH) + + def test_normalize_ignore_selectors(self): + self.assertEqual(local.normalize_ignore_selectors(None), []) + self.assertEqual(local.normalize_ignore_selectors(''), []) + self.assertEqual(local.normalize_ignore_selectors(' '), []) + self.assertEqual(local.normalize_ignore_selectors('.x'), ['.x']) + self.assertEqual(local.normalize_ignore_selectors(['.x', '', None, '.y']), + ['.x', '.y']) + self.assertEqual(local.normalize_ignore_selectors(123), []) + + def test_resolve_max_frame_depth_precedence(self): + # options camel takes precedence + self.assertEqual(local.resolve_max_frame_depth( + {'maxIframeDepth': 2}, {'snapshot': {'maxIframeDepth': 4}}), 2) + # options snake works + self.assertEqual(local.resolve_max_frame_depth( + {'max_iframe_depth': 3}, None), 3) + # falls back to config + self.assertEqual(local.resolve_max_frame_depth( + {}, {'snapshot': {'maxIframeDepth': 4}}), 4) + # defaults when nothing + self.assertEqual(local.resolve_max_frame_depth({}, {}), + local.DEFAULT_MAX_FRAME_DEPTH) + + def test_resolve_ignore_selectors_precedence(self): + self.assertEqual(local.resolve_ignore_selectors( + {'ignoreIframeSelectors': '.a'}, None), ['.a']) + self.assertEqual(local.resolve_ignore_selectors( + {'ignore_iframe_selectors': ['.b', '.c']}, None), ['.b', '.c']) + self.assertEqual(local.resolve_ignore_selectors( + {}, {'snapshot': {'ignoreIframeSelectors': ['.d']}}), ['.d']) + self.assertEqual(local.resolve_ignore_selectors({}, {}), []) + + def test_enumerate_iframes_script_embeds_selectors(self): + script = local.enumerate_iframes_script(['.a', '.b']) + self.assertIn('".a"', script) + self.assertIn('".b"', script) + self.assertIn('querySelectorAll(\'iframe\')', script) + self.assertIn('data-percy-element-id', script) + self.assertIn("hasAttribute('data-percy-ignore')", script) + + def test_enumerate_iframes_script_with_no_selectors(self): + script = local.enumerate_iframes_script(None) + self.assertIn('= [];', script) + + +class TestIframeTreeBehavior(unittest.TestCase): + """Tests exercising the depth cap, cycle guard, ignore attrs, and selectors.""" + + @staticmethod + def _meta(src, percy_id, *, srcdoc=None, ignore=False, matches=False, index=0): + return { + "src": src, + "srcdoc": srcdoc, + "percyElementId": percy_id, + "dataPercyIgnore": ignore, + "matchesIgnoreSelector": matches, + "index": index, + } + + def test_data_percy_ignore_attribute_skips_frame(self): + """An iframe carrying data-percy-ignore is dropped before any switch.""" + driver = Mock() + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("https://cross.example.com/p", "pid-1", ignore=True)], + ] + driver.current_url = "http://main.example.com/" + + dom = local.get_serialized_dom(driver, [], percy_dom_script="script") + self.assertNotIn("corsIframes", dom) + + def test_ignore_iframe_selectors_option_skips_matched_frame(self): + """A frame whose `matchesIgnoreSelector` was set by the enumerate + script is dropped. The selector list is forwarded into the JS.""" + driver = Mock() + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("https://cross.example.com/p", "pid-1", matches=True)], ] driver.current_url = "http://main.example.com/" - fail_frame = Mock() - fail_frame.get_attribute = lambda attr: ( - "https://fail.example.com/page" if attr == 'src' else "pid-fail" + dom = local.get_serialized_dom( + driver, [], percy_dom_script="script", + ignoreIframeSelectors=['.ad', '.tracker'] ) - ok_frame = Mock() - ok_frame.get_attribute = lambda attr: ( - "https://ok.example.com/page" if attr == 'src' else "pid-ok" + self.assertNotIn("corsIframes", dom) + + # The selectors must have been baked into the enumerate JS the SDK ran. + enumerate_call = driver.execute_script.call_args_list[1][0][0] + self.assertIn('".ad"', enumerate_call) + self.assertIn('".tracker"', enumerate_call) + + def test_post_switch_url_recheck_drops_about_blank(self): + """If document.URL inside the frame becomes unsupported (about:blank, + net-error), drop the frame and do not serialize.""" + driver = Mock() + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("https://cross.example.com/p", "pid-1")], + Mock(), # querySelector for the iframe element + "about:blank", # post-switch document.URL is unsupported + ] + driver.current_url = "http://main.example.com/" + + dom = local.get_serialized_dom(driver, [], percy_dom_script="script") + self.assertNotIn("corsIframes", dom) + # parent_frame called in finally + driver.switch_to.parent_frame.assert_called_once() + + def test_max_iframe_depth_caps_recursion(self): + """With maxIframeDepth=1, nested iframes are not entered.""" + driver = Mock() + # Top-level frame at depth 1 is captured; nested children are skipped + # because depth+1 > max. + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("https://a.example.com/", "pid-1")], + Mock(), + "https://a.example.com/", + None, + {"snapshot": {"html": ""}, "frameUrl": "https://a.example.com/"}, + ] + driver.current_url = "http://main.example.com/" + + dom = local.get_serialized_dom( + driver, [], percy_dom_script="script", maxIframeDepth=1 ) - driver.find_elements.return_value = [fail_frame, ok_frame] + self.assertEqual(len(dom["corsIframes"]), 1) + # Only 6 execute_script calls — no nested-enumerate call. + self.assertEqual(driver.execute_script.call_count, 6) + + def test_ancestor_cycle_guard_stops_descent(self): + """If a nested iframe's src appears in the ancestor chain, it is not + recursed into.""" + driver = Mock() + driver.execute_script.side_effect = [ + {"html": ""}, + # top-level + [self._meta("https://a.example.com/", "pid-a")], + # switch into a + Mock(), + "https://a.example.com/", + None, + {"snapshot": {"html": ""}, "frameUrl": "https://a.example.com/"}, + # nested enumeration inside a: cycles back to the page URL + [self._meta("http://main.example.com/", "pid-cycle")], + ] + driver.current_url = "http://main.example.com/" + + dom = local.get_serialized_dom(driver, [], percy_dom_script="script") + self.assertEqual(len(dom["corsIframes"]), 1) + self.assertEqual(dom["corsIframes"][0]["iframeData"]["percyElementId"], "pid-a") + + def test_percy_context_lost_preserves_partial_capture(self): + """If parent_frame() fails at depth > 1, raise PercyContextLost and have + the top-level walk include whatever was captured so far.""" + driver = Mock() + # First top-level frame captures successfully and recurses; the nested + # frame raises on parent_frame, which surfaces as PercyContextLost. + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("https://a.example.com/", "pid-a"), + self._meta("https://b.example.com/", "pid-b", index=1)], + # frame a + Mock(), "https://a.example.com/", None, + {"snapshot": {"html": ""}, "frameUrl": "https://a.example.com/"}, + # nested enumeration inside a + [self._meta("https://c.example.com/", "pid-c")], + # frame c (nested) + Mock(), "https://c.example.com/", None, + {"snapshot": {"html": ""}, "frameUrl": "https://c.example.com/"}, + [], + ] + driver.current_url = "http://main.example.com/" + # parent_frame fails on the *second* call (returning from c -> a). + call_count = {'n': 0} + def parent_frame_side_effect(): + call_count['n'] += 1 + if call_count['n'] == 1: + raise RuntimeError("lost context") + return None + driver.switch_to.parent_frame.side_effect = parent_frame_side_effect dom = local.get_serialized_dom(driver, [], percy_dom_script="script") self.assertIn("corsIframes", dom) - self.assertEqual(len(dom["corsIframes"]), 1) - self.assertEqual(dom["corsIframes"][0]["iframeData"]["percyElementId"], "pid-ok") - self.assertEqual(dom["corsIframes"][0]["iframeSnapshot"]["html"], "") + # The partial capture from inside frame a (a + c) must be preserved. + pids = [e["iframeData"]["percyElementId"] for e in dom["corsIframes"]] + self.assertIn("pid-a", pids) + self.assertIn("pid-c", pids) + # frame b must NOT have been processed — sibling iteration aborted. + self.assertNotIn("pid-b", pids) class TestCreateRegion(unittest.TestCase): From c97ef535a59cb42ff1087236d5fd9793643bd24a Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 11 May 2026 15:17:03 +0530 Subject: [PATCH 03/22] feat: expose closed shadow roots via CDP before serialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds ``expose_closed_shadow_roots`` (mirrored from percy-playwright's ``exposeClosedShadowRoots``) so that PercyDOM.serialize can capture closed-mode shadow DOM that ordinary DOM traversal cannot reach. Flow: 1. ``DOM.enable`` — gates non-Chromium drivers silently (Firefox/WebKit will fail this call and we no-op without touching the page). 2. ``DOM.getDocument`` with ``depth=-1, pierce=True`` — walks the full DOM tree including every shadow root. 3. Recurse the tree, collecting (host, shadowRoot) backend-node pairs for each ``shadowRootType=='closed'`` entry. Subtrees inside an iframe's ``contentDocument`` are skipped — their JS execution contexts can't see the page's WeakMap. 4. Create ``window.__percyClosedShadowRoots`` (same key PercyDOM uses). 5. For each pair, ``DOM.resolveNode`` both ends, then ``Runtime.callFunctionOn`` to stash the shadow root in the WeakMap keyed by its host element. Wired into ``percy_snapshot`` immediately after PercyDOM injection and re-primed after ``driver.refresh()`` inside ``capture_responsive_dom`` (the WeakMap is destroyed on navigation). Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 90 +++++++++++++++++++++++++++++++++++++++-- tests/test_snapshot.py | 91 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 3 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index b0d5390..4ebe6f6 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -435,9 +435,87 @@ def _capture_cors_iframes(driver, page_url, ctx): return [] -def expose_closed_shadow_roots(driver): # pylint: disable=unused-argument - """Stub — closed shadow DOM capture is added in a follow-up commit.""" - return +def expose_closed_shadow_roots(driver): + """Use CDP to find every closed shadow root in the page and stash each + {host -> shadowRoot} pair in a WeakMap on ``window``. PercyDOM.serialize + reads from that map to capture closed-mode shadow DOM that would otherwise + be invisible to ordinary DOM traversal. Non-Chromium drivers will fail the + initial CDP call and we silently no-op. + """ + if not hasattr(driver, 'execute_cdp_cmd'): + return + try: + driver.execute_cdp_cmd("DOM.enable", {}) + except Exception as e: # pylint: disable=broad-except + log(f"CDP unavailable for closed shadow DOM capture: {e}", "debug") + return + try: + doc = driver.execute_cdp_cmd( + "DOM.getDocument", {"depth": -1, "pierce": True} + ) + root = doc.get("root") if isinstance(doc, dict) else None + closed_pairs = [] + + def walk(node): + # Skip nodes inside child frame documents — cross-frame closed + # shadow roots are not yet supported (their execution context + # lacks the WeakMap). + if not isinstance(node, dict) or node.get("contentDocument"): + return + shadow_roots = node.get("shadowRoots") or [] + for sr in shadow_roots: + if sr.get("shadowRootType") == "closed": + closed_pairs.append({ + "hostBackendNodeId": node.get("backendNodeId"), + "shadowBackendNodeId": sr.get("backendNodeId") + }) + walk(sr) + for child in (node.get("children") or []): + walk(child) + + if root: + walk(root) + + if not closed_pairs: + return + + log(f"Found {len(closed_pairs)} closed shadow root(s), exposing via CDP", + "debug") + + # Create the WeakMap on the page (same key as PercyDOM looks up). + driver.execute_script( + "window.__percyClosedShadowRoots = " + "window.__percyClosedShadowRoots || new WeakMap();" + ) + + for pair in closed_pairs: + try: + host_obj = driver.execute_cdp_cmd( + "DOM.resolveNode", {"backendNodeId": pair["hostBackendNodeId"]} + ) + shadow_obj = driver.execute_cdp_cmd( + "DOM.resolveNode", {"backendNodeId": pair["shadowBackendNodeId"]} + ) + host_id = (host_obj.get("object") or {}).get("objectId") + shadow_id = (shadow_obj.get("object") or {}).get("objectId") + if not host_id or not shadow_id: + continue + driver.execute_cdp_cmd("Runtime.callFunctionOn", { + "functionDeclaration": + "function(shadowRoot) {" + " window.__percyClosedShadowRoots.set(this, shadowRoot); }", + "objectId": host_id, + "arguments": [{"objectId": shadow_id}] + }) + except Exception as e: # pylint: disable=broad-except + log(f"Failed to expose a closed shadow root via CDP: {e}", "debug") + except Exception as e: # pylint: disable=broad-except + log(f"Could not expose closed shadow roots via CDP: {e}", "debug") + finally: + try: + driver.execute_cdp_cmd("DOM.disable", {}) + except Exception: # pylint: disable=broad-except + pass # --------------------------------------------------------------------------- @@ -674,6 +752,9 @@ def capture_responsive_dom(driver, cookies, config, percy_dom_script=None, **kwa log(f'Reloading page for width: {width}', 'debug') driver.refresh() driver.execute_script(percy_dom_script) + # Re-prime closed shadow roots after the page reload — the WeakMap + # on window was destroyed when navigation happened. + expose_closed_shadow_roots(driver) _setup_resize_listener(driver) driver.execute_script("PercyDOM.waitForResize();") resize_count = 0 # Reset count because the listener just started fresh @@ -712,6 +793,9 @@ def percy_snapshot(driver, name, **kwargs): # Inject the DOM serialization script percy_dom_script = fetch_percy_dom() driver.execute_script(percy_dom_script) + # Expose closed shadow roots via CDP before serialization so PercyDOM + # can find them through the WeakMap (Chromium-only; non-Chromium no-ops). + expose_closed_shadow_roots(driver) cookies = driver.get_cookies() # Serialize and capture the DOM diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index dbaa323..99dcacb 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1495,6 +1495,97 @@ def parent_frame_side_effect(): self.assertNotIn("pid-b", pids) +class TestExposeClosedShadowRoots(unittest.TestCase): + def test_noop_on_driver_without_cdp(self): + """A driver without execute_cdp_cmd is a silent no-op.""" + class StubDriver: # no execute_cdp_cmd + pass + # Should not raise + local.expose_closed_shadow_roots(StubDriver()) + + def test_noop_when_dom_enable_fails(self): + """If DOM.enable raises (non-Chromium), we exit silently before + anything else runs.""" + driver = Mock() + driver.execute_cdp_cmd.side_effect = Exception("cdp not supported") + local.expose_closed_shadow_roots(driver) + # only the failed DOM.enable call happens + driver.execute_cdp_cmd.assert_called_once_with("DOM.enable", {}) + + def test_exposes_closed_roots_via_weakmap(self): + """When CDP returns a closed shadow root, exposeClosedShadowRoots + creates the WeakMap and calls Runtime.callFunctionOn to populate it.""" + driver = Mock() + cdp_calls = [] + def cdp(cmd, params): + cdp_calls.append(cmd) + if cmd == "DOM.enable": + return {} + if cmd == "DOM.getDocument": + return { + "root": { + "backendNodeId": 1, + "shadowRoots": [], + "children": [{ + "backendNodeId": 2, + "shadowRoots": [{ + "backendNodeId": 3, + "shadowRootType": "closed", + "children": [] + }], + }] + } + } + if cmd == "DOM.resolveNode": + if params["backendNodeId"] == 2: + return {"object": {"objectId": "host-obj"}} + if params["backendNodeId"] == 3: + return {"object": {"objectId": "shadow-obj"}} + if cmd == "Runtime.callFunctionOn": + return {} + if cmd == "DOM.disable": + return {} + return {} + driver.execute_cdp_cmd.side_effect = cdp + + local.expose_closed_shadow_roots(driver) + + # WeakMap was created on the page + scripts = [c[0][0] for c in driver.execute_script.call_args_list] + self.assertTrue(any("__percyClosedShadowRoots" in s for s in scripts)) + self.assertIn("Runtime.callFunctionOn", cdp_calls) + self.assertIn("DOM.resolveNode", cdp_calls) + + def test_skips_content_document_subtrees(self): + """Closed shadow roots inside an iframe's contentDocument are not + exposed (their JS context is separate from the page main world).""" + driver = Mock() + def cdp(cmd, params): + if cmd == "DOM.getDocument": + return { + "root": { + "backendNodeId": 1, + "children": [{ + "backendNodeId": 2, + "contentDocument": { + "backendNodeId": 100, + "shadowRoots": [{ + "backendNodeId": 101, + "shadowRootType": "closed", + }], + }, + }] + } + } + return {} + driver.execute_cdp_cmd.side_effect = cdp + local.expose_closed_shadow_roots(driver) + # No Runtime.callFunctionOn — the closed root inside contentDocument + # was skipped, and execute_script for the WeakMap was never run. + scripts = [c[0][0] for c in driver.execute_script.call_args_list] + self.assertFalse(any("__percyClosedShadowRoots" in s for s in scripts)) + + class TestCreateRegion(unittest.TestCase): def test_create_region_with_all_params(self): From 326a0ba518de531b14ba13b126785b7451f4d684 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 11 May 2026 15:39:21 +0530 Subject: [PATCH 04/22] chore: address pylint warnings on iframe + closed-shadow path Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 4 +++- tests/test_snapshot.py | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index 4ebe6f6..dac26ca 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -214,6 +214,7 @@ def enumerate_iframes_script(selectors): def _should_skip_iframe(iframe, current_origin): + # pylint: disable=too-many-return-statements """Mirror of nightwatch's shouldSkipIframe — pure on the enumerated metadata.""" if iframe.get('dataPercyIgnore'): log(f"Skipping iframe marked with data-percy-ignore: {iframe.get('src') or '(no src)'}", @@ -245,6 +246,7 @@ def _should_skip_iframe(iframe, current_origin): def process_frame_tree(driver, iframe_meta, depth, ancestor_urls, ctx): + # pylint: disable=too-many-return-statements,too-many-statements """Recursively capture a cross-origin iframe and any nested cross-origin descendants. Bounded by ``ctx['max_frame_depth']`` to prevent runaway recursion when pages link to each other in cycles. ``ancestor_urls`` is the @@ -388,7 +390,7 @@ def process_frame_tree(driver, iframe_meta, depth, ancestor_urls, ctx): if captured_error is not None: lost.__cause__ = captured_error # pylint: disable=lost-exception - raise lost # noqa: B904 + raise lost from e # noqa: B904 def _capture_cors_iframes(driver, page_url, ctx): diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index 99dcacb..e2489b0 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1481,7 +1481,6 @@ def parent_frame_side_effect(): call_count['n'] += 1 if call_count['n'] == 1: raise RuntimeError("lost context") - return None driver.switch_to.parent_frame.side_effect = parent_frame_side_effect dom = local.get_serialized_dom(driver, [], percy_dom_script="script") @@ -1498,7 +1497,8 @@ def parent_frame_side_effect(): class TestExposeClosedShadowRoots(unittest.TestCase): def test_noop_on_driver_without_cdp(self): """A driver without execute_cdp_cmd is a silent no-op.""" - class StubDriver: # no execute_cdp_cmd + class StubDriver: # pylint: disable=too-few-public-methods + # no execute_cdp_cmd pass # Should not raise local.expose_closed_shadow_roots(StubDriver()) @@ -1517,7 +1517,7 @@ def test_exposes_closed_roots_via_weakmap(self): creates the WeakMap and calls Runtime.callFunctionOn to populate it.""" driver = Mock() cdp_calls = [] - def cdp(cmd, params): + def cdp(cmd, params): # pylint: disable=too-many-return-statements cdp_calls.append(cmd) if cmd == "DOM.enable": return {} @@ -1560,7 +1560,7 @@ def test_skips_content_document_subtrees(self): """Closed shadow roots inside an iframe's contentDocument are not exposed (their JS context is separate from the page main world).""" driver = Mock() - def cdp(cmd, params): + def cdp(cmd, _params): if cmd == "DOM.getDocument": return { "root": { From 0f649b44569340673b8a1f4309b7849cc4432b4d Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Fri, 22 May 2026 17:47:52 +0530 Subject: [PATCH 05/22] Gate responsive-snapshot debug dump behind PERCY_DEBUG capture_responsive_dom was unconditionally writing output_file.json to the user's CWD on every snapshot, polluting CI workspaces. Only dump when PERCY_DEBUG is set, and swallow IO errors so a read-only CWD doesn't break capture. Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index dac26ca..b427fc2 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -768,8 +768,13 @@ def capture_responsive_dom(driver, cookies, config, percy_dom_script=None, **kwa dom_snapshot['width'] = width print(f'Taken snapshot for width: {width}, height: {height}') dom_snapshots.append(dom_snapshot) - with open("output_file.json", "w", encoding="utf-8") as file_handle: - json.dump(dom_snapshots, file_handle, indent=4) + # Optional debug dump — gated to avoid polluting the user's CWD on every CI run. + if PERCY_DEBUG: + try: + with open("output_file.json", "w", encoding="utf-8") as file_handle: + json.dump(dom_snapshots, file_handle, indent=4) + except Exception as e: # pylint: disable=broad-except + log(f"Could not write debug snapshot dump: {type(e).__name__}: {e}", "debug") change_window_dimension_and_wait(driver, current_width, current_height, resize_count + 1) return dom_snapshots From c63e01f4a0208e32140af4505935d9bac958e70b Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sun, 24 May 2026 13:27:03 +0530 Subject: [PATCH 06/22] Loosen .python-version pin to 3.10 The exact 3.10.3 patch isn't on the actions/setup-python@v6 cache that the GitHub Automatic Dependency Submission workflow uses, so submit-pypi fails on resolution. Pinning to the 3.10 line resolves to the latest cached 3.10.x without affecting our test matrix (test.yml passes python-version explicitly). Mirrors the fix applied to percy-playwright-python. Co-Authored-By: Claude Opus 4.7 (1M context) --- .python-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.python-version b/.python-version index 7d4ef04..c8cfe39 100644 --- a/.python-version +++ b/.python-version @@ -1 +1 @@ -3.10.3 +3.10 From 57abe4cd1537d4d8582a68d7ea40657f2976cfab Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sun, 24 May 2026 13:27:47 +0530 Subject: [PATCH 07/22] fix: check srcdoc before unsupported-src in _should_skip_iframe A pure-srcdoc iframe has no src attribute. The previous ordering ran the unsupported-src branch first, so pure-srcdoc iframes were silently logged as "unsupported src" instead of being routed through the srcdoc-specific branch (which is the path that downstream consumers / parity tests rely on). Reorder so srcdoc is checked first; an iframe carrying both srcdoc and a real src still takes the srcdoc path because srcdoc wins by spec. Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 10 +++++++--- tests/test_snapshot.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index b427fc2..9c6d676 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -224,14 +224,18 @@ def _should_skip_iframe(iframe, current_origin): log(f"Skipping iframe matching ignoreIframeSelectors: " f"{iframe.get('src') or '(no src)'}", "debug") return True + # Check srcdoc BEFORE the src-emptiness check: a pure-srcdoc iframe has no + # src attribute, and we want it routed through the srcdoc-specific branch + # (where same-origin inlining handles it) rather than silently lumped under + # "unsupported src". + if iframe.get('srcdoc'): + log(f"Skipping srcdoc iframe at index {iframe.get('index')}", "debug") + return True src = iframe.get('src') or '' if not src or is_unsupported_iframe_src(src): if src: log(f"Skipping unsupported iframe src: {src}", "debug") return True - if iframe.get('srcdoc'): - log(f"Skipping srcdoc iframe at index {iframe.get('index')}", "debug") - return True frame_origin = get_origin(src) if not frame_origin: log(f"Skipping iframe with invalid URL: {src}", "debug") diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index e2489b0..1edbaea 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1701,5 +1701,47 @@ def test_create_region_with_invalid_algorithm(self): self.assertEqual(result, expected_result) +class TestShouldSkipIframeOrdering(unittest.TestCase): + """Regression tests for the srcdoc-vs-unsupported-src ordering fix.""" + + def test_pure_srcdoc_iframe_with_empty_src_takes_srcdoc_branch(self): + """A pure-srcdoc iframe has empty src; the srcdoc branch must run BEFORE + the unsupported-src check so we route it the same way percy-nightwatch + does (inlined via PercyDOM.serialize).""" + with patch.object(local, 'log') as mock_log: + iframe = { + 'src': '', + 'srcdoc': '

hello

', + 'percyElementId': 'pid-srcdoc', + 'dataPercyIgnore': False, + 'matchesIgnoreSelector': False, + 'index': 3, + } + self.assertTrue(local._should_skip_iframe( + iframe, "http://main.example.com")) + # The skip reason must be the srcdoc branch, not 'unsupported src'. + messages = [c.args[0] for c in mock_log.call_args_list] + self.assertTrue(any("Skipping srcdoc iframe" in m for m in messages), + f"Expected srcdoc skip reason, got: {messages}") + self.assertFalse(any("unsupported iframe src" in m for m in messages)) + + def test_srcdoc_with_supported_src_still_takes_srcdoc_branch(self): + """If a frame carries srcdoc AND a real src, srcdoc still wins — + the inlined HTML takes precedence over the cross-origin load.""" + with patch.object(local, 'log') as mock_log: + iframe = { + 'src': 'https://cross.example.com/page', + 'srcdoc': '

doc

', + 'percyElementId': 'pid-both', + 'dataPercyIgnore': False, + 'matchesIgnoreSelector': False, + 'index': 0, + } + self.assertTrue(local._should_skip_iframe( + iframe, "http://main.example.com")) + messages = [c.args[0] for c in mock_log.call_args_list] + self.assertTrue(any("Skipping srcdoc iframe" in m for m in messages)) + + if __name__ == '__main__': unittest.main() From 2939f418fdcdc239c6e9c5faff746e49c38c15e7 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sun, 24 May 2026 13:28:22 +0530 Subject: [PATCH 08/22] fix: catch redirect-chain cycles via post-switch frameUrl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cycle guard at the top of process_frame_tree only compared iframe_meta['src'] (the static, pre-switch attribute) against ancestor_urls which was seeded with page_url (the post-resolution URL). For a redirect chain — src=A loads B via 30x; B carries src=A — the inner B never matched "A" in ancestors so the cycle wasn't caught until the second hop wasted a full serialize+enumerate round-trip. Add the resolved post-switch document.URL to the comparison so the cycle trips on whichever form happens to match the ancestor chain. Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 11 +++++++++++ tests/test_snapshot.py | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/percy/snapshot.py b/percy/snapshot.py index 9c6d676..384ba60 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -274,6 +274,9 @@ def process_frame_tree(driver, iframe_meta, depth, ancestor_urls, ctx): collected = [] switched_in = False captured_error = None + # Track the post-switch URL so we can also detect cycles where a frame's + # static src differs from its resolved document.URL (redirect chains). + inside_url = None try: log(f"Processing cross-origin iframe (depth {depth}): {iframe_meta.get('src')}", @@ -309,6 +312,14 @@ def process_frame_tree(driver, iframe_meta, depth, ancestor_urls, ctx): if is_unsupported_iframe_src(inside_url): log(f"Skipping iframe (post-switch URL unsupported): {inside_url}", "debug") return [] + # Second cycle check, on the resolved document.URL. A redirect chain + # (src=A → 30x → B) wouldn't trip the pre-switch guard because the + # static src doesn't appear in ancestor_urls — but the post-switch URL + # would. Catch the cycle here before we serialize and recurse. + if ancestor_urls and inside_url and inside_url in ancestor_urls: + log(f"Skipping cyclic iframe ({inside_url} appears in ancestor chain " + "via redirect resolution)", "debug") + return [] # Inject PercyDOM and serialize. enableJavaScript is forced to True so # that the standard iframe serialization path is bypassed — we handle diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index 1edbaea..fc32291 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1743,5 +1743,46 @@ def test_srcdoc_with_supported_src_still_takes_srcdoc_branch(self): self.assertTrue(any("Skipping srcdoc iframe" in m for m in messages)) +class TestRedirectCycleGuardOnFrameUrl(unittest.TestCase): + """A frame whose static src differs from its resolved document.URL still + has to be caught by the cycle guard.""" + + @staticmethod + def _meta(src, percy_id, *, srcdoc=None, ignore=False, matches=False, index=0): + return { + "src": src, "srcdoc": srcdoc, "percyElementId": percy_id, + "dataPercyIgnore": ignore, "matchesIgnoreSelector": matches, + "index": index, + } + + def test_redirect_chain_cycle_caught_on_frameUrl(self): + """page_url=B. Frame on page has src=A which redirects → document.URL=B. + The cycle must be detected via the post-switch frameUrl, not just src.""" + driver = Mock() + # Calls in order: + # 0: main serialize + # 1: enumerate top-level iframes + # 2: querySelector for the iframe element + # 3: post-switch document.URL → resolves to the page URL (cycle!) + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("https://a.example.com/redirector", "pid-a")], + Mock(name="iframe_element"), + "http://main.example.com/", # resolved to page URL → cycle + ] + driver.current_url = "http://main.example.com/" + + dom = local.get_serialized_dom( + driver, [], percy_dom_script="some_script" + ) + + # Frame was switched into and back out of, but never serialized. + self.assertNotIn("corsIframes", dom) + # We did the post-switch URL read (call #3 above) and then bailed — + # no PercyDOM.serialize on this frame, no nested enumeration. + self.assertEqual(driver.execute_script.call_count, 4) + driver.switch_to.parent_frame.assert_called_once() + + if __name__ == '__main__': unittest.main() From 3a0b3f5dea133bdb6982dd2722b6b0c11ddb7166 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sun, 24 May 2026 13:28:54 +0530 Subject: [PATCH 09/22] fix: surface depth==1 iframe-capture failures at info level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The broad-except at the bottom of process_frame_tree was logging every failure at debug, including top-level (depth==1) failures. That meant a user-visible iframe silently going missing produced zero visible output unless PERCY_LOGLEVEL=debug — exactly the case where users would never realize a capture was incomplete. Surface depth==1 failures at info so missing top-level iframes are visible in normal runs. Deeper nested failures stay at debug — chatty pages with many nested iframes (ad/tracker grids) can produce a lot of those. Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 6 +++- tests/test_snapshot.py | 65 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index 384ba60..cf44dc0 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -378,8 +378,12 @@ def process_frame_tree(driver, iframe_meta, depth, ancestor_urls, ctx): err.partial_capture = collected raise except Exception as error: # pylint: disable=broad-except + # Top-level (depth==1) failures mean a user-visible iframe didn't get + # captured. Surface those at info so users notice missing iframes; deeper + # nested failures stay at debug to avoid log spam in chatty pages. + failure_lvl = "info" if depth == 1 else "debug" log(f"Failed to process cross-origin iframe {iframe_meta.get('src')}: {error}", - "debug") + failure_lvl) captured_error = error return collected finally: diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index fc32291..9029fea 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1784,5 +1784,70 @@ def test_redirect_chain_cycle_caught_on_frameUrl(self): driver.switch_to.parent_frame.assert_called_once() +class TestTopLevelIframeFailureLog(unittest.TestCase): + """When a depth==1 iframe fails to process, the user should see it at + info level (not buried at debug).""" + + @staticmethod + def _meta(src, percy_id): + return { + "src": src, "srcdoc": None, "percyElementId": percy_id, + "dataPercyIgnore": False, "matchesIgnoreSelector": False, + "index": 0, + } + + def test_depth1_failure_logs_at_info(self): + driver = Mock() + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("https://cross.example.com/page", "pid-1")], + Mock(), # iframe element + "https://cross.example.com/page", # post-switch document.URL + Exception("dom inject blew up"), # injection of PercyDOM raises + ] + driver.current_url = "http://main.example.com/" + + with patch.object(local, 'log') as mock_log: + local.get_serialized_dom(driver, [], percy_dom_script="script") + + # One of the log calls must mention the failure AT info level. + failure_calls = [ + c for c in mock_log.call_args_list + if "Failed to process cross-origin iframe" in c.args[0] + ] + self.assertEqual(len(failure_calls), 1, "expected exactly one failure log") + # Second positional arg, if present, is the level. Default is 'info'. + lvl = failure_calls[0].args[1] if len(failure_calls[0].args) > 1 \ + else failure_calls[0].kwargs.get('lvl', 'info') + self.assertEqual(lvl, "info") + + def test_nested_failure_stays_at_debug(self): + """Depth > 1 failures stay at debug to avoid log spam on chatty pages.""" + driver = Mock() + driver.execute_script.side_effect = [ + {"html": ""}, + [self._meta("https://a.example.com/", "pid-a")], + Mock(), "https://a.example.com/", None, + {"snapshot": {"html": "
"}, "frameUrl": "https://a.example.com/"}, + # nested enumeration inside a returns a child + [self._meta("https://b.example.com/", "pid-b")], + # nested child blows up at injection + Mock(), "https://b.example.com/", + Exception("nested inject blew up"), + ] + driver.current_url = "http://main.example.com/" + + with patch.object(local, 'log') as mock_log: + local.get_serialized_dom(driver, [], percy_dom_script="script") + + nested_failures = [ + c for c in mock_log.call_args_list + if "Failed to process cross-origin iframe https://b.example.com" in c.args[0] + ] + self.assertEqual(len(nested_failures), 1) + lvl = nested_failures[0].args[1] if len(nested_failures[0].args) > 1 else "info" + self.assertEqual(lvl, "debug") + + if __name__ == '__main__': unittest.main() From ce8fc0045cf4be547eb96a246511f32b4362126a Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sun, 24 May 2026 13:29:34 +0530 Subject: [PATCH 10/22] fix: iterative closed-shadow walker so deep DOMs don't lose exposure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous walker was a recursive Python function. CPython's default recursion limit is ~1000; very deep DOM trees would raise RecursionError which the outer broad-except silently swallowed — so deep pages lost closed-shadow exposure with no diagnostic. Switch to an explicit stack-based walker. Memory now scales with tree breadth rather than tree depth, and deeply linear chains (3000+ nodes) complete cleanly. Add pylint disable for too-many-nested-blocks (the shape of the iterative version trips the default cap of 5). Co-Authored-By: Claude Opus 4.7 (1M context) --- percy/snapshot.py | 42 ++++++++++++++++++------------- tests/test_snapshot.py | 57 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 18 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index cf44dc0..328eb01 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -457,6 +457,7 @@ def _capture_cors_iframes(driver, page_url, ctx): def expose_closed_shadow_roots(driver): + # pylint: disable=too-many-nested-blocks """Use CDP to find every closed shadow root in the page and stash each {host -> shadowRoot} pair in a WeakMap on ``window``. PercyDOM.serialize reads from that map to capture closed-mode shadow DOM that would otherwise @@ -477,25 +478,30 @@ def expose_closed_shadow_roots(driver): root = doc.get("root") if isinstance(doc, dict) else None closed_pairs = [] - def walk(node): - # Skip nodes inside child frame documents — cross-frame closed - # shadow roots are not yet supported (their execution context - # lacks the WeakMap). - if not isinstance(node, dict) or node.get("contentDocument"): - return - shadow_roots = node.get("shadowRoots") or [] - for sr in shadow_roots: - if sr.get("shadowRootType") == "closed": - closed_pairs.append({ - "hostBackendNodeId": node.get("backendNodeId"), - "shadowBackendNodeId": sr.get("backendNodeId") - }) - walk(sr) - for child in (node.get("children") or []): - walk(child) - + # Iterative walker. Recursive Python on a very deep DOM blows past + # CPython's recursion limit (~1000) and raises RecursionError, which + # the outer broad-except would silently swallow — meaning a deep page + # would just lose closed-shadow exposure with no diagnostic. A stack + # keeps memory bounded by tree breadth instead of tree depth. if root: - walk(root) + stack = [root] + while stack: + node = stack.pop() + # Skip nodes inside child frame documents — cross-frame closed + # shadow roots are not yet supported (their execution context + # lacks the WeakMap). + if not isinstance(node, dict) or node.get("contentDocument"): + continue + shadow_roots = node.get("shadowRoots") or [] + for sr in shadow_roots: + if sr.get("shadowRootType") == "closed": + closed_pairs.append({ + "hostBackendNodeId": node.get("backendNodeId"), + "shadowBackendNodeId": sr.get("backendNodeId") + }) + stack.append(sr) + for child in (node.get("children") or []): + stack.append(child) if not closed_pairs: return diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index 9029fea..4d934da 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1849,5 +1849,62 @@ def test_nested_failure_stays_at_debug(self): self.assertEqual(lvl, "debug") +class TestIterativeShadowWalker(unittest.TestCase): + """The closed-shadow walker must handle very deep trees without + RecursionError, which the outer except would otherwise swallow.""" + + def test_deeply_nested_tree_does_not_raise_recursion_error(self): + # Build a deep linear chain well beyond CPython's default recursion + # limit (~1000). Recursive walker would RecursionError; iterative + # walker must finish cleanly. + deep = 3000 + leaf = {"backendNodeId": deep + 1, "shadowRoots": [], "children": []} + node = leaf + for i in range(deep, 0, -1): + node = { + "backendNodeId": i, + "shadowRoots": [], + "children": [node], + } + # Plant one closed shadow root deep in the tree so we can assert it + # was discovered. + cursor = node + for _ in range(deep // 2): + cursor = cursor["children"][0] + cursor["shadowRoots"] = [{ + "backendNodeId": 999999, + "shadowRootType": "closed", + "children": [], + }] + + driver = Mock() + def cdp(cmd, params): + if cmd == "DOM.enable": + return {} + if cmd == "DOM.getDocument": + return {"root": node} + if cmd == "DOM.resolveNode": + return {"object": {"objectId": f"obj-{params.get('backendNodeId')}"}} + if cmd == "Runtime.callFunctionOn": + return {} + if cmd == "DOM.disable": + return {} + return {} + driver.execute_cdp_cmd.side_effect = cdp + + # If the walker were still recursive this would silently no-op (the + # broad except swallows RecursionError). With the iterative walker we + # must actually see the WeakMap script + the Runtime.callFunctionOn + # call for the planted closed shadow root. + local.expose_closed_shadow_roots(driver) + + scripts = [c.args[0] for c in driver.execute_script.call_args_list] + self.assertTrue(any("__percyClosedShadowRoots" in s for s in scripts), + "WeakMap script must have run — walker did not find " + "the deeply nested closed shadow root") + cdp_cmds = [c.args[0] for c in driver.execute_cdp_cmd.call_args_list] + self.assertIn("Runtime.callFunctionOn", cdp_cmds) + + if __name__ == '__main__': unittest.main() From 05479a7dd8c03b698be78855e2fdfb653d719581 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sun, 24 May 2026 13:29:43 +0530 Subject: [PATCH 11/22] chore: bump @percy/cli devDep to 1.31.14 The 1.30.9 pin printed an "out of date by 10+ releases" warning on CI and appears to interact badly with the test harness on python 3.9/3.10 runs (the test process never gets past the warning banner). 1.31.14 is the current release line used across the other percy SDKs. Co-Authored-By: Claude Opus 4.7 (1M context) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index cb9e567..317ad2a 100644 --- a/package.json +++ b/package.json @@ -4,6 +4,6 @@ "test": "make test" }, "devDependencies": { - "@percy/cli": "1.30.9" + "@percy/cli": "1.31.14" } } From 9ce9c6f6c642e3eddaa26b299078799eb96c7f02 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sun, 24 May 2026 13:35:09 +0530 Subject: [PATCH 12/22] test: suppress protected-access on _should_skip_iframe class tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calling the underscore-prefixed helper directly from a regression test is intentional — the function is the boundary the fix lives at. Disable the pylint warning class-wide so the lint gate stays green. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_snapshot.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index 4d934da..e26506b 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1702,6 +1702,7 @@ def test_create_region_with_invalid_algorithm(self): class TestShouldSkipIframeOrdering(unittest.TestCase): + # pylint: disable=protected-access """Regression tests for the srcdoc-vs-unsupported-src ordering fix.""" def test_pure_srcdoc_iframe_with_empty_src_takes_srcdoc_branch(self): From c52a990c7d51ac4f0c9fea4d90faa741c6b7b86f Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Sun, 24 May 2026 17:06:01 +0530 Subject: [PATCH 13/22] =?UTF-8?q?ci:=20retrigger=20Test=20(3.9)=20+=20Test?= =?UTF-8?q?=20(3.10)=20=E2=80=94=20previous=20runner=20hung=20at=202h+?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier run (26355895088) had both Python 3.9 and 3.10 jobs stuck in_progress while Test (3.8) on the same run completed cleanly in 1m40s. Empty commit to kick a fresh CI run and supersede the hung jobs. Co-Authored-By: Claude Opus 4.7 (1M context) From 908295dd4b709d723dc1689cfb5f1467193fe14c Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 22 Jun 2026 19:27:29 +0530 Subject: [PATCH 14/22] fix: silence pylint too-many-lines in snapshot.py The CORS iframe + closed shadow DOM additions push percy/snapshot.py past pylint's 1000-line module limit (1056 lines), which fails the Lint CI job (.pylintrc fail-under=10.0). Add a module-level `# pylint: disable=too-many-lines`, matching the file's existing inline-disable style, to restore a 10.00/10 score. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- percy/snapshot.py | 1 + 1 file changed, 1 insertion(+) diff --git a/percy/snapshot.py b/percy/snapshot.py index 1bdc0d1..3e8887c 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -1,3 +1,4 @@ +# pylint: disable=too-many-lines import os import platform import json From bc3a4690155b38d58bbf7356461a3b726c6790e9 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Wed, 24 Jun 2026 12:00:17 +0530 Subject: [PATCH 15/22] ci: cap Test job at 20m and unbuffer Python output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Test (3.9)/(3.10) jobs hang on a real-headless-Firefox/geckodriver session in CI and burn the full 6h runner budget before cancellation; buffered stdout means the CI log shows nothing, hiding which call wedges. timeout-minutes fails fast, and PYTHONUNBUFFERED flushes test progress live so the hang point is visible in the next run. Passes locally (26s) — this is a CI-env-specific hang. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/test.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b67b6ef..d535b77 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,6 +15,13 @@ jobs: matrix: python: ["3.8", "3.9", "3.10"] runs-on: ubuntu-latest + # Hard cap so a wedged real-Firefox/geckodriver session fails fast instead + # of burning the full 6h runner budget (see PER-7292 CI hang investigation). + timeout-minutes: 20 + env: + # Unbuffer Python stdout so test progress (and the exact point of any + # geckodriver hang) is flushed to the CI log live, not lost on timeout. + PYTHONUNBUFFERED: "1" steps: - uses: actions-ecosystem/action-regex-match@v2 From 05386f2dd011dcbba2675ec1d49cb569a833c07d Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Wed, 24 Jun 2026 12:27:25 +0530 Subject: [PATCH 16/22] fix: gate closed-shadow CDP on Chromium to stop geckodriver CI hang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit expose_closed_shadow_roots guarded only on hasattr(driver,'execute_cdp_cmd'). In CI's Selenium the Firefox driver exposes that method too, so DOM.enable was issued over CDP to geckodriver, which HANGS indefinitely (no timeout) instead of erroring — wedging the Test (3.9)/(3.10) jobs for the full 6h runner budget. Restrict CDP to browserName=='chrome', mirroring the responsive-resize guard; closed shadow DOM via CDP is Chromium-only by design, so Firefox correctly no-ops now (without issuing any CDP command). Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- percy/snapshot.py | 14 ++++++++++++++ tests/test_snapshot.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/percy/snapshot.py b/percy/snapshot.py index 3e8887c..598ed63 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -467,6 +467,20 @@ def expose_closed_shadow_roots(driver): """ if not hasattr(driver, 'execute_cdp_cmd'): return + # CDP is Chromium-only. Some Selenium builds expose execute_cdp_cmd on the + # Firefox driver as well, but the CDP call then HANGS geckodriver + # indefinitely (no timeout) instead of failing fast — wedging CI for hours. + # Restrict to Chromium, mirroring the responsive-resize guard below + # (browserName == 'chrome'). Closed shadow DOM via CDP is Chromium-only by + # design, so non-Chromium drivers correctly no-op here. + try: + browser_name = str((driver.capabilities or {}).get('browserName', '')).lower() + except Exception: # pylint: disable=broad-except + browser_name = '' + if browser_name != 'chrome': + log("Skipping closed shadow DOM capture: CDP requires a Chromium browser", + "debug") + return try: driver.execute_cdp_cmd("DOM.enable", {}) except Exception as e: # pylint: disable=broad-except diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index 0c05f7f..b394045 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1650,15 +1650,27 @@ def test_noop_when_dom_enable_fails(self): """If DOM.enable raises (non-Chromium), we exit silently before anything else runs.""" driver = Mock() + driver.capabilities = {'browserName': 'chrome'} driver.execute_cdp_cmd.side_effect = Exception("cdp not supported") local.expose_closed_shadow_roots(driver) # only the failed DOM.enable call happens driver.execute_cdp_cmd.assert_called_once_with("DOM.enable", {}) + def test_noop_on_non_chromium_even_with_cdp_attr(self): + """Regression (PER-7292 CI hang): some Selenium builds expose + execute_cdp_cmd on the Firefox driver, but invoking CDP on geckodriver + hangs indefinitely instead of erroring. Guard on browserName so a + non-Chromium driver no-ops WITHOUT issuing any CDP command.""" + driver = Mock() + driver.capabilities = {'browserName': 'firefox'} + local.expose_closed_shadow_roots(driver) + driver.execute_cdp_cmd.assert_not_called() + def test_exposes_closed_roots_via_weakmap(self): """When CDP returns a closed shadow root, exposeClosedShadowRoots creates the WeakMap and calls Runtime.callFunctionOn to populate it.""" driver = Mock() + driver.capabilities = {'browserName': 'chrome'} cdp_calls = [] def cdp(cmd, params): # pylint: disable=too-many-return-statements cdp_calls.append(cmd) @@ -1703,6 +1715,7 @@ def test_skips_content_document_subtrees(self): """Closed shadow roots inside an iframe's contentDocument are not exposed (their JS context is separate from the page main world).""" driver = Mock() + driver.capabilities = {'browserName': 'chrome'} def cdp(cmd, _params): if cmd == "DOM.getDocument": return { @@ -2022,6 +2035,7 @@ def test_deeply_nested_tree_does_not_raise_recursion_error(self): }] driver = Mock() + driver.capabilities = {'browserName': 'chrome'} def cdp(cmd, params): if cmd == "DOM.enable": return {} From 1d2cfab2f00c2a3258bd46589bac09f16bb3fb81 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Wed, 24 Jun 2026 12:50:29 +0530 Subject: [PATCH 17/22] ci(diag): run test_snapshot with -v to name the test that hangs in CI The CDP guard fixed the expose_closed_shadow_roots hang, but Test still wedges after a responsive snapshot. unittest -v prints each test method name before it runs so the unbuffered CI log pinpoints the exact hanging test. Diagnostic; will revert once the culprit is fixed. Co-Authored-By: Claude Opus 4.8 (1M context) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8231892..ea5dbd8 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ lint: venv $(VENV)/pylint percy/* tests/* test: venv - npx percy exec --testing -- $(VENV)/python -m unittest tests.test_snapshot + npx percy exec --testing -- $(VENV)/python -m unittest -v tests.test_snapshot $(VENV)/python -m unittest tests.test_cache $(VENV)/python -m unittest tests.test_driver_metadata $(VENV)/python -m unittest tests.test_robot_library From 12a10c3a406c5db503044d3ffd478ca093d81358 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Wed, 24 Jun 2026 19:57:16 +0530 Subject: [PATCH 18/22] fix: skip resize-event poll on final window restore (geckodriver CI hang) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The responsive window-restore called change_window_dimension_and_wait, whose WebDriverWait(driver, 1) poll repeatedly runs execute_script('return window.resizeCount'). WebDriverWait's 1s bounds the polling but NOT a single hung command, and in CI that execute_script wedged geckodriver indefinitely, hanging the Test job for the full runner budget. The restore takes no snapshot after it, so the resize poll is pointless — add wait_for_resize=False to skip it. Also reverts the temporary '-v' unittest diagnostic in the Makefile. Co-Authored-By: Claude Opus 4.8 (1M context) --- Makefile | 2 +- percy/snapshot.py | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index ea5dbd8..8231892 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ lint: venv $(VENV)/pylint percy/* tests/* test: venv - npx percy exec --testing -- $(VENV)/python -m unittest -v tests.test_snapshot + npx percy exec --testing -- $(VENV)/python -m unittest tests.test_snapshot $(VENV)/python -m unittest tests.test_cache $(VENV)/python -m unittest tests.test_driver_metadata $(VENV)/python -m unittest tests.test_robot_library diff --git a/percy/snapshot.py b/percy/snapshot.py index 598ed63..2cf7692 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -839,7 +839,7 @@ def _setup_resize_listener(driver): window.addEventListener('resize', window._percyResizeHandler); """) -def change_window_dimension_and_wait(driver, width, height, resizeCount): +def change_window_dimension_and_wait(driver, width, height, resizeCount, wait_for_resize=True): try: if CDP_SUPPORT_SELENIUM and driver.capabilities['browserName'] == 'chrome': print(f'Attempting to resize using CDP for width {width} and height {height}') @@ -856,6 +856,13 @@ def change_window_dimension_and_wait(driver, width, height, resizeCount): ) #driver.execute_script(f"window.resizeTo({width}, {height});") driver.set_window_size(width, height) + # The final window-restore after the responsive loop passes + # wait_for_resize=False: no snapshot is taken after it, so polling for the + # resize event is pointless — and that poll (a tight WebDriverWait whose + # 1s timeout bounds the polling but NOT a single hung execute_script) is + # exactly what wedges geckodriver indefinitely in CI, hanging the whole job. + if not wait_for_resize: + return print(f'Resized to {width}x{height}, waiting for resize event...') try: @@ -942,7 +949,11 @@ def capture_responsive_dom(driver, cookies, config, percy_dom_script=None, **kwa json.dump(dom_snapshots, file_handle, indent=4) except Exception as e: # pylint: disable=broad-except log(f"Could not write debug snapshot dump: {type(e).__name__}: {e}", "debug") - change_window_dimension_and_wait(driver, current_width, current_height, resize_count + 1) + # Restore the original window size after capture. No snapshot follows, so + # skip the resize-event poll (wait_for_resize=False) — it serves no purpose + # here and is the call that hangs geckodriver in CI. + change_window_dimension_and_wait( + driver, current_width, current_height, resize_count + 1, wait_for_resize=False) return dom_snapshots def is_responsive_snapshot_capture(config, **kwargs): From f72e20715930441f8990b845022df035a9da015d Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 29 Jun 2026 10:31:45 +0530 Subject: [PATCH 19/22] fix: align iframe depth + unsupported-scheme list with canonical sdk-utils Bring the inlined iframe helpers into parity with the canonical @percy/sdk-utils single source of truth (percy/cli #2319): - DEFAULT_MAX_FRAME_DEPTH 5 -> 3 and add HARD_MAX_FRAME_DEPTH = 10. clamp_frame_depth now mirrors clampIframeDepth: invalid / < 1 falls back to the default (3) and the value is capped at the hard max (10), instead of conflating default and cap into a single 5. - is_unsupported_iframe_src now matches the canonical 15-prefix UNSUPPORTED_IFRAME_SRCS (adds devtools/edge/opera/view-source/file/ws/ wss/ftp) and is case-insensitive. Tests updated for the new bounds and scheme list; full suite: 96 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- percy/snapshot.py | 39 +++++++++++++++++++++------------------ tests/test_snapshot.py | 12 ++++++++---- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/percy/snapshot.py b/percy/snapshot.py index 2cf7692..fc1847f 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -566,24 +566,26 @@ def expose_closed_shadow_roots(driver): # behavior stays in sync with percy-nightwatch / percy-webdriverio. # --------------------------------------------------------------------------- -DEFAULT_MAX_FRAME_DEPTH = 5 +# Canonical iframe depth bounds, single source of truth (percy/cli #2319): +# default applies when unset/invalid, HARD_MAX is the upper clamp bound. +DEFAULT_MAX_FRAME_DEPTH = 3 +HARD_MAX_FRAME_DEPTH = 10 def is_unsupported_iframe_src(frame_src): - """True if a frame's src cannot be navigated/loaded for serialization.""" + """True if a frame's src cannot be navigated/loaded for serialization. + + Mirrors the canonical @percy/sdk-utils UNSUPPORTED_IFRAME_SRCS list + (percy/cli #2319): a missing/empty src is unsupported, and the check is a + case-insensitive startswith over the 15 canonical scheme prefixes.""" if not frame_src: return True - unsupported_exact = ("about:blank", "about:srcdoc") unsupported_prefixes = ( - "javascript:", "data:", "vbscript:", "blob:", - "chrome:", "chrome-extension:", "about:" + "about:", "chrome:", "chrome-extension:", "devtools:", "edge:", + "opera:", "view-source:", "data:", "javascript:", "blob:", + "vbscript:", "file:", "ws:", "wss:", "ftp:" ) - if frame_src in unsupported_exact: - return True - for prefix in unsupported_prefixes: - if frame_src.startswith(prefix): - return True - return False + return str(frame_src).lower().startswith(unsupported_prefixes) # Backwards-compatible private alias kept for any external callers. @@ -607,17 +609,18 @@ def _get_origin(url): return origin if origin is not None else "" -def clamp_frame_depth(value, default_max=DEFAULT_MAX_FRAME_DEPTH): - """Clamp a user-provided depth into [1, default_max].""" +def clamp_frame_depth(value, default=DEFAULT_MAX_FRAME_DEPTH, + hard_max=HARD_MAX_FRAME_DEPTH): + """Clamp a user-provided depth, mirroring @percy/sdk-utils clampIframeDepth: + an invalid or < 1 value falls back to ``default`` (3); otherwise the value + is capped at ``hard_max`` (10).""" try: n = int(value) except (TypeError, ValueError): - return default_max + return default if n < 1: - return 1 - if n > default_max: - return default_max - return n + return default + return min(n, hard_max) def normalize_ignore_selectors(value): diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index b394045..963e268 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -1427,7 +1427,9 @@ class TestIframeHelpers(unittest.TestCase): def test_is_unsupported_iframe_src_truthy_cases(self): for src in [None, '', 'about:blank', 'about:srcdoc', 'javascript:alert(1)', 'data:text/html;base64,', 'blob:http://foo', 'chrome://settings', - 'vbscript:msg']: + 'vbscript:msg', 'devtools://x', 'edge://settings', 'opera://x', + 'view-source:http://x', 'file:///etc/passwd', 'ftp://h/f', + 'ws://h/s', 'wss://h/s', 'JavaScript:alert(1)', 'FILE:///c']: self.assertTrue(local.is_unsupported_iframe_src(src), msg=src) def test_is_unsupported_iframe_src_falsy_cases(self): @@ -1435,12 +1437,14 @@ def test_is_unsupported_iframe_src_falsy_cases(self): self.assertFalse(local.is_unsupported_iframe_src(src), msg=src) def test_clamp_frame_depth_bounds(self): - self.assertEqual(local.clamp_frame_depth(0), 1) - self.assertEqual(local.clamp_frame_depth(-3), 1) + # Canonical clampIframeDepth semantics: invalid / < 1 -> default (3), + # capped at hard_max (10). + self.assertEqual(local.clamp_frame_depth(0), local.DEFAULT_MAX_FRAME_DEPTH) + self.assertEqual(local.clamp_frame_depth(-3), local.DEFAULT_MAX_FRAME_DEPTH) self.assertEqual(local.clamp_frame_depth(1), 1) self.assertEqual(local.clamp_frame_depth(3), 3) self.assertEqual(local.clamp_frame_depth(5), 5) - self.assertEqual(local.clamp_frame_depth(99), local.DEFAULT_MAX_FRAME_DEPTH) + self.assertEqual(local.clamp_frame_depth(99), local.HARD_MAX_FRAME_DEPTH) self.assertEqual(local.clamp_frame_depth("not-a-number"), local.DEFAULT_MAX_FRAME_DEPTH) self.assertEqual(local.clamp_frame_depth(None), local.DEFAULT_MAX_FRAME_DEPTH) From a975419a0c7b9ad6c1b084928f2539cd3d153713 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 29 Jun 2026 13:51:20 +0530 Subject: [PATCH 20/22] fix: remove CI timeout cap; eliminate the geckodriver resize-poll hang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert the test.yml job guard (timeout-minutes: 20 + PYTHONUNBUFFERED) and fix the underlying hang it was capping. Root cause: in change_window_dimension_and_wait the per-width resize wait polled `window.resizeCount` via execute_script inside a 1s WebDriverWait. That timeout bounds the poll *loop*, not a single execute_script that geckodriver has wedged on — so a wedged Firefox session hung the job until the runner budget ran out (the Test 3.9/3.10 lanes were cancelled at the 20m cap). Fix: replace the driver round-trip with a short fixed settle delay (RESIZE_SETTLE_SECONDS, env-tunable) between resize and snapshot. No command is issued in that window, so it cannot wedge. Drop the now-unused WebDriverWait / TimeoutException imports. Test: the responsive-dom-chrome unit test pinned execute_script returns by call position; switch it to route by script content (serialize / iframe enumeration / other) so it no longer depends on the removed poll. Full suite 96 pass; pylint 10.00/10. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/test.yml | 7 ------- percy/snapshot.py | 29 +++++++++++++++++++---------- tests/test_snapshot.py | 25 ++++++++++++++----------- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d535b77..b67b6ef 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,13 +15,6 @@ jobs: matrix: python: ["3.8", "3.9", "3.10"] runs-on: ubuntu-latest - # Hard cap so a wedged real-Firefox/geckodriver session fails fast instead - # of burning the full 6h runner budget (see PER-7292 CI hang investigation). - timeout-minutes: 20 - env: - # Unbuffer Python stdout so test progress (and the exact point of any - # geckodriver hang) is flushed to the CI log live, not lost on timeout. - PYTHONUNBUFFERED: "1" steps: - uses: actions-ecosystem/action-regex-match@v2 diff --git a/percy/snapshot.py b/percy/snapshot.py index fc1847f..cb63685 100644 --- a/percy/snapshot.py +++ b/percy/snapshot.py @@ -9,8 +9,6 @@ import requests from selenium.webdriver import __version__ as SELENIUM_VERSION -from selenium.common.exceptions import TimeoutException -from selenium.webdriver.support.ui import WebDriverWait from percy.version import __version__ as SDK_VERSION from percy.driver_metadata import DriverMetaData @@ -30,6 +28,13 @@ def _get_bool_env(key): ) PERCY_RESPONSIVE_CAPTURE_MIN_HEIGHT = _get_bool_env("PERCY_RESPONSIVE_CAPTURE_MIN_HEIGHT") PERCY_RESPONSIVE_CAPTURE_RELOAD_PAGE = _get_bool_env("PERCY_RESPONSIVE_CAPTURE_RELOAD_PAGE") +# Seconds to let the viewport reflow after a responsive resize before snapshotting. +# A fixed delay rather than a driver poll, so a wedged geckodriver can't hang the +# run (see change_window_dimension_and_wait). Tunable via env for slow pages. +try: + RESIZE_SETTLE_SECONDS = float(os.environ.get('PERCY_RESIZE_SETTLE_SECONDS') or 0.5) +except (TypeError, ValueError): + RESIZE_SETTLE_SECONDS = 0.5 # for logging LABEL = '[\u001b[35m' + ('percy:python' if PERCY_DEBUG else 'percy') + '\u001b[39m]' CDP_SUPPORT_SELENIUM = (str(SELENIUM_VERSION)[0].isdigit() and int( @@ -843,6 +848,9 @@ def _setup_resize_listener(driver): """) def change_window_dimension_and_wait(driver, width, height, resizeCount, wait_for_resize=True): + # resizeCount is retained for call-site/signature stability and the in-page + # resize listener; its value is no longer polled (see the settle note below). + # pylint: disable=unused-argument try: if CDP_SUPPORT_SELENIUM and driver.capabilities['browserName'] == 'chrome': print(f'Attempting to resize using CDP for width {width} and height {height}') @@ -866,14 +874,15 @@ def change_window_dimension_and_wait(driver, width, height, resizeCount, wait_fo # exactly what wedges geckodriver indefinitely in CI, hanging the whole job. if not wait_for_resize: return - print(f'Resized to {width}x{height}, waiting for resize event...') - - try: - WebDriverWait(driver, 1).until( - lambda driver: driver.execute_script("return window.resizeCount") == resizeCount - ) - except TimeoutException: - log(f"Timed out waiting for window resize event for width {width}", 'debug') + # Let the viewport reflow before the snapshot. This previously polled + # `window.resizeCount` via execute_script inside a 1s WebDriverWait, but that + # timeout only bounds the poll *loop* — not a single execute_script that + # geckodriver has wedged on, which hung CI jobs until the runner budget ran + # out. A short fixed settle delay issues no driver command between the resize + # and the snapshot, so it cannot wedge; we trade event-precise timing for a + # hang-proof path. + print(f'Resized to {width}x{height}, letting layout settle...') + sleep(RESIZE_SETTLE_SECONDS) def _responsive_sleep(): if not RESPONSIVE_CAPTURE_SLEEP_TIME: diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index 963e268..37e258e 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -400,17 +400,20 @@ def test_posts_snapshots_to_the_local_percy_server_with_defer_and_responsive(sel @patch.object(local, 'RESPONSIVE_CAPTURE_SLEEP_TIME', 1) def test_posts_snapshots_to_the_local_percy_server_for_responsive_dom_chrome(self, MockChrome): driver = MockChrome.return_value - # execute_script calls (reload=False): - # [0] inject percy_dom [1] _setup_resize_listener [2] waitForResize - # [3] resize-check w375 [4] serialize w375 [5] enumerate iframes w375 - # [6] resize-check w390 [7] serialize w390 [8] enumerate iframes w390 - # [9] restore resize-check - driver.execute_script.side_effect = [ - '', '', None, - 1, { 'html': 'some_dom' }, [], - 2, { 'html': 'some_dom_1' }, [], - 3 - ] + # Route execute_script by script content rather than call position: the + # resize path now uses a fixed settle delay (no `window.resizeCount` + # poll), so the exact call count/order is no longer fixed. Serialize + # returns the per-width DOMs in order; iframe enumeration returns no + # frames; everything else (inject percy_dom, waitForResize) -> ''. + _responsive_serialize = iter([{ 'html': 'some_dom' }, { 'html': 'some_dom_1' }]) + def _exec_script(script, *_args, **_kwargs): + text = script if isinstance(script, str) else '' + if 'PercyDOM.serialize' in text: + return next(_responsive_serialize) + if 'querySelectorAll' in text: + return [] + return '' + driver.execute_script.side_effect = _exec_script driver.get_cookies.return_value = '' driver.execute_cdp_cmd.return_value = '' driver.get_window_size.return_value = { 'height': 400, 'width': 800 } From ba64a28522b455646a9364f735b7f7e73373b95d Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 29 Jun 2026 14:28:57 +0530 Subject: [PATCH 21/22] fix(ci): pin selenium to 4.27.1 in test env to stop the 3.9/3.10 geckodriver hang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 3.9/3.10 Test lanes hung (cancelled at the runner budget) while 3.8 passed. Root cause: requirements.txt had an unpinned `selenium>=3`, which resolves differently by Python version — 3.8 caps at selenium 4.27.1 (selenium 4.28+ dropped Python 3.8) and passes, while 3.9/3.10 pull selenium 4.36.0 (+ urllib3 2.6, trio 0.31), whose newer webdriver connection handling wedges geckodriver on the responsive-capture test. Pin the CI/dev test env to selenium 4.27.1 so every Python lane uses the dependency set that 3.8 already passes with. This touches requirements.txt only (the Makefile installs from it); setup.py install_requires stays `selenium>=3`, so end users are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- requirements.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 9519e34..b68baa8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,6 @@ -selenium>=3 +# CI/dev test env pin only — setup.py keeps `selenium>=3` for end users. +# selenium 4.28+ drops Python 3.8, and on 3.9/3.10 its newer webdriver/urllib3 +# connection handling wedges geckodriver on the responsive test (CI hung at +# 4.36.0); 4.27.1 is the last line that runs cleanly across 3.8-3.10. +selenium==4.27.1 requests==2.* From a5ed4ce1543a4be5bc11675ba2aae11b4882cfb6 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Wed, 1 Jul 2026 10:56:06 +0530 Subject: [PATCH 22/22] chore(ci): loosen selenium test pin to a bounded range (>=4,<4.28) Replaces the exact ==4.27.1 pin so patch releases are picked up and the 4.28 boundary (the wedging line) is explicit; setup.py stays selenium>=3. --- requirements.txt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/requirements.txt b/requirements.txt index b68baa8..8d8b96b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,8 @@ -# CI/dev test env pin only — setup.py keeps `selenium>=3` for end users. -# selenium 4.28+ drops Python 3.8, and on 3.9/3.10 its newer webdriver/urllib3 -# connection handling wedges geckodriver on the responsive test (CI hung at -# 4.36.0); 4.27.1 is the last line that runs cleanly across 3.8-3.10. -selenium==4.27.1 +# CI/dev test env only — setup.py keeps `selenium>=3` for end users. This caps +# the test env below Selenium 4.28: 4.28+ drops Python 3.8, and on 3.9/3.10 its +# newer webdriver/urllib3 connection handling wedges geckodriver on the +# responsive test in CI (hung at 4.36.0). 4.27.x is the last line proven clean +# across 3.8-3.10; revisit once that wedge is root-caused so CI can track +# current Selenium. +selenium>=4,<4.28 requests==2.*