[dotnet] Remove client side validation of well-known Options

[nvborisenko]

User description

This resolves design issue in .NET driver options class. Options class is a wrapper of well-known supported remote-end options, usually via strong typing. But meanwhile there is AddAdditionalOption(...) method which allows user to do what he wants.

The issue in design is that AddAdditionalOption(...) method doesn't allow user what he wants because this method dictates to use another API. This is really bad, at compile time user is going to do something legible, but gets weird exception in runtime.

🔗 Related Issues

It contributes to #16102, but doesn't resolve completely.

💥 What does this PR do?

Do not validate that user uses Selenium library improperly, he uses it correctly.

🔄 Types of changes

• Bug fix (backwards compatible)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Bug fix

---

Description

• Remove client-side validation preventing legitimate API usage

• Clean up constructor code removing capability name tracking

• Simplify ValidateCapabilityName method implementation

• Fix design issue with AddAdditionalOption method restrictions

---

Diagram Walkthrough

flowchart LR
  A["DriverOptions Constructor"] -- "removes" --> B["Known Capability Tracking"]
  C["ValidateCapabilityName Method"] -- "simplifies" --> D["Basic Null Check Only"]
  E["ChromiumOptions Constructor"] -- "removes" --> F["Capability Name Registration"]
  G["FirefoxOptions Constructor"] -- "removes" --> H["Capability Name Registration"]
  I["InternetExplorerOptions Constructor"] -- "removes" --> J["Capability Name Registration"]
  K["SafariOptions Constructor"] -- "removes" --> L["Capability Name Registration"]

Loading

File Walkthrough

Relevant files

Bug fix

DriverOptions.cs Remove capability validation infrastructure                            dotnet/src/webdriver/DriverOptions.cs Remove constructor and known capability tracking system Simplify ValidateCapabilityName to only check for null/empty Remove methods for managing known capabilities Clean up unused imports +0/-83    ChromiumOptions.cs Remove ChromiumOptions constructor validation                        dotnet/src/webdriver/Chromium/ChromiumOptions.cs Remove entire constructor that registered known capabilities Eliminate all AddKnownCapabilityName calls +0/-23    FirefoxOptions.cs Clean up FirefoxOptions constructor                                            dotnet/src/webdriver/Firefox/FirefoxOptions.cs Remove all AddKnownCapabilityName calls from constructor Keep BiDi protocol preference setting +1/-11    InternetExplorerOptions.cs Clean up InternetExplorerOptions constructor                          dotnet/src/webdriver/IE/InternetExplorerOptions.cs Remove all AddKnownCapabilityName calls from constructor Keep browser name and platform settings +0/-21    SafariOptions.cs Clean up SafariOptions constructor                                              dotnet/src/webdriver/Safari/SafariOptions.cs Remove AddKnownCapabilityName calls from constructor Keep browser name and technology preview settings +0/-2

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 16102 - PR Code Verified Compliant requirements: • Remove client-side validation that prevents legitimate API usage • Fix the ArgumentException when trying to use AddAdditionalOption for unhandledPromptBehavior capability • Allow users to use AddAdditionalOption method to set dictionary values for unhandledPromptBehavior Requires further human verification: • Verify that setting unhandledPromptBehavior as a dictionary works correctly with Chrome 137+ BiDi driver • Test that browser prompts are handled properly with the dictionary format

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Breaking Change The removal of known capability validation is a significant behavioral change that could allow users to accidentally override type-safe properties without warning, potentially leading to unexpected behavior or configuration conflicts. protected void ValidateCapabilityName([NotNull] string? capabilityName) { if (capabilityName is null || string.IsNullOrEmpty(capabilityName)) { throw new ArgumentException("Capability name may not be null an empty string.", nameof(capabilityName)); } }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Fix grammatical error in exception message Fix the grammatical error in the exception message. The message should read "null or an empty string" instead of "null an empty string" to be grammatically correct. dotnet/src/webdriver/DriverOptions.cs [340-346] protected void ValidateCapabilityName([NotNull] string? capabilityName) { if (capabilityName is null || string.IsNullOrEmpty(capabilityName)) { - throw new ArgumentException("Capability name may not be null an empty string.", nameof(capabilityName)); + throw new ArgumentException("Capability name may not be null or an empty string.", nameof(capabilityName)); } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 2 __ Why: The suggestion correctly identifies and fixes a minor grammatical error in an exception message, which slightly improves code clarity.	Low
No more code suggestions

[nvborisenko]

OMG!

var options = new ChromeOptions();

options.AddAdditionalOption("myCustomOption", "myCastomValue");

using var driver = new ChromeDriver(options);

18:03:07.617 DEBUG HttpCommandExecutor: Executing command: []: newSession
18:03:07.632 TRACE HttpCommandExecutor: >> POST http://localhost:52206/session
{"capabilities":{"firstMatch":[{"browserName":"chrome","goog:chromeOptions":{"binary":"C:\\Program Files\\Google\\Chrome\\Application\\chrome.exe"}}]}}

It silently ignores unknown options! :(

[nvborisenko]

Tagging @SeleniumHQ/selenium-committers team. Do you strange situation as me?

[shbenzer]

Tagging @SeleniumHQ/selenium-committers team. Do you strange situation as me?

Maybe I’m not fully understanding… but since we’re removing our validation, then we’re putting the onus on the driver devs. I’m not sure what to do but notify them of this issue and hope they resolve it

[nvborisenko]

Here we should choose between:

1. User knows what he is doing
2. Raise an exception like "Selenium determined you are stupid, please don't use this method, use another method instead"

The second option is unacceptable IMHO.

[diemol]

@SeleniumHQ/selenium-tlc needs to decide on this.

[titusfortner]

I think I understand what this is doing, but it would be good to add a test that would not have passed under existing code that will now work with this change.
Otherwise, I think this is a good update.

[nvborisenko]

I think I understand what this is doing, but it would be good to add a test that would not have passed under existing code that will now work with this change. Otherwise, I think this is a good update.

I don't see a good place where to add it, there is no kind of existing verifications. Please suggest.

[nvborisenko]

Ping pong.

[RenderMichael]

It is a well-known state of affairs that custom/third-party capabilities have a prefix and a colon, eg. goog:myChromiumCapability.

This PR basically allows first-party capabilities to be defined without validation that it is a real property.

Is this desirable? What do other bindings do? Is the behavior for a non-existent capability defined by the spec? If not, what does each browser do?

It silently ignores unknown options! :(

I agree that this isn’t good. Do we know if that was a design choice, or just something that happened to be? It would be a breaking change, is it too big of one? If possible, we should throw on unknown first-party capabilities.

[nvborisenko]

Java allows it somehow, user provided gist how to do it.