[dotnet] [bidi] Support WebExtension module

[nvborisenko]

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.

[qodo-code-review[bot]]

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); }

[qodo-code-review[bot]]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	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]

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]

[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]

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]

--remote-debugging-pipe argument breaks CDP.

https://github.com/SeleniumHQ/selenium/pull/15749#discussion_r2099985400

[nvborisenko]

Parking it until better weather.

[nvborisenko]

Here we should refactor tests projects. Any test Fixture should be capable to override the initialization of WebDriver. That's it. Probably relates to https://github.com/SeleniumHQ/selenium/issues/15536

[SKumar-777]

Hi Team,

Will this option be available in the upcoming Selenium version 4.35?

Thanks.

[cgoldberg]

@SKumar-777 no, sorry... 4.35 was released yesterday and this hasn't been merged yet.

[nvborisenko]

Sorry, I am stuck here. Options to move further:

• Refactor internal tests infrastructure (big effort)
• Apply some dirty hack to support this specific case
• Temporary ignore internal tests and release the functionality

[diemol]

@navin772 @Delta456 what approach did you take when implementing this?

[cgoldberg]

Refactor internal tests infrastructure (big effort)

what approach did you take when implementing this?

Python tests and fixtures were updated to handle this.

[navin772]

@navin772 @Delta456 what approach did you take when implementing this?

The extension related files/dirs had to be copied via bazel, which this PR is already doing. @nvborisenko I don't have much idea about dotnet but what exactly is blocking this PR? The implementation looks good. Are we blocked on tests, in the last CI run, I see that the related tests are passing.

[nvborisenko]

Ideally to move further we should be able to add arguments for specific fixture/suite/test.

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

The issue is that in internal implemented testing infrastructure we cannot do it, only globally, but it breaks DevTools/CDP. Looked at the code, and I don't even see "quick dirty hack" to improve the situation.

Seems our option is to merge without tests, and then add them later when weather becomes good: https://github.com/SeleniumHQ/selenium/issues/15536

[qodo-code-review[bot]]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1234 - Partially compliant Compliant requirements: Non-compliant requirements: Investigate regression where click() no longer triggers JavaScript in link href in Selenium 2.48.x (worked in 2.47.1) on Firefox 42. Provide fix or explanation/workaround if applicable. Validate behavior across affected versions/browsers. Requires further human verification: 5678 - Partially compliant Compliant requirements: Non-compliant requirements: Diagnose "ConnectFailure (Connection refused)" when instantiating ChromeDriver after the first instance on Ubuntu 16.04 with Chrome 65/ChromeDriver 2.35. Determine root cause and propose fix or configuration guidance. Provide reproducible steps and ensure subsequent instances work reliably. Requires further human verification:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Handling The converter assumes non-null IDs in Read and Write without null checks. If the protocol returns a null or missing ID, this will throw. Consider validating or throwing a clearer error. public override Extension? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { var id = reader.GetString(); return new Extension(_bidi, id!); } public override void Write(Utf8JsonWriter writer, Extension value, JsonSerializerOptions options) { writer.WriteStringValue(value.Id); } Polymorphic Types The JsonPolymorphic mapping relies on "type" discriminators. Verify server responses match these strings and that unknown types are handled gracefully to avoid deserialization failures. [JsonPolymorphic(TypeDiscriminatorPropertyName = "type")] [JsonDerivedType(typeof(ExtensionArchivePath), "archivePath")] [JsonDerivedType(typeof(ExtensionBase64Encoded), "base64")] [JsonDerivedType(typeof(ExtensionPath), "path")] public abstract record ExtensionData; public sealed record ExtensionArchivePath(string Path) : ExtensionData; public sealed record ExtensionBase64Encoded(string Value) : ExtensionData; public sealed record ExtensionPath(string Path) : ExtensionData; Skipped Tests Entire test class is ignored due to driver args conflicts, and some tests are skipped for Chrome/Edge. This limits coverage of the new feature; consider isolating environment to enable execution. [Ignore(""" The following test suite wants to set driver arguments via Options, but it breaks CDP/DevTools tests. The desired arguments (for Chromium only?): --enable-unsafe-extension-debugging --remote-debugging-pipe Ignoring these tests for now. Hopefully https://github.com/SeleniumHQ/selenium/issues/15536 will be resolved soon. """)] class WebExtensionTest : BiDiTestFixture { [Test]

[qodo-code-review[bot]]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Validate protocol compatibility The WebExtension API surface is added without any feature-detection or capability checks, yet different browsers (and even channels) may not support archive/base64 installs or the module at all. Add a negotiated capability/version check via BiDi session status or browsingContext/permissions before exposing or invoking webExtension.install/uninstall to prevent runtime protocol errors across engines. Examples: dotnet/src/webdriver/BiDi/BiDi.cs [154-165] public WebExtension.WebExtensionModule WebExtension { get { if (_webExtensionModule is not null) return _webExtensionModule; lock (_moduleLock) { _webExtensionModule ??= new WebExtension.WebExtensionModule(_broker); } return _webExtensionModule; ... (clipped 2 lines) dotnet/src/webdriver/BiDi/WebExtension/WebExtensionModule.cs [27-32] public async Task<InstallResult> InstallAsync(ExtensionData extensionData, InstallOptions? options = null) { var @params = new InstallCommandParameters(extensionData); return await Broker.ExecuteCommandAsync<InstallCommand, InstallResult>(new InstallCommand(@params), options).ConfigureAwait(false); } Solution Walkthrough: Before: // In BiDi.cs public WebExtension.WebExtensionModule WebExtension { get { // Lazily initializes module without checking for browser support. if (_webExtensionModule is not null) return _webExtensionModule; lock (_moduleLock) { _webExtensionModule ??= new WebExtension.WebExtensionModule(_broker); } return _webExtensionModule; } } After: // In BiDi.cs // Assumes _supportedModules is populated from session.status on connection. private HashSet<string> _supportedModules; public WebExtension.WebExtensionModule WebExtension { get { if (!_supportedModules.Contains("webExtension")) { throw new WebDriverException("The WebExtension BiDi module is not supported by this browser."); } // Lazily initialize module only if supported. _webExtensionModule ??= new WebExtension.WebExtensionModule(_broker); return _webExtensionModule; } } Suggestion importance[1-10]: 8 __ Why: This suggestion addresses a significant design oversight by highlighting the lack of protocol compatibility checks, which is crucial for a library intended to work across different browsers.	Medium
Possible issue	Validate JSON token and nulls Guard against null or non-string JSON tokens to prevent runtime exceptions when deserializing. Validate the token type and value, and throw a descriptive JsonException if invalid. dotnet/src/webdriver/BiDi/Communication/Json/Converters/WebExtensionConverter.cs [36-41] public override Extension? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { + if (reader.TokenType != JsonTokenType.String) + { + throw new JsonException($"Expected string for Extension id, but got {reader.TokenType}."); + } + var id = reader.GetString(); + if (string.IsNullOrEmpty(id)) + { + throw new JsonException("Extension id cannot be null or empty."); + } - return new Extension(_bidi, id!); + return new Extension(_bidi, id); } [ ] Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies that the custom JSON converter lacks validation, making it more robust against unexpected or malformed server responses and preventing potential runtime exceptions.	Medium
	Align JSON field names Ensure serialized property names match the protocol schema (camelCase) to avoid server-side deserialization issues. Add JsonPropertyName attributes to fields to guarantee correct casing regardless of serializer naming policy. dotnet/src/webdriver/BiDi/WebExtension/InstallCommand.cs [30-40] [JsonPolymorphic(TypeDiscriminatorPropertyName = "type")] [JsonDerivedType(typeof(ExtensionArchivePath), "archivePath")] [JsonDerivedType(typeof(ExtensionBase64Encoded), "base64")] [JsonDerivedType(typeof(ExtensionPath), "path")] public abstract record ExtensionData; -public sealed record ExtensionArchivePath(string Path) : ExtensionData; +public sealed record ExtensionArchivePath([property: JsonPropertyName("path")] string Path) : ExtensionData; -public sealed record ExtensionBase64Encoded(string Value) : ExtensionData; +public sealed record ExtensionBase64Encoded([property: JsonPropertyName("value")] string Value) : ExtensionData; -public sealed record ExtensionPath(string Path) : ExtensionData; +public sealed record ExtensionPath([property: JsonPropertyName("path")] string Path) : ExtensionData; [ ] Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly proposes using JsonPropertyName for explicit serialization, which improves code robustness by making it independent of the global serializer naming policy.	Low
General	Add null input validation Validate input to avoid null reference issues and clearer error messages. Throw an ArgumentNullException when the required 'extensionData' parameter is null. dotnet/src/webdriver/BiDi/WebExtension/WebExtensionModule.cs [27-32] public async Task<InstallResult> InstallAsync(ExtensionData extensionData, InstallOptions? options = null) { + if (extensionData is null) throw new ArgumentNullException(nameof(extensionData)); + var @params = new InstallCommandParameters(extensionData); - return await Broker.ExecuteCommandAsync<InstallCommand, InstallResult>(new InstallCommand(@params), options).ConfigureAwait(false); } [ ] Apply / Chat Suggestion importance[1-10]: 6 __ Why: This suggestion correctly adds a null check for the extensionData parameter, which is a good practice for public API methods to fail early and provide a clear ArgumentNullException.	Low
Learned best practice	Add constructor argument guards Guard the constructor against null or empty arguments to ensure the object is always in a valid state. Throw ArgumentNullException/ArgumentException with clear messages. dotnet/src/webdriver/BiDi/WebExtension/Extension.cs [28-32] public Extension(BiDi bidi, string id) { - _bidi = bidi; - Id = id; + _bidi = bidi ?? throw new ArgumentNullException(nameof(bidi)); + Id = !string.IsNullOrEmpty(id) ? id : throw new ArgumentException("Extension id cannot be null or empty.", nameof(id)); } [ ] Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add null checks and validation for parameters and variables before using them to prevent NullReferenceException.	Low
[ ] More

[nvborisenko]

Let's merge it with the proved assumptions:

• it works in general
• test data works, bazel + IDE

Some tests are written, but just ignore for now. Show must go on.

[nvborisenko]

CI failure is not related to these changes. I am merging it because users wait this functionality, even untested.

[praveenkumar800]

Thanks, Team! I’m hoping this option will be available in the upcoming .NET Selenium v4.36. Could you please share a sample code snippet showing how to enable this option in .NET and load a Chrome extension from a local path?

[nvborisenko]

@praveenkumar800 some internal samples: https://github.com/SeleniumHQ/selenium/blob/trunk/dotnet/test/common/BiDi/WebExtension/WebExtensionTest.cs