[dotnet] Align binary location property for FirefoxOptions with other options

[nvborisenko]

User description

Description

This is continuation of https://github.com/SeleniumHQ/selenium/issues/13885.

Motivation and Context

FirefoxOptions class contains two properties with no difference. It is confusing.

Probably we can just deprecate one of them to be aligned with other optionos? (chromium, safari)

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• [ ] I have read the contributing document.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

---

Type

enhancement

---

Description

• Renamed browserBinaryLocation to binaryLocation in FirefoxOptions to standardize property naming across different browser options.
• Deprecated BrowserExecutableLocation and redirected its usage to BinaryLocation, marking the former for future removal.
• Updated all references in the codebase from the old property name to the new one, ensuring consistency.

---

Changes walkthrough

Relevant files

Enhancement

FirefoxOptions.cs Align FirefoxOptions Binary Location Property Naming          dotnet/src/webdriver/Firefox/FirefoxOptions.cs Renamed browserBinaryLocation to binaryLocation to align with other browser options. Deprecated BrowserExecutableLocation property and redirected its getter and setter to BinaryLocation. Updated references to the renamed property in capability settings. +8/-7      DriverFactory.cs Update Driver Factory to Use New Binary Location Property dotnet/test/common/Environment/DriverFactory.cs Updated reference from BrowserExecutableLocation to BinaryLocation in the driver factory setup. +1/-1

---

✨ PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-code-review[bot]]

PR Description updated to latest commit (https://github.com/SeleniumHQ/selenium/commit/cd95b8324687539d6f0d74086524f299a35372cd)

• [ ] Copy walkthrough table to "Files Changed" Tab

[qodo-code-review[bot]]

PR Review

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward and limited to renaming and deprecating properties in a single class. The logic remains unchanged, making the review process less complex.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The PR does not mention updating any tests to reflect the changes in property names. This might lead to failures in existing tests that rely on the old property names.
🔒 Security concerns	No

---

✨ Review tool usage guide:

---

Overview: The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[qodo-code-review[bot]]

PR Code Suggestions

Category	Suggestions
Best practice	Improve thread safety by using thread-safe operations for property setters. To ensure thread safety and avoid potential race conditions in multi-threaded environments, consider using properties with backing fields that are thread-safe, such as using Interlocked operations or locks when setting the binaryLocation. dotnet/src/webdriver/Firefox/FirefoxOptions.cs [108] get { return this.binaryLocation; } -set { this.binaryLocation = value; } +set { Interlocked.Exchange(ref this.binaryLocation, value); }
	Improve type safety by checking the type before casting. To ensure type safety and avoid runtime errors, consider checking the type of options before casting it to FirefoxOptions. dotnet/test/common/Environment/DriverFactory.cs [120] -((FirefoxOptions)options).BinaryLocation = this.browserBinaryLocation; +if (options is FirefoxOptions firefoxOptions) +{ + firefoxOptions.BinaryLocation = this.browserBinaryLocation; +}
Maintainability	Ensure consistency by replacing all references to the deprecated property with the new property. Since the BinaryLocation property is now the main way to set the binary location, ensure that all references to the old BrowserExecutableLocation are replaced or refactored in the codebase to avoid confusion and maintain consistency. dotnet/src/webdriver/Firefox/FirefoxOptions.cs [114] -[Obsolete("Use BinaryLocation property instead of BrowserExecutableLocation. This one will be removed soon.")] +// Ensure all references in the codebase are updated: +// Replace all instances of `BrowserExecutableLocation` with `BinaryLocation`
	Enhance code clarity and maintainability by refactoring complex conditions into separate methods. To improve the clarity and maintainability of the ToCapabilities method, consider refactoring the condition checking and assignment into a separate method or using a more descriptive method name. dotnet/src/webdriver/Firefox/FirefoxOptions.cs [306-308] -if (!string.IsNullOrEmpty(this.binaryLocation)) +AddBinaryLocationToCapabilities(firefoxOptions); + +// Elsewhere in the class +private void AddBinaryLocationToCapabilities(Dictionary<string, object> capabilities) { - firefoxOptions[FirefoxBinaryCapability] = this.binaryLocation; + if (!string.IsNullOrEmpty(this.binaryLocation)) + { + capabilities[FirefoxBinaryCapability] = this.binaryLocation; + } }
Bug prevention	Prevent potential null reference exceptions by initializing strings with default values. To avoid null reference exceptions and ensure the binaryLocation is always in a valid state, consider initializing binaryLocation with a default value or ensure it is set before being accessed. dotnet/src/webdriver/Firefox/FirefoxOptions.cs [64] -private string binaryLocation; +private string binaryLocation = string.Empty; // Default to an empty string to prevent null reference issues.

---

✨ Improve tool usage guide:

---

Overview: The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.