Nullability for browsers

[asolntsev]

🔗 Related Issues

Partially implements #14291

💥 What does this PR do?

🔧 Implementation Notes

Added JSpecify nullability annotations to packages:

• org.openqa.selenium.edge
• org.openqa.selenium.ie
• org.openqa.selenium.firefox
• org.openqa.selenium.safari
• (and few more)

🔄 Types of changes

• Cleanup (formatting, renaming)

[qodo-code-review]

Review Summary by Qodo

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Add JSpecify nullability annotations to browser driver packages
• Move null-check logic to superclass DriverService for consistency
• Remove unused imports and unused logger fields across driver classes
• Improve error handling with better null checks and exception messages
• Add package-info.java files with @NullMarked annotation to packages

File Changes

1. java/src/org/openqa/selenium/chrome/ChromeDriverService.java ✨ Enhancement +1/-8

Simplify constructor by delegating null handling to superclass

java/src/org/openqa/selenium/chrome/ChromeDriverService.java

---

2. java/src/org/openqa/selenium/edge/EdgeDriverInfo.java Cleanup +0/-3

Remove unused logger field

java/src/org/openqa/selenium/edge/EdgeDriverInfo.java

---

3. java/src/org/openqa/selenium/edge/EdgeDriverService.java ✨ Enhancement +1/-1

Simplify constructor by delegating null handling to superclass

java/src/org/openqa/selenium/edge/EdgeDriverService.java

---

View more (34)

4. java/src/org/openqa/selenium/edge/package-info.java 📝 Documentation +50/-0

Add package-level nullability annotation with @NullMarked

java/src/org/openqa/selenium/edge/package-info.java

---

5. java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementAccount.java 📝 Documentation +10/-12

Add nullability annotations to constructor parameter and fields

java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementAccount.java

---

6. java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementDialog.java Cleanup +0/-2

Remove class-level @NullMarked annotation

java/src/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementDialog.java

---

7. java/src/org/openqa/selenium/federatedcredentialmanagement/HasFederatedCredentialManagement.java Cleanup +0/-2

Remove class-level @NullMarked annotation

java/src/org/openqa/selenium/federatedcredentialmanagement/HasFederatedCredentialManagement.java

---

8. java/src/org/openqa/selenium/federatedcredentialmanagement/package-info.java 📝 Documentation +50/-0

Add package-level nullability annotation with @NullMarked

java/src/org/openqa/selenium/federatedcredentialmanagement/package-info.java

---

9. java/src/org/openqa/selenium/firefox/AddHasContext.java ✨ Enhancement +1/-2

Use executeRequired method for non-null result handling

java/src/org/openqa/selenium/firefox/AddHasContext.java

---

10. java/src/org/openqa/selenium/firefox/AddHasExtensions.java ✨ Enhancement +3/-4

Use executeRequired method and simplify generic type syntax

java/src/org/openqa/selenium/firefox/AddHasExtensions.java

---

11. java/src/org/openqa/selenium/firefox/FileExtension.java Error handling +12/-8

Improve null safety with requireNonNull and better error messages

java/src/org/openqa/selenium/firefox/FileExtension.java

---

12. java/src/org/openqa/selenium/firefox/FirefoxCommandContext.java Error handling +5/-7

Improve error handling by throwing exception for unknown context

java/src/org/openqa/selenium/firefox/FirefoxCommandContext.java

---

13. java/src/org/openqa/selenium/firefox/FirefoxDriver.java Cleanup +2/-2

Remove @nonnull annotation and add TODO comment for null check

java/src/org/openqa/selenium/firefox/FirefoxDriver.java

---

14. java/src/org/openqa/selenium/firefox/FirefoxDriverLogLevel.java 📝 Documentation +4/-1

Add @nullable annotations to fromString and fromJson methods

java/src/org/openqa/selenium/firefox/FirefoxDriverLogLevel.java

---

15. java/src/org/openqa/selenium/firefox/FirefoxDriverService.java ✨ Enhancement +1/-1

Change constructor visibility from public to protected

java/src/org/openqa/selenium/firefox/FirefoxDriverService.java

---

16. java/src/org/openqa/selenium/firefox/FirefoxOptions.java 📝 Documentation +11/-2

Add @nullable annotations to multiple methods and mirror implementations

java/src/org/openqa/selenium/firefox/FirefoxOptions.java

---

17. java/src/org/openqa/selenium/firefox/FirefoxProfile.java 📝 Documentation +9/-6

Add @nullable annotations to fields and parameters with null checks

java/src/org/openqa/selenium/firefox/FirefoxProfile.java

---

18. java/src/org/openqa/selenium/firefox/GeckoDriverInfo.java Cleanup +0/-3

Remove unused logger field

java/src/org/openqa/selenium/firefox/GeckoDriverInfo.java

---

19. java/src/org/openqa/selenium/firefox/Preferences.java 📝 Documentation +2/-1

Add @nullable annotation to getPreference method

java/src/org/openqa/selenium/firefox/Preferences.java

---

20. java/src/org/openqa/selenium/firefox/ProfilesIni.java 📝 Documentation +7/-2

Add @nullable annotations to methods and parameters

java/src/org/openqa/selenium/firefox/ProfilesIni.java

---

21. java/src/org/openqa/selenium/firefox/package-info.java 📝 Documentation +21/-0

Add package-level nullability annotation with @NullMarked

java/src/org/openqa/selenium/firefox/package-info.java

---

22. java/src/org/openqa/selenium/ie/ElementScrollBehavior.java 📝 Documentation +3/-0

Add @nullable annotation to fromString method

java/src/org/openqa/selenium/ie/ElementScrollBehavior.java

---

23. java/src/org/openqa/selenium/ie/InternetExplorerDriverInfo.java Cleanup +0/-3

Remove unused logger field

java/src/org/openqa/selenium/ie/InternetExplorerDriverInfo.java

---

24. java/src/org/openqa/selenium/ie/InternetExplorerDriverService.java ✨ Enhancement +1/-8

Simplify constructor by delegating null handling to superclass

java/src/org/openqa/selenium/ie/InternetExplorerDriverService.java

---

25. java/src/org/openqa/selenium/ie/InternetExplorerOptions.java 📝 Documentation +2/-0

Add @nullable annotation to getExtraCapability method

java/src/org/openqa/selenium/ie/InternetExplorerOptions.java

---

26. java/src/org/openqa/selenium/ie/package-info.java 📝 Documentation +50/-0

Add package-level nullability annotation with @NullMarked

java/src/org/openqa/selenium/ie/package-info.java

---

27. java/src/org/openqa/selenium/print/PageMargin.java Cleanup +0/-2

Remove class-level @NullMarked annotation

java/src/org/openqa/selenium/print/PageMargin.java

---

28. java/src/org/openqa/selenium/print/PageSize.java ✨ Enhancement +2/-5

Replace manual null check with Require.nonNull utility

java/src/org/openqa/selenium/print/PageSize.java

---

29. java/src/org/openqa/selenium/print/PrintOptions.java Cleanup +0/-2

Remove class-level @NullMarked annotation

java/src/org/openqa/selenium/print/PrintOptions.java

---

30. java/src/org/openqa/selenium/print/package-info.java 📝 Documentation +50/-0

Add package-level nullability annotation with @NullMarked

java/src/org/openqa/selenium/print/package-info.java

---

31. java/src/org/openqa/selenium/remote/service/DriverService.java ✨ Enhancement +3/-2

Move null-check logic to superclass for args and environment

java/src/org/openqa/selenium/remote/service/DriverService.java

---

32. java/src/org/openqa/selenium/safari/AddHasPermissions.java ✨ Enhancement +6/-12

Use executeRequired method and simplify null checking logic

java/src/org/openqa/selenium/safari/AddHasPermissions.java

---

33. java/src/org/openqa/selenium/safari/SafariDriverInfo.java Cleanup +0/-3

Remove unused logger field

java/src/org/openqa/selenium/safari/SafariDriverInfo.java

---

34. java/src/org/openqa/selenium/safari/SafariDriverService.java ✨ Enhancement +1/-1

Simplify constructor by delegating null handling to superclass

java/src/org/openqa/selenium/safari/SafariDriverService.java

---

35. java/src/org/openqa/selenium/safari/SafariOptions.java ✨ Enhancement +3/-1

Simplify boolean logic and add @nullable annotation

java/src/org/openqa/selenium/safari/SafariOptions.java

---

36. java/src/org/openqa/selenium/safari/SafariTechPreviewDriverService.java 📝 Documentation +3/-2

Add @nullable annotation to diagnose field and simplify constructor

java/src/org/openqa/selenium/safari/SafariTechPreviewDriverService.java

---

37. java/src/org/openqa/selenium/safari/package-info.java 📝 Documentation +50/-0

Add package-level nullability annotation with @NullMarked

java/src/org/openqa/selenium/safari/package-info.java

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (3) 📎 Requirement gaps (0)

1. FirefoxDriverService ctor restricted 📘 Rule violation ✓ Correctness

Description

The constructor visibility was changed from public to protected, which is an API-breaking change
for external users who subclass or instantiate this type. This violates the requirement to maintain
backward-compatible public interfaces.

Code

java/src/org/openqa/selenium/firefox/FirefoxDriverService.java[R38-41]

+  protected FirefoxDriverService(
      @Nullable File executable,
      int port,
      @Nullable Duration timeout,

Evidence

PR Compliance ID 2 requires maintaining backward-compatible public interfaces. The diff changes the
FirefoxDriverService constructor access modifier to protected, reducing accessibility and
breaking source compatibility for downstream code.

AGENTS.md
java/src/org/openqa/selenium/firefox/FirefoxDriverService.java[38-42]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`FirefoxDriverService`'s constructor visibility changed from `public` to `protected`, which is a breaking API change for external clients.

## Issue Context
This PR must preserve API/ABI compatibility for public interfaces unless explicitly allowed.

## Fix Focus Areas
- java/src/org/openqa/selenium/firefox/FirefoxDriverService.java[38-42]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. fromString now throws 📘 Rule violation ✓ Correctness

Description

FirefoxCommandContext.fromString no longer returns null for unknown input and now throws
IllegalArgumentException, which can break callers relying on the prior behavior. This is a
backward-incompatible user-visible behavior change.

Code

java/src/org/openqa/selenium/firefox/FirefoxCommandContext.java[R39-44]

+    for (FirefoxCommandContext b : FirefoxCommandContext.values()) {
+      if (text.equalsIgnoreCase(b.text)) {
+        return b;
      }
    }
-    return null;
+    throw new IllegalArgumentException("Unknown Firefox context: " + text);

Evidence

PR Compliance ID 2 requires backward-compatible signatures/behavior for public APIs. The updated
implementation now throws an exception instead of returning null when the context string is
unrecognized.

AGENTS.md
java/src/org/openqa/selenium/firefox/FirefoxCommandContext.java[39-44]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`FirefoxCommandContext.fromString` changed behavior from returning `null` on unknown input to throwing `IllegalArgumentException`, which breaks existing callers.

## Issue Context
API/ABI compatibility must be maintained for public interfaces unless a breaking change is explicitly intended and communicated.

## Fix Focus Areas
- java/src/org/openqa/selenium/firefox/FirefoxCommandContext.java[39-44]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. ProfilesIni null dereference 🐞 Bug ✓ Correctness

Description

ProfilesIni can pass a null appData into readProfiles(), which immediately dereferences it via new
File(appData, ...), causing a NullPointerException in environments where the Firefox profile
directory is absent. This can break FirefoxOptions.configureFromEnv when webdriver.firefox.profile
is set, turning an expected “profile not found” flow into an NPE.

Code

java/src/org/openqa/selenium/firefox/ProfilesIni.java[R45-48]

+  protected Map<String, File> readProfiles(@Nullable File appData) {
    Map<String, File> toReturn = new HashMap<>();

    File profilesIni = new File(appData, "profiles.ini");

Evidence

locateAppDataDirectory explicitly returns null when the expected Firefox data directory does not
exist; ProfilesIni’s constructor passes that value into readProfiles; readProfiles then dereferences
appData without a guard. FirefoxOptions.configureFromEnv calls new ProfilesIni() when a profile
system property is set, so this NPE can occur in a real user-facing path.

java/src/org/openqa/selenium/firefox/ProfilesIni.java[40-49]
java/src/org/openqa/selenium/firefox/ProfilesIni.java[134-155]
java/src/org/openqa/selenium/firefox/FirefoxOptions.java[142-145]
java/src/org/openqa/selenium/firefox/ProfilesIni.java[99-105]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ProfilesIni.locateAppDataDirectory` can return `null`, but `ProfilesIni.readProfiles` and `ProfilesIni.newProfile` dereference `appData` unconditionally in `new File(appData, ...)`, leading to a runtime `NullPointerException`.

### Issue Context
This can occur on CI/containers or machines without a Firefox profile directory, and is reachable via `FirefoxOptions.configureFromEnv()` when `webdriver.firefox.profile` is set.

### Fix Focus Areas
- java/src/org/openqa/selenium/firefox/ProfilesIni.java[40-106]
- java/src/org/openqa/selenium/firefox/ProfilesIni.java[134-165]
- java/src/org/openqa/selenium/firefox/FirefoxOptions.java[142-151]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Package Javadoc restates "what" 📘 Rule violation ✓ Correctness

Description

New package-info.java documentation primarily explains implementation details and general
mechanics rather than rationale/intent for the package, increasing noise and potential confusion.
This violates the guideline that comments should focus on why rather than restating what/how.

Code

java/src/org/openqa/selenium/edge/package-info.java[R18-24]

+/**
+ * Mechanisms to configure and run selenium via the command line. There are two key classes {@link
+ * org.openqa.selenium.cli.CliCommand} and {@link org.openqa.selenium.grid.config.HasRoles}.
+ * Ultimately, these are used to build a {@link org.openqa.selenium.grid.config.Config} instance,
+ * for which there are strongly-typed role-specific classes that use a {@code Config}, such as
+ * {@link org.openqa.selenium.grid.node.docker.DockerOptions}.
+ *

Evidence

PR Compliance ID 8 requires comments to explain rationale (why) instead of restating implementation
details (what/how). The newly added package-level Javadoc is a long procedural description and does
not capture rationale/intent specific to the org.openqa.selenium.edge package.

AGENTS.md
java/src/org/openqa/selenium/edge/package-info.java[18-24]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New package-level Javadocs focus on procedural/implementation detail instead of explaining package intent/rationale.

## Issue Context
Comments should explain `why` (intent/constraints) rather than repeating `what/how`.

## Fix Focus Areas
- java/src/org/openqa/selenium/edge/package-info.java[18-24]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Safari permissions cast regression 🐞 Bug ⛯ Reliability

Description

Safari AddHasPermissions.getPermissions now assumes the remote response is a Map and performs an
unchecked cast via assignment from executeRequired, losing the previous explicit type validation. If
the response is non-Map (or changes), this will fail with a ClassCastException and reduced
diagnostic clarity.

Code

java/src/org/openqa/selenium/safari/AddHasPermissions.java[R69-76]

      public Map<String, Boolean> getPermissions() {
-        Object resultObject = executeMethod.execute(GET_PERMISSIONS, null);
+        Map<?, ?> resultMap = executeMethod.executeRequired(GET_PERMISSIONS, null);

-        if (resultObject instanceof Map<?, ?>) {
-          Map<?, ?> resultMap = (Map<?, ?>) resultObject;
-          Map<String, Boolean> permissionMap = new HashMap<>();
-          for (Map.Entry<?, ?> entry : resultMap.entrySet()) {
-            if (entry.getKey() instanceof String && entry.getValue() instanceof Boolean) {
-              permissionMap.put((String) entry.getKey(), (Boolean) entry.getValue());
-            }
+        Map<String, Boolean> permissionMap = new HashMap<>();
+        for (Map.Entry<?, ?> entry : resultMap.entrySet()) {
+          if (entry.getKey() instanceof String && entry.getValue() instanceof Boolean) {
+            permissionMap.put((String) entry.getKey(), (Boolean) entry.getValue());
          }
-          return permissionMap;
-        } else {
-          throw new IllegalStateException(
-              "Unexpected result type: " + resultObject.getClass().getName());

Evidence

executeRequired only enforces non-null; AddHasPermissions assigns the result directly to Map<?,?>
with no instanceof check, so a non-Map response will throw ClassCastException at runtime without a
tailored message.

java/src/org/openqa/selenium/safari/AddHasPermissions.java[69-78]
java/src/org/openqa/selenium/remote/ExecuteMethod.java[40-44]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`AddHasPermissions.getPermissions()` now relies on an implicit cast from `executeRequired(...)` to `Map<?, ?>` without validating the response type. Unexpected responses will surface as `ClassCastException` with poor diagnostics.

### Issue Context
`ExecuteMethod.executeRequired` only checks non-null; it does not validate the runtime type of the returned value.

### Fix Focus Areas
- java/src/org/openqa/selenium/safari/AddHasPermissions.java[68-79]
- java/src/org/openqa/selenium/remote/ExecuteMethod.java[40-44]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more