-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[py]: fix WebView2 and target issues related to CDP and BiDi #16140
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
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
@nxs7 Thanks for the contribution! The changes look good, have you tested this with |
Hello. Given that you're a Selenium developer, I hope it's not too bold to assume you already have Visual Studio with C# installed :) Microsoft provides several WebView2 samples, including the MicrosoftEdge/WebView2Samples repository. Please clone the repository and navigate to either After building the application, you can use the following code to reproduce the issue. from selenium import webdriver
options = webdriver.EdgeOptions()
options.use_webview = True
options.binary_location = <path_to_your_application>
driver = webdriver.Edge(options=options)
driver.start_devtools() # Crashes with "variable not associated with value" error
driver.quit() |
@nxs7 I was able to get this working on a windows machine and with your fix, the mentioned error is no longer present. But I wasn't able to get
After digging a bit, I found that the websocket connection was getting closed (this behavior was flaky), adding a few retries did solve it. Note that this happens only with Did you noticed any such issues with selenium and |
Nope, I've tested this on multiple machines with both |
It is now working consistently after a restart! Spent a good amount of time debugging the previous error :( |
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.
LGTM!
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 tiny nitpick requested... other than that LGTM.
@nxs7 please make that small change and we will merge this.
thanks!
User description
💥 What does this PR do?
This PR addresses two issues related to the CDP and BiDi functionalities in the
selenium.webdriver.remote.webdriver.WebDriver
class.WebDriver._get_cdp_details
, the current implementation does not properly account for WebView2 when determining whether the browser is Edge. This causes crashes when using CDP or BiDi features with WebView2-based applications.WebDriver.start_devtools
andWebDriver.bidi_connection
, the current implementation hard codes the attachment to the first target returned by the CDP methodTarget.getTargets
, resulting in the following two issues:Minimal reproduction code for target locating:
Case 1: Multiple pages
Case 2: Single page with service workers or iframes
🔧 Implementation Notes
To address the target locating issue, this PR iterates through the targets returned, matching the target ID with the handle of the current window.
💡 Additional Considerations
In future implementations, the
start_devtools
andbidi_connection
may receive an optional parameter specifying the window handle, enabling more precise attachment to a specific window.🔄 Types of changes
PR Type
Bug fix
Description
Fix WebView2 browser identification in CDP connection setup
Fix target selection to use current window handle instead of first target
Prevent crashes when multiple pages or service workers are present
Ensure correct target attachment for CDP and BiDi connections
Diagram Walkthrough
File Walkthrough
webdriver.py
Fix target selection and WebView2 support
py/selenium/webdriver/remote/webdriver.py
start_devtools
andbidi_connection
methods