Repository navigation
fix(extension): count XPath positions among the siblings of each parent - #3096
Open
mikhail-koviazin wants to merge 2 commits into
Open
mikhail-koviazin wants to merge 2 commits into
mikhail-koviazin wants to merge 2 commits into
Conversation
The composed-tree parser runs whenever the document contains a shadow root, and for a descendant step it applied a positional predicate to the flat list of every match, so `//div[2]` behaved like `(//div)[2]`. It returned one element where document.evaluate() returns one per parent, and with a preceding predicate it returned the wrong element. - a descendant step now filters candidates within their own sibling group and keeps document order; child steps already pooled the children of a single parent - the same grouping applies to the first step of the shadow-hop resolver - regression coverage runs over http, since data: URLs stay on the native engine
XPath positions start at 1, so `//div[0]` selects nothing. The composed-tree parser clamped the index to 1 and returned the first match instead. - `[0]` is now parsed as written and yields an empty set, as document.evaluate() does
🦋 Changeset detectedLatest commit: e5510e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 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 |
Contributor
|
This PR is from an external contributor and must be approved by a stagehand team member with write access before CI can run. |
github-actions
Bot
added
external-contributor
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
why
Part of #2693, the two positional items from its "Where it comes from" list.
When the document contains a shadow root, XPath locators go through the composed-tree parser instead of
document.evaluate(). For a//step that parser collects every matching descendant into one list and then applies[n]to the list, so//div[2]is evaluated as(//div)[2]. In XPath the position counts along the child axis://div[2]is every div that is the second div child of its parent.Separately,
[0]is clamped to 1 (Math.max(1, Number(input))), so a predicate that selects nothing in XPath returns the first match.Measured on a build of
main(50f3ab81), fixture served over http. Without the shadow root every row agrees with native. With one unrelatedattachShadow()call on the page:document.evaluate()//div[2]//div[1]//section//div[2]//div[@class='x'][2]B2//div[0]A1//section[@id='s1']/div[2]Fixture:
The
[@class='x'][2]row is the one I'd worry about. The count is 1 on both sides, so nothing looks off, but it is a different element, and a click lands on it without an error.what changed
applyPredicatesPerParentinxpathParser.tsgroups the candidates of a step byparentNode, applies the predicates inside each group and returns the survivors in the order they came in. Steps without a positional predicate skip the grouping.xpathResolver.tsuses it for//steps in bothresolveXPathComposedMatchesandresolveStagehandShadowHopMatches. Child steps keep callingapplyPredicates: their pool is already the children of one node.[0]is parsed as written and matches nothing.Not in this PR:
position(),last()and(//div)[n]from the same issue are still unsupported.test plan
packages/extension/tests/xpath-positional-predicates.test.ts(new, unit): per-parent counting, a position after another predicate and before it,[0], and that order is kept when candidates from different parents are interleaved.packages/sdk-ts/tests/integration/locatorXPathPositionalPredicates.test.ts(new, integration): the table above against real Chrome, plus//em[2]inside the shadow root to check that piercing still works. Served over http, becausedata:URLs stay on the native engine and never reach this parser. Registered inscripts/test-integration.ts.On
mainthe integration test fails withexpected 1 to be 3and with the wrapper div's HTML whereB2is expected. With the change it passes, and so dolocatorXPathTextPredicates,locatorCount,locatorNthandwaitForSelector, each run as its own file.Summary by cubic
Fixes XPath positional predicates in the composed-tree parser so
//div[2]counts among the siblings of each parent, matchingdocument.evaluate(), and[0]now selects nothing instead of clamping to the first match.Written for commit e5510e5. Summary will update on new commits.