Skip to content

[dotnet] [bidi] Support WebExtension module #15850

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

Draft
wants to merge 15 commits into
base: trunk
Choose a base branch
from

Conversation

nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Jun 3, 2025

User description

https://w3c.github.io/webdriver-bidi/#module-webExtension

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Bug fix (backwards compatible)
  • New feature (non-breaking change which adds functionality and tests!)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR Type

Enhancement, Tests


Description

  • Add BiDi WebExtension module for installing/uninstalling extensions

  • Implement WebExtension install/uninstall commands and types

  • Integrate WebExtension support into BiDi broker and serializer

  • Add comprehensive tests for WebExtension install/uninstall

  • Update test project to include extension test data


Changes walkthrough 📝

Relevant files
Enhancement
8 files
BiDi.cs
Integrate WebExtension module into BiDi client                     
+3/-0     
Broker.cs
Register WebExtensionConverter in BiDi broker                       
+1/-0     
BiDiJsonSerializerContext.cs
Add WebExtension commands/results to serializer context   
+4/-0     
WebExtensionConverter.cs
Add JSON converter for WebExtension type                                 
+47/-0   
Extension.cs
Implement WebExtension Extension class with uninstall       
+40/-0   
InstallCommand.cs
Define install command and types for WebExtension               
+44/-0   
UninstallCommand.cs
Define uninstall command and options for WebExtension       
+29/-0   
WebExtensionModule.cs
Implement WebExtensionModule with install/uninstall methods
+40/-0   
Tests
1 files
WebExtensionTest.cs
Add tests for WebExtension install/uninstall scenarios     
+76/-0   
Configuration changes
1 files
WebDriver.Common.Tests.csproj
Include extension test data in test project                           
+4/-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.
  • @selenium-ci selenium-ci added the C-dotnet .NET Bindings label Jun 3, 2025
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 3, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    1234 - Not compliant

    Non-compliant requirements:

    • Fix issue where Selenium 2.48 doesn't trigger JavaScript in link's href on click() in Firefox 42.0
    • Ensure JavaScript alerts are triggered properly when clicking links with JavaScript in href attribute

    5678 - Not compliant

    Non-compliant requirements:

    • Fix "ConnectFailure (Connection refused)" error when instantiating ChromeDriver multiple times
    • Ensure subsequent ChromeDriver instances can be created without connection errors

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

    Method Visibility

    The UninstallAsync method is marked as internal while it's being called from the public Extension class. Consider making this method public for consistency and to allow direct uninstallation without going through the Extension object.

    internal async Task UninstallAsync(Extension extension, UninstallOptions? options = null)
    {
        var @params = new UninstallCommandParameters(extension);
    
        await Broker.ExecuteCommandAsync(new UninstallCommand(@params), options).ConfigureAwait(false);
    }

    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 3, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix inheritance contradiction

    The InstallResult class inherits from EmptyResult but also contains an Extension
    property, which is contradictory. Either make it a standalone result class or
    ensure it properly extends the base class without conflicting behavior.

    dotnet/src/webdriver/BiDi/WebExtension/InstallCommand.cs [44]

    -public record InstallResult(Extension Extension) : EmptyResult;
    +public record InstallResult(Extension Extension);
    • Apply / Chat
    Suggestion importance[1-10]: 2

    __

    Why: The suggestion assumes EmptyResult means no properties are allowed, but this may not be accurate. Removing the inheritance could break expected functionality without understanding the base class purpose.

    Low
    • Update

    @nvborisenko
    Copy link
    Member Author

    Test fails on CI, because I don't know how to prepare test data in bazel test environment. It passes locally in IDE and dotnet test.

    @cgoldberg
    Copy link
    Contributor

    [IgnoreBrowser(Selenium.Browser.Chrome, "Web extensions are not supported yet?")]
    [IgnoreBrowser(Selenium.Browser.Edge, "Web extensions are not supported yet?")]

    They should work for Chrome/Edge if you start the browser using the flags:

    --remote-debugging-pipe
    --enable-unsafe-extension-debugging
    

    The Python bindings added a property on Chromium's Options class named enable_webextensions that a user can enable to set these flags.

    see #15794

    @nvborisenko
    Copy link
    Member Author

    Test fails on CI, because I don't know how to prepare test data in bazel test environment. It passes locally in IDE and dotnet test.

    I managed it and now we are able to execute tests in different environments (dotnet test, bazel test, IDE).

    @nvborisenko
    Copy link
    Member Author

    --remote-debugging-pipe argument breaks CDP.

    #15749 (comment)

    @nvborisenko
    Copy link
    Member Author

    Parking it until better weather.

    @nvborisenko nvborisenko marked this pull request as draft June 20, 2025 18:48
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants