[java] Check the resolved driver for JS support in By.getJavascriptExecutor()

[Mochxd]

🔗 Related Issues

No existing issue. I hit this while looking at how relative locators resolve their search context.

💥 What does this PR do?

By.getJavascriptExecutor() resolves the driver with getWebDriver(context), which unwraps a WrapsDriver, but then checks context instanceof JavascriptExecutor and casts driver. The guard and the cast look at two different objects.

The practical effect is that element-scoped relative locators throw. RemoteWebElement implements WrapsDriver but not JavascriptExecutor, so:

element.findElements(with(tagName("p")).below(other));

fails with IllegalArgumentException: Context does not provide a mechanism to execute JS: ... even though the driver it just unwrapped can execute JavaScript perfectly well. The same call works when made on the driver.

The mismatch can also turn an intended IllegalArgumentException into a ClassCastException if a context is itself a JavascriptExecutor while the driver it wraps is not.

Checking driver instead of context fixes both, and the rejection path is unchanged when the resolved driver genuinely can't run JavaScript.

🔧 Implementation Notes

One-word change, since getWebDriver() already does the unwrapping. Left the exception message pointing at context, as that's the object the caller passed in.

Both call sites already treat this as driver-based: RelativeLocator uses the returned executor directly, and RelativeLocatorServerSide calls getWebDriver(context) on the next line.

A unit test covers a context that wraps a JavaScript-capable driver, which fails before this change. It reuses the existing StubDriver rather than a mock, which meant adding :helpers to the SmallTests deps since that stub lives in a separate target.

💡 Additional Considerations

I couldn't get Bazel running locally, so I compiled the test against these sources and ran it by hand instead: it passes with this change, and fails with the original context check restored (IllegalArgumentException: Context does not provide a mechanism to execute JS). It's a plain unit test and doesn't need a browser.

What that doesn't cover is the Bazel wiring, since running it that way sidesteps strict deps...so the :helpers line is worth a glance. I followed the pattern already used by the small suite in java/test/org/openqa/selenium/interactions/BUILD.bazel.

Relative locators against a ShadowRoot still won't work, since ShadowRoot doesn't wrap a driver. That's #13634 and needs a different approach.

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Fix By.getJavascriptExecutor() to validate unwrapped driver JS support

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Fix JS executor resolution to validate the unwrapped WebDriver, not the SearchContext
• Prevent element-scoped relative locators from failing when context wraps a JS-capable driver
• Add unit tests covering wrapped-driver acceptance and non-JS driver rejection

Diagram

graph TD
  A["By.getJavascriptExecutor(context)"] --> B["getWebDriver(context)"] --> C["Resolved WebDriver"] --> D{{"driver is JavascriptExecutor?"}}
  D -->|Yes| E["Return JavascriptExecutor (driver)"]
  D -->|No| F["Throw IllegalArgumentException"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Check both context and resolved driver

• ➕ Preserves any edge semantics where a context directly provides JS execution independent of the wrapped driver
• ➕ Could make error messages more specific to which object lacks capability
• ➖ Adds complexity without clear benefit for Selenium’s typical model (execution is driver-based)
• ➖ Still requires choosing which executor to return when they differ

2. Move JS capability logic into getWebDriver() / return Optional executor

• ➕ Centralizes unwrapping + capability validation in one helper
• ➕ Reduces risk of future mismatched guard/cast patterns elsewhere
• ➖ More invasive API/behavioral surface change for a one-line bug
• ➖ May require refactoring multiple call sites and tests

Recommendation: Keep the PR’s approach: validate JavascriptExecutor on the resolved (unwrapped) WebDriver. It directly fixes the guard/cast mismatch with minimal behavioral change and aligns with how call sites already treat JS execution as driver-based.

Files changed (2) +29 / -1

Bug fix (1) +1 / -1

By.javaValidate JS support on unwrapped WebDriver in getJavascriptExecutor() +1/-1

Validate JS support on unwrapped WebDriver in getJavascriptExecutor()

• Fixes a guard/cast mismatch by checking whether the resolved WebDriver (after WrapsDriver unwrapping) implements JavascriptExecutor. Prevents false negatives for element contexts that wrap a JS-capable driver and avoids potential ClassCastException scenarios.

java/src/org/openqa/selenium/By.java

Tests (1) +28 / -0

ByTest.javaAdd unit coverage for wrapped-driver JS executor resolution +28/-0

Add unit coverage for wrapped-driver JS executor resolution

• Adds tests verifying that a SearchContext wrapping a JavascriptExecutor-capable driver is accepted and that contexts whose resolved drivers cannot execute JS still throw IllegalArgumentException. Introduces small test-only interfaces to model RemoteWebDriver/RemoteWebElement-like behavior.

java/test/org/openqa/selenium/ByTest.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

1. No cross-binding comparison noted 📘 Rule violation ≡ Correctness ⭐ New

Description

This change alters user-visible Java behavior for relative locators by accepting wrapped contexts
whose unwrapped driver supports JavaScript, but the PR does not document any comparison with other
language bindings. Without an explicit cross-binding check, similar APIs may diverge in observable
behavior across Selenium bindings.

Code

java/src/org/openqa/selenium/By.java[153]

+    if (!(driver instanceof JavascriptExecutor)) {

Evidence

PR Compliance ID 389265 requires documenting a cross-language comparison when changing user-visible
binding behavior. The diff changes the JS-executor capability check to use the resolved driver
rather than the caller-provided context (behavior change), and adds a regression test
demonstrating the newly-supported wrapped-context scenario, but adds no cross-binding comparison
note or documentation.

Rule 389265: Compare cross-language bindings when changing user-visible behavior
java/src/org/openqa/selenium/By.java[153-156]
java/test/org/openqa/selenium/ByTest.java[188-194]

Agent prompt

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

## Issue description
A user-visible behavior change was made in the Java binding (`By.getJavascriptExecutor()`), but there is no evidence in-code (comments/docs) that the behavior was compared with at least one other Selenium language binding as required.

## Issue Context
PR Compliance requires verifying cross-language consistency (or documenting intentional divergence) when changing user-visible behavior in a binding.

## Fix Focus Areas
- java/src/org/openqa/selenium/By.java[153-156]
- java/test/org/openqa/selenium/ByTest.java[188-199]

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

---

2. Mockito mocks in ByTest ✓ Resolved 📘 Rule violation ▣ Testability

Description

The new regression tests rely on Mockito (mock, when) instead of real/contract-driven
integrations or simple fakes. This can make tests less representative of real behavior and violates
the project's no-mocks testing guidance.

Code

java/test/org/openqa/selenium/ByTest.java[R191-193]

+    JavascriptCapableDriver driver = mock(JavascriptCapableDriver.class);
+    DriverWrappingContext context = mock(DriverWrappingContext.class);
+    when(context.getWrappedDriver()).thenReturn(driver);

Evidence

PR Compliance ID 389270 disallows use of mocking frameworks like Mockito in tests unless
contract-driven; the added tests explicitly import and use Mockito (when(...) and mock(...)).

Rule 389270: Avoid mocks in tests; use real or contract-driven integrations
java/test/org/openqa/selenium/ByTest.java[20-27]
java/test/org/openqa/selenium/ByTest.java[189-205]

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 tests added in `ByTest` use Mockito mocks (`mock`, `when`), which violates the rule to avoid mocks in tests unless they are contract-driven.

## Issue Context
The regression tests can likely be implemented using simple in-memory fakes/stubs (e.g., reuse existing `StubDriver`) and small inline implementations for `SearchContext`/`WrapsDriver`, without Mockito.

## Fix Focus Areas
- java/test/org/openqa/selenium/ByTest.java[189-205]

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

---

Context

✅ Compliance rules (platform): 18 rules

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit bdc7fc4 ⚖️ Balanced

Results up to commit 9ce385e ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Mockito mocks in ByTest ✓ Resolved 📘 Rule violation ▣ Testability

Description

The new regression tests rely on Mockito (mock, when) instead of real/contract-driven
integrations or simple fakes. This can make tests less representative of real behavior and violates
the project's no-mocks testing guidance.

Code

java/test/org/openqa/selenium/ByTest.java[R191-193]

+    JavascriptCapableDriver driver = mock(JavascriptCapableDriver.class);
+    DriverWrappingContext context = mock(DriverWrappingContext.class);
+    when(context.getWrappedDriver()).thenReturn(driver);

Evidence

PR Compliance ID 389270 disallows use of mocking frameworks like Mockito in tests unless
contract-driven; the added tests explicitly import and use Mockito (when(...) and mock(...)).

Rule 389270: Avoid mocks in tests; use real or contract-driven integrations
java/test/org/openqa/selenium/ByTest.java[20-27]
java/test/org/openqa/selenium/ByTest.java[189-205]

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 tests added in `ByTest` use Mockito mocks (`mock`, `when`), which violates the rule to avoid mocks in tests unless they are contract-driven.

## Issue Context
The regression tests can likely be implemented using simple in-memory fakes/stubs (e.g., reuse existing `StubDriver`) and small inline implementations for `SearchContext`/`WrapsDriver`, without Mockito.

## Fix Focus Areas
- java/test/org/openqa/selenium/ByTest.java[189-205]

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

---

[qodo-code-review]

1. No cross-binding comparison noted 📘 Rule violation ≡ Correctness

This change alters user-visible Java behavior for relative locators by accepting wrapped contexts
whose unwrapped driver supports JavaScript, but the PR does not document any comparison with other
language bindings. Without an explicit cross-binding check, similar APIs may diverge in observable
behavior across Selenium bindings.

Agent Prompt

## Issue description
A user-visible behavior change was made in the Java binding (`By.getJavascriptExecutor()`), but there is no evidence in-code (comments/docs) that the behavior was compared with at least one other Selenium language binding as required.

## Issue Context
PR Compliance requires verifying cross-language consistency (or documenting intentional divergence) when changing user-visible behavior in a binding.

## Fix Focus Areas
- java/src/org/openqa/selenium/By.java[153-156]
- java/test/org/openqa/selenium/ByTest.java[188-199]

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

[qodo-code-review]

Code review by qodo was updated up to the latest commit bdc7fc4