-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
[py][bidi] Route navigation commands through BiDi #17715
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ | |
| ) | ||
| from selenium.webdriver.common.bidi.browser import Browser | ||
| from selenium.webdriver.common.bidi.browsing_context import BrowsingContext | ||
| from selenium.webdriver.common.bidi.browsing_context import ReadinessState | ||
| from selenium.webdriver.common.bidi.emulation import Emulation | ||
| from selenium.webdriver.common.bidi.input import Input | ||
| from selenium.webdriver.common.bidi.network import Network | ||
|
|
@@ -509,7 +510,12 @@ def get(self, url: str) -> None: | |
| Example: | ||
| `driver.get("https://example.com")` | ||
| """ | ||
| self.execute(Command.GET, {"url": url}) | ||
| if self._is_bidi_enabled(): | ||
| self.browsing_context.navigate( | ||
| context=self.current_window_handle, url=url, wait=self._page_load_readiness() | ||
| ) | ||
| else: | ||
| self.execute(Command.GET, {"url": url}) | ||
|
|
||
| @property | ||
| def title(self) -> str: | ||
|
|
@@ -708,15 +714,44 @@ def switch_to(self) -> SwitchTo: | |
| # Navigation | ||
| def back(self) -> None: | ||
| """Goes one step backward in the browser history.""" | ||
| self.execute(Command.GO_BACK) | ||
| if self._is_bidi_enabled(): | ||
| self.browsing_context.traverse_history(context=self.current_window_handle, delta=-1) | ||
| else: | ||
| self.execute(Command.GO_BACK) | ||
|
|
||
| def forward(self) -> None: | ||
| """Goes one step forward in the browser history.""" | ||
| self.execute(Command.GO_FORWARD) | ||
| if self._is_bidi_enabled(): | ||
| self.browsing_context.traverse_history(context=self.current_window_handle, delta=1) | ||
| else: | ||
| self.execute(Command.GO_FORWARD) | ||
|
|
||
| def refresh(self) -> None: | ||
| """Refreshes the current page.""" | ||
| self.execute(Command.REFRESH) | ||
| if self._is_bidi_enabled(): | ||
| self.browsing_context.reload(context=self.current_window_handle, wait=self._page_load_readiness()) | ||
| else: | ||
| self.execute(Command.REFRESH) | ||
|
|
||
| def _is_bidi_enabled(self) -> bool: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this method is needed. Elsewhere in this class, we already use |
||
| """Returns True if WebDriver BiDi is enabled for this session. | ||
| BiDi is enabled when the remote end advertised a ``webSocketUrl`` in | ||
| the session capabilities (i.e. the user requested it via options). | ||
| """ | ||
| return bool(self.caps.get("webSocketUrl")) | ||
|
Comment on lines
+736
to
+742
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4. Websocketurl type unchecked _is_bidi_enabled() treats any truthy caps['webSocketUrl'] as BiDi-enabled, so a boolean value can incorrectly route navigation through BiDi and then fail when BiDi startup uses that value as a websocket URL. Agent Prompt
|
||
|
|
||
| def _page_load_readiness(self) -> ReadinessState: | ||
| """Maps the session's ``pageLoadStrategy`` capability to a BiDi readiness state. | ||
| ``normal`` -> COMPLETE, ``eager`` -> INTERACTIVE, ``none`` -> NONE. | ||
| Defaults to COMPLETE to match Classic ``get()`` blocking semantics. | ||
| """ | ||
| return { | ||
| "normal": ReadinessState.COMPLETE, | ||
| "eager": ReadinessState.INTERACTIVE, | ||
| "none": ReadinessState.NONE, | ||
| }.get(self.caps.get("pageLoadStrategy", "normal"), ReadinessState.COMPLETE) | ||
|
|
||
| def get_cookies(self) -> list[dict]: | ||
| """Get all cookies visible to the current WebDriver instance. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # Licensed to the Software Freedom Conservancy (SFC) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The SFC licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| """Tests that the classic navigation commands are routed through WebDriver BiDi. | ||
|
|
||
| These run under the ``--bidi`` pytest flag, which sets ``web_socket_url = True`` | ||
| on the options. With BiDi enabled, ``driver.get()``, ``back()``, ``forward()`` | ||
| and ``refresh()`` are expected to go through the BiDi ``browsingContext`` module | ||
| instead of the Classic HTTP endpoints, while preserving the same behaviour. | ||
| """ | ||
|
|
||
| from selenium.webdriver.common.bidi.browsing_context import ReadinessState | ||
|
|
||
|
|
||
| def test_bidi_is_enabled(driver): | ||
| """The driver should report BiDi as enabled when web_socket_url is set.""" | ||
| assert driver._is_bidi_enabled() is True | ||
|
|
||
|
|
||
| def test_get_navigates_via_bidi(driver, pages): | ||
| """driver.get() should load the page through BiDi and block until complete.""" | ||
| url = pages.url("simpleTest.html") | ||
| driver.get(url) | ||
| assert driver.current_url == url | ||
| assert "Hello WebDriver" in driver.title | ||
|
|
||
|
|
||
| def test_back_and_forward_via_bidi(driver, pages): | ||
| """back()/forward() should traverse history through BiDi.""" | ||
| first = pages.url("simpleTest.html") | ||
| second = pages.url("formPage.html") | ||
|
|
||
| driver.get(first) | ||
| driver.get(second) | ||
| assert driver.current_url == second | ||
|
|
||
| driver.back() | ||
| assert driver.current_url == first | ||
|
|
||
| driver.forward() | ||
| assert driver.current_url == second | ||
|
|
||
|
|
||
| def test_refresh_via_bidi(driver, pages): | ||
| """refresh() should reload the current context through BiDi.""" | ||
| url = pages.url("formPage.html") | ||
| driver.get(url) | ||
| driver.refresh() | ||
| assert driver.current_url == url | ||
|
|
||
|
|
||
| def test_page_load_readiness_default_is_complete(driver): | ||
| """With no explicit pageLoadStrategy, readiness should default to COMPLETE.""" | ||
| assert driver._page_load_readiness() == ReadinessState.COMPLETE | ||
|
Comment on lines
+29
to
+68
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. bidi_navigation_tests missing type hints New pytest test functions were added without parameter type annotations and without explicit `-> None` return annotations. This violates the requirement that new function signatures include full type annotations. Agent Prompt
|
||
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.
5. Back/forward blocking changed
π BugβΌ ReliabilityAgent Prompt
β Copy this prompt and use it to remediate the issue with your preferred AI generation tools