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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Jun 5, 2024
Merged

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

merged 2 commits into from
Jun 5, 2024

Conversation

sadym-chromium
Copy link
Contributor

@sadym-chromium sadym-chromium commented May 17, 2024 •
edited
Loading

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

@sadym-chromium sadym-chromium mentioned this pull request May 17, 2024
Merged
@sadym-chromium sadym-chromium marked this pull request as ready for review May 21, 2024 08:58
This was referenced May 21, 2024
Closed
Merged
sadym-chromium added a commit to GoogleChromeLabs/chromium-bidi that referenced this pull request May 21, 2024
@sadym-chromium @browser-automation-bot
feat: support document as startNodes in `browsingContext.locateNode…
ad7318f 
…s` (#2218)

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

---------

Signed-off-by: Browser Automation Bot <[email protected]>
Co-authored-by: Browser Automation Bot <[email protected]>
@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

wpt/webdriver/tests/bidi/browsing_context/locate_nodes/start_nodes.py

Line 244 in 0a17913

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

OrKoN
OrKoN approved these changes Jun 4, 2024
View reviewed changes
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
Reviewers

@lutien lutien lutien approved these changes

@OrKoN OrKoN OrKoN approved these changes

@AutomatedTester AutomatedTester Awaiting requested review from AutomatedTester

@jgraham jgraham Awaiting requested review from jgraham

@jrandolf-2 jrandolf-2 Awaiting requested review from jrandolf-2

@juliandescottes juliandescottes Awaiting requested review from juliandescottes

@shs96c shs96c Awaiting requested review from shs96c

@whimboo whimboo Awaiting requested review from whimboo

Assignees

@whimboo whimboo

@OrKoN OrKoN

Labels
webdriver-bidi
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sadym-chromium @OrKoN @whimboo @lutien @wpt-pr-bot @moz-wptsync-bot