Skip to content
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

[WebDriver] align locate_nodes/locator.py with spec #46345

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

sadym-chromium
Copy link
Contributor

@sadym-chromium sadym-chromium commented May 17, 2024

documentElement is html, which contains body: document -> html -> body. Update test to respect this fact.

@sadym-chromium sadym-chromium marked this pull request as ready for review May 21, 2024 08:58
sadym-chromium added a commit to GoogleChromeLabs/chromium-bidi that referenced this pull request May 21, 2024
…s` (#2218)

* w3c/webdriver-bidi#705
* w3c/webdriver-bidi#713
* web-platform-tests/wpt#46345

---------

Signed-off-by: Browser Automation Bot <browser-automation-bot@google.com>
Co-authored-by: Browser Automation Bot <browser-automation-bot@google.com>
@sadym-chromium sadym-chromium changed the title [WebDriver] browsingContext.locateNodes decrease maxDepth only on html elements [WebDriver] align locate_nodes/locator.py with spec Jun 4, 2024
@sadym-chromium sadym-chromium requested a review from OrKoN June 4, 2024 13:02
@OrKoN
Copy link
Contributor

OrKoN commented Jun 4, 2024

@sadym-chromium is the linked spec change relevant for the change? as far as I see the test does not deal with document or document fragment nodes.

@sadym-chromium
Copy link
Contributor Author

@sadym-chromium is the linked spec change relevant for the change? as far as I see the test does not deal with document or document fragment nodes.

Missing startNodes means the document is used:

To locate nodes using accessibility attributes ...
9. If start nodes parameter is null, append the document element of context’s active document to context nodes. Otherwise ....

@OrKoN
Copy link
Contributor

OrKoN commented Jun 4, 2024

@sadym-chromium document element is not a document

@sadym-chromium
Copy link
Contributor Author

@sadym-chromium document element is not a document

Right, but it still aligns the tests with the spec.

@sadym-chromium
Copy link
Contributor Author

@sadym-chromium document element is not a document

the document is tested in

async def test_locate_with_document_context_node(bidi_session, inline, top_context, type, value):

@sadym-chromium
Copy link
Contributor Author

@sadym-chromium document element is not a document

updated description

Copy link
Contributor

@OrKoN OrKoN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@whimboo whimboo requested a review from lutien June 5, 2024 06:59
@whimboo
Copy link
Contributor

whimboo commented Jun 5, 2024

@lutien would you mind reviewing these changes? Thanks

@sadym-chromium sadym-chromium merged commit db218f8 into master Jun 5, 2024
16 of 17 checks passed
@sadym-chromium sadym-chromium deleted the sadym/locate-nodes branch June 5, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants