fix(v4): make locator screenshot masks reliable - #2557
Conversation
|
a8816f7 to
c75a4da
Compare
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
c75a4da to
e57a072
Compare
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
69d8a3c to
763479f
Compare
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Confidence score: 4/5
- In
packages/server/dom/screenshotScripts/resolveMaskRect.ts, setting the mask overlay toposition: fixedbased on the target element while still usinglocalRectcoordinates from the top-layer root can misalign fixed-element masks, so screenshots may hide or expose the wrong areas—align the coordinate space and positioning strategy (or compute fixed-element rects in viewport space) to de-risk this regression.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/server/dom/screenshotScripts/resolveMaskRect.ts">
<violation number="1" location="packages/server/dom/screenshotScripts/resolveMaskRect.ts:163">
P2: The new `position` field forces the mask overlay to `position: fixed` whenever the target's computed position is `fixed`, but the mask coordinates (`localRect`) are always computed relative to the top-layer root. A `fixed` overlay resolves `left/top` against the viewport unless the root itself is a fixed-containing block (root has a transform or `will-change: transform`). When a fixed descendant lives inside a top-layer root that has no transform, the root is not a containing block and the overlay's viewport-relative coordinates no longer match the root-relative `localRect`, so the mask is shifted by the root's on-page position. The previous always-`absolute` overlay resolved `localRect` against the root and stayed correct in that scenario, so this is a regression, not just an uncovered edge. Consider computing the overlay offset relative to the appropriate coordinate space (add the root's viewport offset when using `fixed` positioning on a non-transformed root), or only selecting `fixed` when the root actually remains a fixed-containing block after restore.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
## Summary - port 23 public-SDK integration specs from main, growing the v4 suite from 5 to 28 files - adapt legacy cases to hermetic v4 fixtures and public APIs - define ten stable local semantic groups with complete, unique test ownership - leave FlowLogger coverage retired because FlowLogger is absent from both v4 implementations ## Scope boundary This PR is mechanical test transfer only. It has no packages/server production changes, generated extension update, or .github/workflows diff from v4-spike. Three transferred specs that expose runtime gaps travel with their standalone fixes: - operation timeouts: #2555 - well-formed snapshot Unicode: #2556 - locator screenshot masks: #2557 Together the stack grows the suite to 31 files. CI orchestration and external-contributor approval policy remain isolated in #2553. The remaining main integration specs are not silently omitted: Agent/streaming/cache cases require the v4 Agent surface; Browserbase/CDP lifecycle, downloads, OOPIF, and connection cases require dedicated browser infrastructure; logger and FlowLogger cases target lifecycle code removed from v4. ## Verification - integration discovery and ownership tests: 12 passed - focused default-page and screenshot-adjacent integration run: 9 passed before extraction - GitHub semantic matrix is the authoritative full-suite validation after this history rewrite ## Review order #2551 → #2555 → #2556 → #2557 → #2553
Supports deep-locator screenshot masks, preserves all matches unless nth() narrows them, fixes top-layer mask geometry, and performs the paint delay outside page-controlled timers. Includes the transferred screenshot integration coverage.\n\nStacked on the Unicode snapshot PR.
Summary by cubic
Make
Page.screenshotmasks reliable across frames and Chromium’s transformed top-layer content. Supports deep-locator masks and stabilizes geometry and timing for consistent captures; adds tests.New Features
maskacceptsDeepLocatorDelegate(including iframe hops); keeps all matches by default, narrowed withnth().Bug Fixes
Written for commit 2f3eb6d. Summary will update on new commits.