Skip to content

Conversation

nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Oct 5, 2025

User description

https://w3c.github.io/webdriver-bidi/#command-browser-setDownloadBehavior

💥 What does this PR do?

Implements SetDownloadBehavior command.

💡 Additional Considerations

Not yet supported by browsers, but I verified locally whether we serialize command properly.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement


Description

  • Implement WebDriver BiDi SetDownloadBehavior command

  • Add support for allowed/denied download behaviors

  • Include comprehensive test coverage for all scenarios


Diagram Walkthrough

flowchart LR
  A["BrowserModule"] --> B["SetDownloadBehaviorCommand"]
  B --> C["DownloadBehavior Types"]
  C --> D["DownloadBehaviorAllowed"]
  C --> E["DownloadBehaviorDenied"]
  F["JSON Serialization"] --> B
  G["Tests"] --> A
Loading

File Walkthrough

Relevant files
Enhancement
BrowserModule.cs
Add SetDownloadBehavior methods to BrowserModule                 

dotnet/src/webdriver/BiDi/Browser/BrowserModule.cs

  • Add three async methods for setting download behavior
  • Support allowed behavior with destination folder
  • Support allowed behavior with default settings
  • Support denied behavior configuration
+21/-0   
SetDownloadBehaviorCommand.cs
Create SetDownloadBehavior command infrastructure               

dotnet/src/webdriver/BiDi/Browser/SetDownloadBehaviorCommand.cs

  • Create new command class for SetDownloadBehavior
  • Define polymorphic DownloadBehavior base class
  • Implement DownloadBehaviorAllowed and DownloadBehaviorDenied types
  • Add SetDownloadBehaviorOptions configuration class
+43/-0   
Configuration changes
BiDiJsonSerializerContext.cs
Register command for JSON serialization                                   

dotnet/src/webdriver/BiDi/Communication/Json/BiDiJsonSerializerContext.cs

  • Register SetDownloadBehaviorCommand for JSON serialization
+1/-0     
Tests
BrowserTest.cs
Add comprehensive SetDownloadBehavior tests                           

dotnet/test/common/BiDi/Browser/BrowserTest.cs

  • Add test for allowed download behavior with path
  • Add test for default allowed download behavior
  • Add test for denied download behavior
  • Include browser ignore attributes for unsupported browsers
+33/-0   

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Oct 5, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 5, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link
Contributor

qodo-merge-pro bot commented Oct 5, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Re-evaluate the command parameter serialization

The command parameter serialization for SetDownloadBehaviorCommand is incorrect.
The implementation uses a nested, polymorphic object structure instead of the
flat JSON structure with behavior and downloadPath fields required by the
WebDriver BiDi specification.

Examples:

dotnet/src/webdriver/BiDi/Browser/SetDownloadBehaviorCommand.cs [29-38]
internal sealed record SetDownloadBehaviorParameters([property: JsonIgnore(Condition = JsonIgnoreCondition.Never)] DownloadBehavior? DownloadBehavior, IEnumerable<UserContext>? UserContexts) : Parameters;

[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")]
[JsonDerivedType(typeof(DownloadBehaviorAllowed), "allowed")]
[JsonDerivedType(typeof(DownloadBehaviorDenied), "denied")]
internal abstract record DownloadBehavior;

internal sealed record DownloadBehaviorAllowed(string DestinationFolder) : DownloadBehavior;

internal sealed record DownloadBehaviorDenied : DownloadBehavior;

Solution Walkthrough:

Before:

// dotnet/src/webdriver/BiDi/Browser/SetDownloadBehaviorCommand.cs
internal sealed record SetDownloadBehaviorParameters(
    DownloadBehavior? DownloadBehavior, 
    IEnumerable<UserContext>? UserContexts) : Parameters;

[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")]
[JsonDerivedType(typeof(DownloadBehaviorAllowed), "allowed")]
[JsonDerivedType(typeof(DownloadBehaviorDenied), "denied")]
internal abstract record DownloadBehavior;

internal sealed record DownloadBehaviorAllowed(string DestinationFolder) : DownloadBehavior;

internal sealed record DownloadBehaviorDenied : DownloadBehavior;

After:

// dotnet/src/webdriver/BiDi/Browser/SetDownloadBehaviorCommand.cs
internal sealed record SetDownloadBehaviorParameters(string Behavior) : Parameters
{
    [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
    public string? DownloadPath { get; init; }

    [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
    public IEnumerable<UserContext>? UserContexts { get; init; }
}

// dotnet/src/webdriver/BiDi/Browser/BrowserModule.cs
public async Task<EmptyResult> SetDownloadBehaviorAllowedAsync(string destinationFolder, ...)
{
    var @params = new SetDownloadBehaviorParameters("allow") { DownloadPath = destinationFolder, ... };
    // ...
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical serialization issue where the implemented JSON structure for SetDownloadBehaviorCommand parameters does not match the WebDriver BiDi specification, rendering the command incompatible.

High
Possible issue
Align download behavior models with spec

Update the SetDownloadBehaviorParameters and DownloadBehaviorAllowed records to
match the BiDi specification by making the behavior parameter non-nullable,
renaming DestinationFolder to DownloadPath, making it optional, and adding
correct JSON serialization attributes.

dotnet/src/webdriver/BiDi/Browser/SetDownloadBehaviorCommand.cs [29-36]

-internal sealed record SetDownloadBehaviorParameters([property: JsonIgnore(Condition = JsonIgnoreCondition.Never)] DownloadBehavior? DownloadBehavior, IEnumerable<UserContext>? UserContexts) : Parameters;
+internal sealed record SetDownloadBehaviorParameters(DownloadBehavior Behavior, IEnumerable<UserContext>? UserContexts) : Parameters;
 
 [JsonPolymorphic(TypeDiscriminatorPropertyName = "type")]
 [JsonDerivedType(typeof(DownloadBehaviorAllowed), "allowed")]
 [JsonDerivedType(typeof(DownloadBehaviorDenied), "denied")]
 internal abstract record DownloadBehavior;
 
-internal sealed record DownloadBehaviorAllowed(string DestinationFolder) : DownloadBehavior;
+internal sealed record DownloadBehaviorAllowed(
+    [property: JsonPropertyName("downloadPath")]
+    [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
+    string? DownloadPath) : DownloadBehavior;
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that the data models do not align with the BiDi specification, fixing nullability, property names, and optionality, which is critical for correct serialization and functionality.

High
Update method calls for new models

In BrowserModule.cs, update the SetDownloadBehaviorAllowedAsync overload to pass
new DownloadBehaviorAllowed(null) instead of null for the behavior parameter to
align with model changes and prevent a runtime error.

dotnet/src/webdriver/BiDi/Browser/BrowserModule.cs [56-75]

 public async Task<EmptyResult> SetDownloadBehaviorAllowedAsync(string destinationFolder, SetDownloadBehaviorOptions? options = null)
 {
     var @params = new SetDownloadBehaviorParameters(new DownloadBehaviorAllowed(destinationFolder), options?.UserContexts);
 
     return await Broker.ExecuteCommandAsync<SetDownloadBehaviorCommand, EmptyResult>(new SetDownloadBehaviorCommand(@params), options).ConfigureAwait(false);
 }
 
 public async Task<EmptyResult> SetDownloadBehaviorAllowedAsync(SetDownloadBehaviorOptions? options = null)
 {
-    var @params = new SetDownloadBehaviorParameters(null, options?.UserContexts);
+    var @params = new SetDownloadBehaviorParameters(new DownloadBehaviorAllowed(null), options?.UserContexts);
 
     return await Broker.ExecuteCommandAsync<SetDownloadBehaviorCommand, EmptyResult>(new SetDownloadBehaviorCommand(@params), options).ConfigureAwait(false);
 }
 
 public async Task<EmptyResult> SetDownloadBehaviorDeniedAsync(SetDownloadBehaviorOptions? options = null)
 {
     var @params = new SetDownloadBehaviorParameters(new DownloadBehaviorDenied(), options?.UserContexts);
 
     return await Broker.ExecuteCommandAsync<SetDownloadBehaviorCommand, EmptyResult>(new SetDownloadBehaviorCommand(@params), options).ConfigureAwait(false);
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This suggestion fixes a bug where a null value was passed for a required parameter, which would cause a runtime failure. The proposed change correctly instantiates the behavior object, ensuring the feature works as intended.

High
Learned
best practice
Validate required string parameter

Guard destinationFolder with null/empty/whitespace validation and throw a
precise exception message before constructing parameters.

dotnet/src/webdriver/BiDi/Browser/BrowserModule.cs [56-61]

 public async Task<EmptyResult> SetDownloadBehaviorAllowedAsync(string destinationFolder, SetDownloadBehaviorOptions? options = null)
 {
+    if (string.IsNullOrWhiteSpace(destinationFolder))
+    {
+        throw new ArgumentException("destinationFolder is required and must be a non-empty path.", nameof(destinationFolder));
+    }
+
     var @params = new SetDownloadBehaviorParameters(new DownloadBehaviorAllowed(destinationFolder), options?.UserContexts);
 
     return await Broker.ExecuteCommandAsync<SetDownloadBehaviorCommand, EmptyResult>(new SetDownloadBehaviorCommand(@params), options).ConfigureAwait(false);
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate inputs early to prevent null/invalid state; check required string parameters for null/empty before use.

Low
  • More

@nvborisenko nvborisenko merged commit 59a27f1 into SeleniumHQ:trunk Oct 5, 2025
13 checks passed
@nvborisenko nvborisenko deleted the bidi-browser-setdownloadbehaviour branch October 5, 2025 13:04
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.

2 participants