You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The AddArguments method should validate each individual argument in the collection for null/empty values, not just the collection itself, to prevent invalid arguments from being added.
public void AddArguments(IEnumerable<string> argumentsToAdd)
{
if (argumentsToAdd == null)
{
throw new ArgumentNullException(nameof(argumentsToAdd), "Arguments to add cannot be null");
}
+ foreach (string arg in argumentsToAdd)+ {+ if (string.IsNullOrEmpty(arg))+ {+ throw new ArgumentException("Individual arguments cannot be null or empty");+ }+ }
this.arguments.AddRange(argumentsToAdd);
}
Apply this suggestion
Suggestion importance[1-10]: 7
Why: The suggestion adds important input validation to prevent invalid arguments from being added to the collection, which could cause issues later when the arguments are used. This improves robustness and error handling.
7
Add null check before access
The EnableMobileEmulation method should validate that the device settings object is not null before accessing its properties to avoid potential null reference exceptions.
public void EnableMobileEmulation(ChromiumMobileEmulationDeviceSettings? deviceSettings)
{
this.mobileEmulationDeviceName = null;
- if (deviceSettings != null && string.IsNullOrEmpty(deviceSettings.UserAgent))+ if (deviceSettings == null)+ {+ this.mobileEmulationDeviceSettings = null;+ return;+ }++ if (string.IsNullOrEmpty(deviceSettings.UserAgent))
{
throw new ArgumentException("Device settings must include user agent string", nameof(deviceSettings));
}
this.mobileEmulationDeviceSettings = deviceSettings;
}
Apply this suggestion
Suggestion importance[1-10]: 5
Why: The suggestion improves null handling by separating the null check from the UserAgent validation, making the code more maintainable and clearer in its intent. However, the original code already handled nulls safely.
Value preferenceValue parameter of AddUserProfilePreference
Value preferenceValue parameter of AddLocalStatePreference
Each string element of AddWindowTypes
Value optionValue parameter of AddAdditionalChromiumOption
This is to avoid breaking users. There is no downstream issue on the C# side of passing in null (it will just get serialized normally). If there are exceptions on the backend, let users see it. We will add a null warning, but not throw an exception on our side here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Motivation and Context
Types of changes
Checklist
PR Type
Enhancement, Bug fix
Description
Annotated nullability for properties and methods in
ChromiumOptions.Replaced mutable fields with
readonlywhere applicable.Added exception handling for invalid arguments in public methods.
Refactored helper methods to use static and nullable parameters.
Changes walkthrough 📝
ChromiumOptions.cs
Added nullability annotations and refactored `ChromiumOptions`.dotnet/src/webdriver/Chromium/ChromiumOptions.cs
#nullable enable.readonlyfor immutability.