Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[dotnet] SupportUnhandledPromptBehavior option as string and map#16557

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
nvborisenko wants to merge18 commits intoSeleniumHQ:trunk
base:trunk
Choose a base branch
Loading
fromnvborisenko:dotnet-prompt-behavior-dict

Conversation

@nvborisenko
Copy link
Member

@nvborisenkonvborisenko commentedNov 6, 2025•
edited by qodo-code-reviewbot
Loading

User description

🔗 Related Issues

Fixes#16102

💥 What does this PR do?

This PR changesUnhandledPromptBehavior property return type to custom type, supporting bothstring andmap representation at serialization phase.

🔧 Implementation Notes

It is not a breaking change at compilation level! One file change to emphasize that users are not affected.

💡 Additional Considerations

I intentionally didn't introduce new property in Options class, it would lead to a mess in my opinion.

🔄 Types of changes

  • New feature (non-breaking change which adds functionalityand tests!)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR Type

Enhancement

Description

  • SupportUnhandledPromptBehavior as both string and dictionary representations

  • Introduce abstract record hierarchy for flexible prompt behavior configuration

  • Enable per-prompt-type behavior specification viaUnhandledPromptBehaviorMultiOption

  • Maintain backward compatibility with existing enum-based API

Diagram Walkthrough

flowchart LR  A["UnhandledPromptBehavior enum"] -->|implicit conversion| B["UnhandledPromptBehaviorOption"]  B -->|Single| C["UnhandledPromptBehaviorSingleOption"]  B -->|Multi| D["UnhandledPromptBehaviorMultiOption"]  C -->|serializes to| E["string capability"]  D -->|serializes to| F["dictionary capability"]
Loading

File Walkthrough

Relevant files
Enhancement
DriverOptions.cs
Add flexible prompt behavior option types with dual serialization

dotnet/src/webdriver/DriverOptions.cs

  • Introduced abstract recordUnhandledPromptBehaviorOption with two
    sealed implementations:UnhandledPromptBehaviorSingleOption (for
    string representation) andUnhandledPromptBehaviorMultiOption (for
    dictionary representation with per-prompt-type settings)
  • Added implicit operator and factory methods (Single(),Multi()) for
    convenient instantiation
  • ChangedUnhandledPromptBehavior property type from enum to nullable
    UnhandledPromptBehaviorOption
  • Refactored serialization logic inGenerateDesiredCapabilities() to
    handle both single and multi-option cases, converting to appropriate
    capability format
  • Updated merge conflict detection to use null checks instead of enum
    comparison
  • Extracted enum-to-string conversion into local helper function
    UnhandledPromptBehaviorToString()
+79/-18 

@selenium-ciselenium-ci added the C-dotnet.NET Bindings labelNov 6, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-reviewbot commentedNov 6, 2025•
edited
Loading

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identifiedNo security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡
🎫#16102
🟢Allow setting unhandledPromptBehavior as a dictionary/map per spec for BiDi/Chrome 137+.
Preserve ability to set unhandledPromptBehavior as a single value (legacy enum) for
backward compatibility.
Ensure options serialization outputs correct wire values (strings or map) for
capabilities.
Avoid requiring users to use AdditionalOption workaround or internal DesiredCapabilities.
Behavior should work in .NET bindings (C#) without breaking compilation for existing
users.
⚪Validate in real Chrome 137+ with BiDi that alerts behave per provided map
(runtime/browser check).
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow theguide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

⚪
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No auditing: The new serialization logic forUnhandledPromptBehavior does not include any audit logging
of critical actions, but this may be out of scope for a client options class.

Referred Code
[return:NotNullIfNotNull(nameof(behavior))]staticstring?UnhandledPromptBehaviorToString(UnhandledPromptBehavior?behavior)=>behaviorswitch{Selenium.UnhandledPromptBehavior.Ignore=>"ignore",Selenium.UnhandledPromptBehavior.Accept=>"accept",Selenium.UnhandledPromptBehavior.Dismiss=>"dismiss",Selenium.UnhandledPromptBehavior.AcceptAndNotify=>"accept and notify",Selenium.UnhandledPromptBehavior.DismissAndNotify=>"dismiss and notify",    _=>null};if(this.UnhandledPromptBehaviorisUnhandledPromptBehaviorSingleOptionsingleOption){varstringValue=UnhandledPromptBehaviorToString(singleOption.Value);if(stringValueis notnull){capabilities.SetCapability(CapabilityType.UnhandledPromptBehavior,stringValue);}}elseif(this.UnhandledPromptBehaviorisUnhandledPromptBehaviorMultiOptionmultiOption) ...(clipped33lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing validation: Mapping functions accept nullable behaviors and silently skip invalid/unknown values
without error or logging, which could hide misconfigurations.

Referred Code
[return:NotNullIfNotNull(nameof(behavior))]staticstring?UnhandledPromptBehaviorToString(UnhandledPromptBehavior?behavior)=>behaviorswitch{Selenium.UnhandledPromptBehavior.Ignore=>"ignore",Selenium.UnhandledPromptBehavior.Accept=>"accept",Selenium.UnhandledPromptBehavior.Dismiss=>"dismiss",Selenium.UnhandledPromptBehavior.AcceptAndNotify=>"accept and notify",Selenium.UnhandledPromptBehavior.DismissAndNotify=>"dismiss and notify",    _=>null};if(this.UnhandledPromptBehaviorisUnhandledPromptBehaviorSingleOptionsingleOption){varstringValue=UnhandledPromptBehaviorToString(singleOption.Value);if(stringValueis notnull){capabilities.SetCapability(CapabilityType.UnhandledPromptBehavior,stringValue);}}elseif(this.UnhandledPromptBehaviorisUnhandledPromptBehaviorMultiOptionmultiOption) ...(clipped33lines)
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input validation: The new API accepts nullable option values and constructs a dictionary without explicit
validation of keys/values, potentially allowing unsupported configurations to pass
silently.

Referred Code
{Dictionary<string,string>multiOptionDictionary=[];if(multiOption.Alertis notnull){multiOptionDictionary["alert"]=UnhandledPromptBehaviorToString(multiOption.Alert);}if(multiOption.Confirmis notnull){multiOptionDictionary["confirm"]=UnhandledPromptBehaviorToString(multiOption.Confirm);}if(multiOption.Promptis notnull){multiOptionDictionary["prompt"]=UnhandledPromptBehaviorToString(multiOption.Prompt);}if(multiOption.BeforeUnloadis notnull){multiOptionDictionary["beforeUnload"]=UnhandledPromptBehaviorToString(multiOption.BeforeUnload); ...(clipped12lines)
  • Update
Compliance status legend🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-reviewbot commentedNov 6, 2025•
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                   Impact
High-level
✅The solution introduces a breaking change
Suggestion Impact:The commit adjusts the new UnhandledPromptBehaviorOption approach to be more backward-friendly by setting a default value (Default) and tightening handling to non-null enum values, reducing source-breaking risk for readers. It retains the Option type but mitigates the breaking change concerns, reflecting responsiveness to the suggestion.

code diff:

@@ -195,7 +126,7 @@     /// Gets or sets the value for describing how unexpected alerts are to be handled in the browser.     /// Defaults to <see cref="UnhandledPromptBehavior.Default"/>.     /// </summary>-    public UnhandledPromptBehaviorOption? UnhandledPromptBehavior { get; set; }+    public UnhandledPromptBehaviorOption? UnhandledPromptBehavior { get; set; } = Selenium.UnhandledPromptBehavior.Default;      /// <summary>     /// Gets or sets the value for describing how the browser is to wait for pages to load in the browser.@@ -334,7 +265,7 @@             return result;         }-        if (this.UnhandledPromptBehavior is not null && other.UnhandledPromptBehavior is not null)+        if (this.UnhandledPromptBehavior != other.UnhandledPromptBehavior)         {             result.IsMergeConflict = true;             result.MergeConflictOptionName = "UnhandledPromptBehavior";@@ -539,51 +470,47 @@             capabilities.SetCapability(CapabilityType.PageLoadStrategy, pageLoadStrategySetting);         }-        [return: NotNullIfNotNull(nameof(behavior))]-        static string? UnhandledPromptBehaviorToString(UnhandledPromptBehavior? behavior) => behavior switch+        static string UnhandledPromptBehaviorToString(UnhandledPromptBehavior behavior) => behavior switch         {             Selenium.UnhandledPromptBehavior.Ignore => "ignore",             Selenium.UnhandledPromptBehavior.Accept => "accept",             Selenium.UnhandledPromptBehavior.Dismiss => "dismiss",             Selenium.UnhandledPromptBehavior.AcceptAndNotify => "accept and notify",             Selenium.UnhandledPromptBehavior.DismissAndNotify => "dismiss and notify",-            _ => null+            _ => throw new ArgumentOutOfRangeException(nameof(behavior), $"UnhandledPromptBehavior value '{behavior}' is not recognized."),         };-        if (this.UnhandledPromptBehavior is UnhandledPromptBehaviorSingleOption singleOption)+        if (this.UnhandledPromptBehavior is UnhandledPromptBehaviorSingleOption singleOption && singleOption.Value != Selenium.UnhandledPromptBehavior.Default)         {             var stringValue = UnhandledPromptBehaviorToString(singleOption.Value);-            if (stringValue is not null)-            {-                capabilities.SetCapability(CapabilityType.UnhandledPromptBehavior, stringValue);-            }+            capabilities.SetCapability(CapabilityType.UnhandledPromptBehavior, stringValue);         }         else if (this.UnhandledPromptBehavior is UnhandledPromptBehaviorMultiOption multiOption)         {             Dictionary<string, string> multiOptionDictionary = [];-            if (multiOption.Alert is not null)+            if (multiOption.Alert is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["alert"] = UnhandledPromptBehaviorToString(multiOption.Alert);             }-            if (multiOption.Confirm is not null)+            if (multiOption.Confirm is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["confirm"] = UnhandledPromptBehaviorToString(multiOption.Confirm);             }-            if (multiOption.Prompt is not null)+            if (multiOption.Prompt is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["prompt"] = UnhandledPromptBehaviorToString(multiOption.Prompt);             }-            if (multiOption.BeforeUnload is not null)+            if (multiOption.BeforeUnload is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["beforeUnload"] = UnhandledPromptBehaviorToString(multiOption.BeforeUnload);             }-            if (multiOption.Default is not null)+            if (multiOption.Default is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["default"] = UnhandledPromptBehaviorToString(multiOption.Default);             }

The suggestion highlights that changing theUnhandledPromptBehavior property's
type is a breaking change for users who read its value. It recommends adding a
new property for the dictionary-based functionality to ensure backward
compatibility.

Examples:

dotnet/src/webdriver/DriverOptions.cs [198]
publicUnhandledPromptBehaviorOption?UnhandledPromptBehavior{get;set;}

Solution Walkthrough:

Before:

// In DriverOptions.cspublicclassDriverOptions{publicUnhandledPromptBehaviorUnhandledPromptBehavior{get;set;}=UnhandledPromptBehavior.Default;// ...}// Example user code (compiles)publicvoidConfigureDriver(DriverOptionsoptions){// Reading the property is straightforwardUnhandledPromptBehaviorbehavior=options.UnhandledPromptBehavior;if(behavior==UnhandledPromptBehavior.Ignore){// ...}}

After:

// In DriverOptions.cspublicclassDriverOptions{publicUnhandledPromptBehaviorOption?UnhandledPromptBehavior{get;set;}// ...}// Example user code (breaks compilation)publicvoidConfigureDriver(DriverOptionsoptions){// This line no longer compiles due to type mismatch// UnhandledPromptBehavior behavior = options.UnhandledPromptBehavior;// User must now add type checking and castingif(options.UnhandledPromptBehaviorisUnhandledPromptBehaviorSingleOptionsingle){UnhandledPromptBehavior?behavior=single.Value;// ...}}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical source-breaking change in the public API by altering the type of theUnhandledPromptBehavior property, which contradicts the PR's claim of being non-breaking.

High
Learned
best practice
✅Validate mapped capability values
Suggestion Impact:The commit changed UnhandledPromptBehavior handling to be non-nullable with defaults and made the mapping function throw on unknown values, and it only adds entries when not Default. This achieves the validation intent, though it didn't introduce the exact TryAdd helper.

code diff:

-        [return: NotNullIfNotNull(nameof(behavior))]-        static string? UnhandledPromptBehaviorToString(UnhandledPromptBehavior? behavior) => behavior switch+        static string UnhandledPromptBehaviorToString(UnhandledPromptBehavior behavior) => behavior switch         {             Selenium.UnhandledPromptBehavior.Ignore => "ignore",             Selenium.UnhandledPromptBehavior.Accept => "accept",             Selenium.UnhandledPromptBehavior.Dismiss => "dismiss",             Selenium.UnhandledPromptBehavior.AcceptAndNotify => "accept and notify",             Selenium.UnhandledPromptBehavior.DismissAndNotify => "dismiss and notify",-            _ => null+            _ => throw new ArgumentOutOfRangeException(nameof(behavior), $"UnhandledPromptBehavior value '{behavior}' is not recognized."),         };-        if (this.UnhandledPromptBehavior is UnhandledPromptBehaviorSingleOption singleOption)+        if (this.UnhandledPromptBehavior is UnhandledPromptBehaviorSingleOption singleOption && singleOption.Value != Selenium.UnhandledPromptBehavior.Default)         {             var stringValue = UnhandledPromptBehaviorToString(singleOption.Value);-            if (stringValue is not null)-            {-                capabilities.SetCapability(CapabilityType.UnhandledPromptBehavior, stringValue);-            }+            capabilities.SetCapability(CapabilityType.UnhandledPromptBehavior, stringValue);         }         else if (this.UnhandledPromptBehavior is UnhandledPromptBehaviorMultiOption multiOption)         {             Dictionary<string, string> multiOptionDictionary = [];-            if (multiOption.Alert is not null)+            if (multiOption.Alert is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["alert"] = UnhandledPromptBehaviorToString(multiOption.Alert);             }-            if (multiOption.Confirm is not null)+            if (multiOption.Confirm is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["confirm"] = UnhandledPromptBehaviorToString(multiOption.Confirm);             }-            if (multiOption.Prompt is not null)+            if (multiOption.Prompt is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["prompt"] = UnhandledPromptBehaviorToString(multiOption.Prompt);             }-            if (multiOption.BeforeUnload is not null)+            if (multiOption.BeforeUnload is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["beforeUnload"] = UnhandledPromptBehaviorToString(multiOption.BeforeUnload);             }-            if (multiOption.Default is not null)+            if (multiOption.Default is not Selenium.UnhandledPromptBehavior.Default)             {                 multiOptionDictionary["default"] = UnhandledPromptBehaviorToString(multiOption.Default);             }

Validate that mapped strings are non-null before adding to the dictionary to
prevent invalid capability values. Optionally throw a clear error for unknown
enum values.

dotnet/src/webdriver/DriverOptions.cs [566-589]

-if (multiOption.Alert is not null)+void TryAdd(IDictionary<string, string> dict, string key, UnhandledPromptBehavior? value) {-    multiOptionDictionary["alert"] = UnhandledPromptBehaviorToString(multiOption.Alert);+    var str = UnhandledPromptBehaviorToString(value);+    if (str is null && value is not null)+    {+        throw new ArgumentOutOfRangeException(nameof(value), $"Unsupported UnhandledPromptBehavior value '{value}'.");+    }+    if (str is not null)+    {+        dict[key] = str;+    } }-if (multiOption.Confirm is not null)-{-    multiOptionDictionary["confirm"] = UnhandledPromptBehaviorToString(multiOption.Confirm);-}+TryAdd(multiOptionDictionary, "alert", multiOption.Alert);+TryAdd(multiOptionDictionary, "confirm", multiOption.Confirm);+TryAdd(multiOptionDictionary, "prompt", multiOption.Prompt);+TryAdd(multiOptionDictionary, "beforeUnload", multiOption.BeforeUnload);+TryAdd(multiOptionDictionary, "default", multiOption.Default);-if (multiOption.Prompt is not null)-{-    multiOptionDictionary["prompt"] = UnhandledPromptBehaviorToString(multiOption.Prompt);-}--if (multiOption.BeforeUnload is not null)-{-    multiOptionDictionary["beforeUnload"] = UnhandledPromptBehaviorToString(multiOption.BeforeUnload);-}--if (multiOption.Default is not null)-{-    multiOptionDictionary["default"] = UnhandledPromptBehaviorToString(multiOption.Default);-}-

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Guard external API-facing option mapping with validation to avoid null or invalid values leaking into capabilities.

Low
General
Use init-only setters for record

Replace theset accessors withinit accessors in the
UnhandledPromptBehaviorMultiOption record to make its properties immutable after
initialization.

dotnet/src/webdriver/DriverOptions.cs [47-58]

 public sealed record UnhandledPromptBehaviorMultiOption : UnhandledPromptBehaviorOption {-    public UnhandledPromptBehavior? Alert { get; set; }+    public UnhandledPromptBehavior? Alert { get; init; }-    public UnhandledPromptBehavior? Confirm { get; set; }+    public UnhandledPromptBehavior? Confirm { get; init; }-    public UnhandledPromptBehavior? Prompt { get; set; }+    public UnhandledPromptBehavior? Prompt { get; init; }-    public UnhandledPromptBehavior? BeforeUnload { get; set; }+    public UnhandledPromptBehavior? BeforeUnload { get; init; }-    public UnhandledPromptBehavior? Default { get; set; }+    public UnhandledPromptBehavior? Default { get; init; } }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that usinginit setters for a record's properties enhances immutability, which is idiomatic for records and improves code quality and maintainability.

Low
  • Update

}

if(this.UnhandledPromptBehavior!=UnhandledPromptBehavior.Default&&other.UnhandledPromptBehavior!=UnhandledPromptBehavior.Default)
if(this.UnhandledPromptBehavioris notnull&&other.UnhandledPromptBehavioris notnull)
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Hm,!= operator for records?

Copy link
Contributor

Choose a reason for hiding this comment

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

It does feel like this method should tolerate it when both values are the same.

However, we should investigate the history of this method before making those decisions.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I understand it as don't send anything over wire if default.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

However, we should investigate the history of this method before making those decisions.

Oh yes, we should understand the purpose of this method, not obvious to me at glance.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

@jimevans do you know?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Applied!= operator from records. Theoretically this method compares two values, and if they are not equal mark the merge process as conflicted. I guess!= does the job for us.

Copy link
Contributor

@RenderMichaelRenderMichael left a comment

Choose a reason for hiding this comment

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

A breaking change like this should be well-tested. Could we add tests here?

@nvborisenko
Copy link
MemberAuthor

A breaking change like this should be well-tested. Could we add tests here?

I have no capacity to test the functionality which was not indented to be tested. You can "request changes", please propose.

@RenderMichael
Copy link
Contributor

A breaking change like this should be well-tested. Could we add tests here?

I have no capacity to test the functionality which was not indented to be tested. You can "request changes", please propose.

I propose new tests for this functionality.

@nvborisenko
Copy link
MemberAuthor

Existing tests don't satisfy you? You ask me to increase the coverage. But what is coverage?

@nvborisenko
Copy link
MemberAuthor

nvborisenko commentedNov 6, 2025•
edited
Loading

@RenderMichael welcome to#15536

UPD: is it only one concern from your side?

@YevgeniyShunevych
Copy link
Contributor

Generally, looks good,@nvborisenko. What I don't like is a nullability of properties and parameters (UnhandledPromptBehaviorOption?,UnhandledPromptBehavior?). If I understand correctly, forUnhandledPromptBehavior?null andDefault values are equivalent. So 2 states basically indicate the same state. So why not to make it non-nullable and useDefault as a default. I would also renameUnhandledPromptBehavior.Default toNone, as it is not clear what "Default" means, but "None" means no behavior is specified.


capabilities.SetCapability(CapabilityType.UnhandledPromptBehavior,stringValue);
}
elseif(this.UnhandledPromptBehaviorisUnhandledPromptBehaviorMultiOptionmultiOption)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should follow our usual pattern here to have aToDictionary() method directly on theUnhandledPromptBehaviorMultiOption type:

publicDictionary<string,string>ToDictionary(){Dictionary<string,string>multiOptionDictionary=[];if(Alertis notSelenium.UnhandledPromptBehavior.Default){multiOptionDictionary["alert"]=UnhandledPromptBehaviorToString(Alert);}if(Confirmis notSelenium.UnhandledPromptBehavior.Default){multiOptionDictionary["confirm"]=UnhandledPromptBehaviorToString(Confirm);}if(Promptis notSelenium.UnhandledPromptBehavior.Default){multiOptionDictionary["prompt"]=UnhandledPromptBehaviorToString(Prompt);}if(BeforeUnloadis notSelenium.UnhandledPromptBehavior.Default){multiOptionDictionary["beforeUnload"]=UnhandledPromptBehaviorToString(BeforeUnload);}if(Defaultis notSelenium.UnhandledPromptBehavior.Default){multiOptionDictionary["default"]=UnhandledPromptBehaviorToString(Default);}returnmultiOptionDictionary;}

and then to call that method here

elseif(this.UnhandledPromptBehavioris UnhandledPromptBehaviorMultiOption multiOption){multiOptionDictionary=multiOption.ToDictionary();if(multiOptionDictionary.Count!=0){capabilities.SetCapability(CapabilityType.UnhandledPromptBehavior,multiOptionDictionary);}}

Copy link
Contributor

Choose a reason for hiding this comment

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

This also allows us to moveUnhandledPromptBehaviorToString to theUnhandledPromptBehaviorOption, cleaning up this 150-line method.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Good idea to introduceToCapabilities() internal method.

multiOptionDictionary["default"]=UnhandledPromptBehaviorToString(multiOption.Default);
}

if(multiOptionDictionary.Count!=0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to guard against{}? It feels like we should transparently sent this to the server.

What happens if we do this, anyway?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Originally we sent nothing if the value isdefault, meaning we don't spam wire if user don't specifies non-default. Anyway need to double check whether we really avoid spamming.

/// Creates an <see cref="UnhandledPromptBehaviorOption"/> allowing individual <see cref="UnhandledPromptBehavior"/> values per prompt type.
/// </summary>
/// <returns>An <see cref="UnhandledPromptBehaviorOption"/> with per-prompt configurable behaviors.</returns>
publicstaticUnhandledPromptBehaviorOptionMulti()
Copy link
Contributor

Choose a reason for hiding this comment

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

These semantics are a little unwieldy:

Image

Maybe we could have a nested class here?

Image

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are changing the type name, I think it would align better with the spec to call itUnhandledPromptHandlers

nvborisenko reacted with heart emoji
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Fixed static factory, now it is possible to:

UnhandledPromptBehaviorOption.Multi()with{BeforeUnload=UnhandledPromptBehavior.Accept}

Interesting point aboutUnhandledPromptHandlers.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

  1. We already discussed nested types are bad (in BiDi context), preventing us introducing static factories.
  2. AboutUnhandledPromptHandlers type name.. Could you please provide examples (Single/Multi)? I think ofUnhandledPromptHandlers is moreIEnumerarable<> rather than regular class.

/// <summary>
/// Gets or sets the default behavior to use when an unexpected browser prompt is encountered.
/// </summary>
publicUnhandledPromptBehaviorDefault{get;set;}=UnhandledPromptBehavior.Default;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind, seeing

The "file" prompt type is respected only in [WebDriver-BiDi] sessions.

nvborisenko reacted with thumbs up emoji
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

It should be here as well.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

To usebidi we should initiate the first browser (userContext/browsingContext). It is done only via selenium classic command, and here user is able to specify initial behavior how to deal with prompts. BTW it is impossible to change this later. So yes, we needfile property with good documentation.

RenderMichael reacted with thumbs up emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@RenderMichaelRenderMichaelRenderMichael requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: [dotnet] Unable to set dictionary to the UnhandledPromptBehavior option

4 participants

@nvborisenko@RenderMichael@YevgeniyShunevych@selenium-ci
[8]ページ先頭
©2009-2025 Movatter.jp