Skip to content

[java] Enable bidi support for Chrome and Firefox by default #15531

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

Closed
wants to merge 1 commit into from

Conversation

shs96c
Copy link
Member

@shs96c shs96c commented Mar 28, 2025

PR Type

Enhancement


Description

  • Enabled WebDriver BiDi support by default for Chrome.

  • Enabled WebDriver BiDi support by default for Firefox.

  • Updated constructors in ChromiumOptions and FirefoxOptions to include enableBiDi calls.


Changes walkthrough 📝

Relevant files
Enhancement
ChromiumOptions.java
Enable BiDi support by default in ChromiumOptions               

java/src/org/openqa/selenium/chromium/ChromiumOptions.java

  • Added a call to enableBiDi in the constructor.
  • Ensures BiDi support is enabled by default for Chromium-based
    browsers.
  • +1/-0     
    FirefoxOptions.java
    Enable BiDi support by default in FirefoxOptions                 

    java/src/org/openqa/selenium/firefox/FirefoxOptions.java

  • Added a call to enableBiDi in the default constructor.
  • Ensures BiDi support is enabled by default for Firefox.
  • +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Potential Conflict

    The code already sets "remote.active-protocols" to 3, which appears to be related to protocol support. Verify that enabling BiDi doesn't conflict with this existing preference or require additional configuration.

    addPreference("remote.active-protocols", 3);
    enableBiDi();

    Copy link
    Contributor

    qodo-merge-pro bot commented Mar 28, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @titusfortner
    Copy link
    Member

    We need to figure out how to make this transition with limited impact on users since not all computers have socket access enabled.

    @titusfortner titusfortner added the A-needs decision TLC needs to discuss and agree label Mar 30, 2025
    @nvborisenko
    Copy link
    Member

    CDP works over WebSockets, is there any issues?

    @titusfortner
    Copy link
    Member

    Yeah, when you pass true as a capability and the remote end can't satisfy it, you can't get a session. Everything breaks and it's not obvious why.

    @@ -66,6 +66,7 @@ public FirefoxOptions() {
    // will enable it.
    // https://fxdx.dev/deprecating-cdp-support-in-firefox-embracing-the-future-with-webdriver-bidi/.
    addPreference("remote.active-protocols", 3);
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    @pujagani do you know why this preference is still set to 3 for Java, which basically means that CDP is enabled in Firefox? I thought that with #11736 all the CDP stuff was removed. Maybe it was forgotten?

    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    There was pending PR for this. Let me go sort that out. Thank you!

    @@ -66,6 +66,7 @@ public FirefoxOptions() {
    // will enable it.
    // https://fxdx.dev/deprecating-cdp-support-in-firefox-embracing-the-future-with-webdriver-bidi/.
    addPreference("remote.active-protocols", 3);
    enableBiDi();
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Would it make sense to give the user a choice to opt-out in case they really don't want to see BiDi enabled?

    @shs96c
    Copy link
    Member Author

    shs96c commented Mar 31, 2025

    Seems we don't want to do this yet. Closing.

    @shs96c shs96c closed this Mar 31, 2025
    @shs96c shs96c deleted the enable-bidi branch March 31, 2025 15:14
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    A-needs decision TLC needs to discuss and agree Review effort 1/5
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    5 participants