-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[fix]: .count() not working on xpaths with attribute predicates
#1679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ute predicates countXPathMatchesMainWorld() used a custom XPath parser that only handled simple tag/index steps, failing on attribute predicates like `//img[@alt='Stagehand']`. resolveXPathMainWorld() already had a native document.evaluate() fallback that handled these correctly, which is why isVisible() worked while count() returned 0. Add the same native XPath fallback to countXPathMatchesMainWorld(), falling through to the composed DOM traversal only when native evaluation fails (e.g. shadow DOM). Fixes #1668 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 3d31617 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Greptile OverviewGreptile SummaryFixes The custom XPath parser only recognized numeric positional indices Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Locator as locator.count()
participant Resolver as FrameSelectorResolver
participant CountFn as countXPathMatchesMainWorld()
participant NativeXPath as document.evaluate()
participant Fallback as Custom Traversal
Locator->>Resolver: count(xpath query)
Resolver->>CountFn: countXPathMatchesMainWorld(xpath)
CountFn->>NativeXPath: Try document.evaluate(..., ORDERED_NODE_SNAPSHOT_TYPE)
alt Native XPath succeeds
NativeXPath-->>CountFn: Return snapshotLength
CountFn-->>Resolver: Return count
else Native XPath throws (shadow DOM)
NativeXPath--xCountFn: throw error
CountFn->>Fallback: Parse steps & traverse composed DOM
Fallback-->>CountFn: Return count from traversal
CountFn-->>Resolver: Return count
end
Resolver-->>Locator: Return final count
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 file reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file
Confidence score: 3/5
- Risk is moderate because the primary
document.evaluate()counting path can undercount matches on pages with shadow DOM, which is user-visible behavior. - The issue is localized to
packages/core/lib/v3/dom/locatorScripts/counts.ts, but it affects correctness of XPath count results in shadow DOM contexts. - Pay close attention to
packages/core/lib/v3/dom/locatorScripts/counts.ts- counting viadocument.evaluate()may miss matches in shadow DOM.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/dom/locatorScripts/counts.ts">
<violation number="1" location="packages/core/lib/v3/dom/locatorScripts/counts.ts:305">
P1: Native `document.evaluate()` as the primary path silently undercounts on pages with shadow DOM. Unlike `resolveXPathMainWorld()` (which only needs one match), counting requires finding ALL matches — but `document.evaluate()` doesn't throw when shadow DOM exists, it just ignores elements inside shadow roots. This regresses simple XPaths like `//div` on shadow DOM pages by returning only the light DOM count and skipping the composed traversal.
Consider detecting whether the page uses shadow DOM before taking the fast path, or restructuring so `document.evaluate()` is only attempted when the step parser fails (e.g., for attribute predicates the regex can't parse).</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User/Test Code
participant Locator as Page Locator API
participant CountScript as countXPathMatchesMainWorld
participant NativeDOM as Browser (document.evaluate)
participant CustomParser as Custom XPath Parser
participant ComposedDOM as Composed DOM Traversal
Note over User,ComposedDOM: XPath Count Request (e.g., //img[@alt='Stagehand'])
User->>Locator: count()
Locator->>CountScript: Execute script in browser context
rect rgb(23, 37, 84)
Note right of CountScript: NEW: Native Fallback Strategy
CountScript->>NativeDOM: document.evaluate(xpath, ORDERED_NODE_SNAPSHOT_TYPE)
alt Native Success (Standard DOM)
NativeDOM-->>CountScript: XPathResult (snapshotLength)
CountScript-->>Locator: count
else Native Error (e.g., Shadow DOM Boundary)
NativeDOM-->>CountScript: Throws Error
Note over CountScript,ComposedDOM: Fallback to Composed Traversal
CountScript->>CustomParser: parseSteps(xpath)
Note right of CustomParser: Uses Regex: /^(.*?)(\[(\d+)\])?$/
CustomParser-->>CountScript: parsed steps
CountScript->>ComposedDOM: Walk tree matching tags/indices
ComposedDOM-->>CountScript: total matches
CountScript-->>Locator: count
end
end
Locator-->>User: integer count
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Native document.evaluate() silently ignores shadow DOM elements rather than throwing, so using it as the primary path undercounts on pages with shadow roots. Move it to a fallback that only triggers when the custom composed traversal returns 0 matches at a step (indicating the custom parser likely can't handle the XPath syntax, e.g. attribute predicates). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/dom/locatorScripts/counts.ts">
<violation number="1" location="packages/core/lib/v3/dom/locatorScripts/counts.ts:441">
P2: The native `document.evaluate()` fallback is placed inside the step loop as a last resort, but `resolveXPathMainWorld` in `selectors.ts` (the `isVisible()` path) tries native XPath **first**, before the custom parser. This inverted order means `count()` and `isVisible()` can disagree: `count()` now traverses shadow DOM (custom parser runs first) while `isVisible()` uses native XPath (which skips shadow DOM). For consistency and correctness, consider trying native `document.evaluate()` first (as the removed code did, and as sibling functions do), then falling through to the custom parser when native returns 0 or throws — that way the custom parser provides shadow-DOM counts as a superset.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…ests Extract the duplicated parseSteps function from counts.ts and selectors.ts into a shared xpathParser module with proper attribute predicate parsing. The custom parser now handles [@attr='value'] predicates natively in the composed DOM traversal, so attribute predicate XPaths work correctly even on pages with shadow DOM. This removes the need for the native document.evaluate() fallback that was added in the previous commit. Adds 23 vitest unit tests covering: - Basic tag parsing and case normalization - Child vs descendant axes - Positional indices - Attribute predicates (single/double quotes, multiple, combined with index) - Edge cases (empty input, xpath= prefix, whitespace) - Element matching logic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the third inline copy of the XPath step parser in the shadow piercer with imports from the shared xpathParser module. The piercer's resolveSimpleXPath now gets attribute predicate support for free. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lector The step splitter now tracks bracket depth so `/` inside predicates (e.g. `[@href='/api/endpoint']`) no longer incorrectly splits the step. This was a pre-existing bug in all copies of the parser; waitForSelector.ts had independently fixed it but the other copies hadn't. Also deduplicates the fourth and final copy of the XPath parser from waitForSelector.ts, replacing its local XPathStep type, parseXPathSteps, and inline matching logic with imports from the shared xpathParser module. Adds 2 unit tests for forward slashes in attribute values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 7 files
Confidence score: 4/5
- A moderate parsing edge case was found in
packages/core/lib/v3/dom/locatorScripts/xpathParser.ts: the predicate regex can drop attribute filters when]appears inside quoted values, which could affect some XPath queries. - This is a single medium-severity issue (5/10) with limited scope, so overall risk looks low and likely safe to merge with awareness.
- Pay close attention to
packages/core/lib/v3/dom/locatorScripts/xpathParser.ts- predicate extraction may silently skip filters for quoted values containing].
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/lib/v3/dom/locatorScripts/xpathParser.ts">
<violation number="1" location="packages/core/lib/v3/dom/locatorScripts/xpathParser.ts:97">
P2: The predicate extraction regex `[^\]]*` does not handle `]` inside quoted attribute values, silently dropping the attribute filter. This is inconsistent with the step splitter which already tracks bracket depth.
Consider replacing the simple regex with a bracket-depth–aware scan (similar to the step splitter logic), or at minimum add a comment/test documenting this limitation.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Node.js Core
participant Script as Browser Script (Main World)
participant Parser as NEW: Consolidated XPath Parser
participant DOM as Browser DOM (Composed Tree)
Note over Client,DOM: Runtime Flow for count(), resolve(), or waitForSelector()
Client->>Script: execute(xpath)
Script->>Parser: NEW: parseXPathSteps(xpath)
Note over Parser: Handles //tag, [index], and<br/>NEW: [@attr='val'] predicates
Parser->>Parser: CHANGED: Split steps respecting brackets
Parser-->>Script: XPathStep[]
loop For each XPathStep
Script->>DOM: Get composed children/descendants
Note right of DOM: Accesses elements across<br/>Shadow DOM boundaries
DOM-->>Script: candidate elements
loop Each candidate element
Script->>Parser: NEW: elementMatchesStep(el, step)
Parser->>Parser: Match tag (or wildcard)
opt Step has attributes
Parser->>Parser: NEW: Verify all [@attr='val'] matches
end
Parser-->>Script: boolean match
end
alt step.index is defined
Script->>Script: Filter by positional index [n]
else no index
Script->>Script: Accumulate all matches
end
end
alt Elements found
Script-->>Client: return count / element
else No matches
Script-->>Client: return 0 / null
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
The predicate extraction regex `[^\]]*` stopped at the first `]`, breaking attribute values containing brackets (e.g. `[@title='array[0]']`). Replace with a quote-aware character scan that only treats `]` as a predicate terminator when it's outside quoted strings. Adds 2 unit tests for bracket-containing attribute values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
.count() not working on xpaths with attribute predicates
|
@seanmcguire12 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 8 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant User as Test/User Script
participant Runner as Locator Runner (counts/selectors/wait)
participant Parser as Shared XPath Parser (xpathParser.ts)
participant DOM as Browser DOM (Composed Tree)
Note over Runner, Parser: Consolidated Parser Logic
User->>Runner: locator("//img[@alt='Stagehand']").count()
Runner->>Parser: NEW: parseXPathSteps(rawXpath)
Parser->>Parser: Handle 'xpath=' prefix
Parser->>Parser: NEW: Split steps by axis (/ or //) while respecting bracket depth
Parser->>Parser: NEW: Extract tag, index [n], and multiple [@attr='val'] predicates
Parser-->>Runner: Return XPathStep[] (metadata)
loop For each step in XPath
Runner->>DOM: Get composed children/descendants
DOM-->>Runner: elementPool[]
loop For each candidate element
Runner->>Parser: NEW: elementMatchesStep(element, step)
Parser->>DOM: element.localName
Parser->>DOM: CHANGED: element.getAttribute(name)
Note right of Parser: Matches tag name AND all attribute predicates
Parser-->>Runner: boolean match result
end
opt step.index != null
Runner->>Runner: Filter by positional index [n]
end
Note over Runner: Update current context to matched elements
end
Runner-->>User: Return final count (e.g., 3)
alt Native Evaluation (resolveXPath only)
Runner->>DOM: document.evaluate(xpath)
DOM-->>Runner: Result
Note over Runner, DOM: Native XPath fails to see into Shadow DOM
else NEW Fallback: Composed Tree Traversal
Note over Runner, DOM: Uses the Shared Parser logic described above
end
Summary
locator.count()returning 0 for XPath selectors containing attribute predicates (e.g.//img[@alt='Stagehand'])Root Cause
Stagehand has two ways to evaluate XPath expressions:
document.evaluate()) — understands the full XPath spec, including attribute filters like[@alt='Stagehand'], but cannot see into shadow DOM//divor//div[2], not attribute predicatesThe
isVisible()code path (viaresolveXPathMainWorld) tried native browser XPath first and fell back to the custom parser. But thecount()code path (countXPathMatchesMainWorld) went straight to the custom parser. The custom parser's regex only recognized numeric positional indices ([1],[2]), so[@alt='Stagehand']was treated as part of the tag name, matching nothing.Fix
1. Extract shared parser (
xpathParser.ts)The XPath step parser was duplicated four times across
counts.ts,selectors.ts,piercer.runtime.ts, andwaitForSelector.ts. All four are consolidated into a singlexpathParser.tsmodule exportingparseXPathSteps()andelementMatchesStep().2. Add attribute predicate parsing
The parser now handles
[@attr='value']predicates (single and double quotes, multiple predicates, combined with positional indices). Attribute predicate XPaths work correctly including inside shadow DOM.3. Fix forward slash splitting in predicates
The step splitter now tracks bracket depth so
/inside attribute values (e.g.[@href='/api/endpoint']) no longer incorrectly splits the step. This was a pre-existing bug in three of the four copies;waitForSelector.tshad independently fixed it.4. Document supported XPath subset
The parser docstring explicitly lists what's supported and what isn't, so future contributors know the scope.
Files changed
xpathParser.ts(new) — shared parser withparseXPathSteps()andelementMatchesStep()counts.ts— uses shared parser (removed ~40 lines of inline parser)selectors.ts— uses shared parser (removed ~40 lines of inline parser)piercer.runtime.ts— uses shared parser (removed ~40 lines of inline parser)waitForSelector.ts— uses shared parser (removed ~50 lines of inline parser + matching)global.d.ts— updated docstringxpath-parser.test.ts(new) — 25 vitest unit testsTest plan
Unit tests (25 vitest tests in
xpath-parser.test.ts)[@href='/api/endpoint'])[@data-url='http://example.com/path/to/page'])xpath=prefix, whitespaceIntegration tests (verified manually)
page.locator("//img[@alt='Stagehand']").count()returns correct count (was 0, now 3) on https://www.stagehand.dev/page.locator("//img[@alt='Stagehand']").isVisible()still returnstrue//divreturns correct count//divon page with shadow DOM counts all elements (light + shadow = 6)//div[@class='test-item']on page with shadow DOM counts all elements (light + shadow = 5)Fixes #1668
🤖 Generated with Claude Code