Skip to content

[py][bidi]: add note for enable_webextensions = False#15981

Merged
navin772 merged 3 commits intoSeleniumHQ:trunkfrom
navin772:py-add-note-webex
Jun 30, 2025
Merged

[py][bidi]: add note for enable_webextensions = False#15981
navin772 merged 3 commits intoSeleniumHQ:trunkfrom
navin772:py-add-note-webex

Conversation

@navin772
Copy link
Member

@navin772 navin772 commented Jun 30, 2025 •
edited by qodo-code-review bot
Loading

User description

🔗 Related Issues

Addresses the concerns mentioned in - #15794 (comment)

💥 What does this PR do?

Adds a relevant note when enable_webextensions is set to False.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Documentation

Description

  • Add clarification note for enable_webextensions = False behavior

  • Document that disabling removes manually added flags

  • Fix indentation in existing documentation

Changes diagram

flowchart LR
  A["enable_webextensions = False"] --> B["Removes flags automatically"]
  B --> C["Even manually added flags"]
  C --> D["Updated documentation"]
Loading

Changes walkthrough 📝

Relevant files
Documentation
options.py
Enhanced webextensions documentation with removal behavior

py/selenium/webdriver/chromium/options.py

  • Added note explaining flag removal behavior when webextensions
    disabled
  • Clarified that manually added flags are also removed
  • Fixed indentation alignment in existing documentation
    		• +4/-2     
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @navin772 navin772 requested a review from shbenzer June 30, 2025 07:23
    @selenium-ci selenium-ci added the C-py Python Bindings label Jun 30, 2025
    @navin772 navin772 requested a review from cgoldberg June 30, 2025 07:23
    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 30, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    1234 - Not compliant

    Non-compliant requirements:

    • Fix JavaScript execution in link href on click() for Firefox
    • Ensure compatibility with Firefox 42.0
    • Restore functionality that worked in version 2.47.1 but broke in 2.48.0/2.48.2

    5678 - Not compliant

    Non-compliant requirements:

    • Fix ChromeDriver ConnectFailure error on Ubuntu 16.04.4
    • Resolve connection refused errors for subsequent ChromeDriver instances
    • Ensure compatibility with Chrome 65.0.3325.181 and ChromeDriver 2.35

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

    Documentation Clarity

    The added note about flag removal behavior when webextensions is disabled should be verified for accuracy and completeness regarding the actual implementation behavior.

     
    - When disabled, this removes BOTH flags listed above, even if they were manually added via add_argument()
  before enabling webextensions.

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 30, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Verify flag removal implementation matches documentation

    The note mentions removing flags when disabled, but the actual implementation
    logic for flag removal is not shown in the diff. Verify that the implementation
    actually removes these flags when value is False to match the documentation.

    py/selenium/webdriver/chromium/options.py [156-157]

    +- When disabled, this removes BOTH flags listed above, even if they were manually added via add_argument()
+  before enabling webextensions.
 
-
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly points out that the added documentation about flag removal needs a corresponding implementation, which is not visible in the diff, and rightly asks for verification.

    		Medium
    • Update

    cgoldberg
    cgoldberg approved these changes Jun 30, 2025
    View reviewed changes
    Copy link
    Member

    @cgoldberg cgoldberg left a comment

    Choose a reason for hiding this comment

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

    LGTM

    @navin772
    Copy link
    Member Author

    navin772 commented Jun 30, 2025

    The failing test is unrelated and fails on trunk also.

    @navin772 navin772 merged commit 954ffec into SeleniumHQ:trunk Jun 30, 2025
    16 of 18 checks passed
    @navin772 navin772 deleted the py-add-note-webex branch June 30, 2025 16:58
    This was referenced Sep 12, 2025
    Closed
    Closed
    Merged
    This was referenced Sep 19, 2025
    Closed
    Closed
    Closed
    This was referenced Oct 1, 2025
    Closed
    Closed
    Closed
    Merged
    Closed
    Closed
    Closed
    This was referenced Oct 20, 2025
    Closed
    Closed
    Closed
    @dependabot dependabot bot mentioned this pull request Oct 27, 2025
    Closed
    This was referenced Dec 15, 2025
    Closed
    Open
    Closed
    Closed
    Closed
    @dependabot dependabot bot mentioned this pull request Dec 22, 2025
    Closed
    This was referenced Jan 19, 2026
    Open
    Open
    Open
    Open
    Open
    Merged
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @cgoldberg cgoldberg cgoldberg approved these changes

    @shbenzer shbenzer shbenzer approved these changes

    Assignees

    No one assigned

    Labels

    C-py Python Bindings Review effort 1/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    4 participants

    @navin772 @cgoldberg @shbenzer @selenium-ci