[java][dotnet] Support relative locators from elements and shadow roots

[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, when a context is itself a JavascriptExecutor but the driver it wraps is not.

Checking driver instead of context fixes both. On its own that would have been incomplete: both relative-locator call sites invoke the FIND_ELEMENTS atom without a root, so the atom falls back to document and an element-scoped call would silently search the whole page. This PR also passes the search context as the atom root when it is not the driver, on both the client and server-side paths.

🔧 Implementation Notes

• By.getJavascriptExecutor(): one-word change. Left the exception message pointing at context, as that's the object the caller passed in.
• RelativeLocator / RelativeLocatorServerSide: pass context as the atom's second argument when it is not a WebDriver; pass null for a driver context so the atom keeps using the document.
• No change to javascript/atoms/. The atom already accepts an optional root and already scopes candidate search with it.

A unit test covers a context that wraps a JavaScript-capable driver. It reuses StubDriver rather than a mock, which meant adding :helpers to the SmallTests deps.

A browser test covers the scoped-search path: two matching candidates, one inside the context element and one outside. Element-scoped search returns only the inner one; driver-level search on the same page still returns both.

💡 Additional Considerations

For comparison, .NET already unwraps IWrapsDriver in RelativeBy.GetExecutor when the context itself isn't an IJavaScriptExecutor. Its documented contract is "context is not IJavaScriptExecutor or wraps a driver that does." This brings Java in line with that.

One difference I'm deliberately leaving alone: .NET walks the wrapper chain; getWebDriver() unwraps a single level.

Python doesn't hit this, since RelativeBy is only handled at the driver level.

On shadow roots: I had this wrong in an earlier version of this description. ShadowRoot does implement WrapsDriver, and RelativeBy is not By.Remotable, so ElementLocation falls through to locator.findElements(context) with the shadow root as the context. It therefore reaches this path and is passed as the atom root, where querySelectorAll scopes it correctly. I would still not call shadow DOM covered: there is no fixture for it here, and XPath inside a shadow tree is a separate weak spot in the atom.

🔄 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 (0) 📜 Skill insights (0)

1. No cross-binding comparison noted ✗ Dismissed 📘 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 sources

Review mode: ⚖️ Balanced: This push adds behavior across Java shadow-root routing, TypeScript locator resolution, and integration tests, creating real cross-layer correctness risk but not enough independent logic to warrant extended review.

Tip of the day

💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit b0f9598

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]

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

[Mochxd]

Quick follow-up, since this has been sitting a while.

CI never started on this fork PR (the workflow runs are action_required with no jobs). If someone with write access can approve the workflows, that would help a lot.

On the cross-binding note: I compared this with .NET. RelativeBy.GetExecutor in dotnet/src/webdriver/RelativeBy.cs already falls back to IWrapsDriver when the search context itself is not an IJavaScriptExecutor. The documented contract is "context is not IJavaScriptExecutor or wraps a driver that does." So this Java change matches that behavior. .NET also walks the wrapper chain; getWebDriver() only unwraps one level, and I left that alone.

Python does not hit this path, since RelativeBy is handled on the driver rather than on elements.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[Mochxd]

@diemol @pujagani would you have a few minutes to review this? It was suggested I also ping @SeleniumHQ/selenium-tlc.

Small Java fix: By.getJavascriptExecutor() was checking the search context for JS support after already unwrapping the driver, so element-scoped relative locators throw even when the driver can run JS. CI on this fork PR is still waiting for workflow approval.

[qodo-code-review]

Code Review by Qodo

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

1. Single shadow finds lack coverage 📘 Rule violation ☼ Reliability

Description

shouldBeAbleToRootASearchWithinAShadowRoot sends only the plural /shadow/{shadowId}/elements
request, despite the handler adding a separate singular route. A defect in the singular matcher,
context selection, or response shape would therefore pass while shadow-root users call
findElement.

Code

java/test/org/openqa/selenium/grid/node/CustomLocatorHandlerTest.java[R321-327]

+    HttpRequest request =
+        new HttpRequest(POST, "/session/1234/shadow/shadow-1234/elements")
+            .setContent(
+                Contents.asJson(
+                    Map.of(
+                        "using", "cheese",
+                        "value", "tasty")));

Evidence

Compliance rule 5 requires focused regression coverage for altered behavior. The handler introduces
distinct singular and plural shadow-root branches, while the new test invokes and validates only the
plural branch.

AGENTS.md: Provide Focused Tests Without Contract-Distorting Mocks
java/src/org/openqa/selenium/grid/node/CustomLocatorHandler.java[219-227]
java/test/org/openqa/selenium/grid/node/CustomLocatorHandlerTest.java[321-336]

Agent prompt

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

## Issue description
The new custom-locator routing supports both singular and plural shadow-root find endpoints, but its regression test exercises only the plural endpoint.

## Fix Focus Areas
- java/test/org/openqa/selenium/grid/node/CustomLocatorHandlerTest.java[287-338]

## Recommended Fix
Add a focused test that sends a custom locator to `/session/{sessionId}/shadow/{shadowId}/element`, verifies that the handler matches and executes it, and asserts the singular element response and identifier.

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

---

2. Shadow tag searches match extra elements ✓ Resolved 📘 Rule violation ≡ Correctness

Description

tagNameMany passes target directly to querySelectorAll when a root lacks
getElementsByTagName, parsing a literal tag-name value as CSS instead of preserving native
tag-name matching. Relative searches rooted at a ShadowRoot therefore let values such as
div.foo, p > span, or p,span select descendants that document and element searches would not
match, while values such as p:unsupported can instead reach an invalid-selector DOMException;
the added test covers only the CSS-compatible value p.

Code

javascript/atoms/typescript/find-elements.ts[99]

+    return Array.from(root.querySelectorAll(target))

Evidence

The fallback is specifically reached for ShadowRoot, which provides querySelectorAll but not
getElementsByTagName. Unlike the retained native branch and the existing tag-name atom's literal
lookup, CSS parsing recognizes combinators and selector lists and can reject unsupported
pseudo-selectors; Java also accepts every nonempty tag-name value and keeps tag-name and CSS
strategies distinct. The new ShadowRoot test uses only plain p, for which literal tag matching and
CSS selection behave identically, so it does not expose the inconsistent behavior required to remain
uniform across roots and bindings.

AGENTS.md: Maintain Cross-Binding Consistency for User-Visible and Shared Behavior
javascript/atoms/typescript/find-elements.ts[92-99]
javascript/atoms/locators/tag_name.js[42-52]
java/src/org/openqa/selenium/By.java[256-272]
dotnet/test/webdriver/ElementFindingTests.cs[243-254]
javascript/atoms/test/find_elements_typescript_test.html[207-215]
javascript/atoms/typescript/find-elements.ts[90-99]
java/src/org/openqa/selenium/By.java[256-273]

Agent prompt

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

## Issue description
The ShadowRoot fallback interprets tag-name locator values as CSS selectors, which can broaden matches or throw selector-parsing exceptions instead of preserving the literal tag-name behavior used for documents, elements, and other bindings.

## Fix Focus Areas
- javascript/atoms/typescript/find-elements.ts[95-99]
- javascript/atoms/test/find_elements_typescript_test.html[207-215]

## Recommended Fix
Retain the native `getElementsByTagName` branch where it is available, but replace the raw `querySelectorAll(target)` fallback with literal tag-name matching, such as enumerating descendants with `querySelectorAll('*')` and filtering by the appropriate tag-name value. Add focused ShadowRoot coverage using CSS-looking values such as `div.foo`, `p > span`, or `p,span` to verify that selector syntax does not broaden the search, and include an invalid CSS selector candidate such as `p:unsupported` to verify that tag-name lookup does not throw a selector-parsing exception.

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

---

3. Single-result child finds can regress ✓ Resolved 📘 Rule violation ☼ Reliability

Description

childFindKeepsRelativeLocatorValueAsJsonObject exercises only FIND_CHILD_ELEMENTS, although the
singular FIND_CHILD_ELEMENT path also changed from String to Object. A future divergence in
RemoteWebElement.findElement or its command factory could stringify the nested relative-locator
map without any test detecting the malformed request.

Code

java/test/org/openqa/selenium/remote/codec/w3c/W3CHttpCommandCodecTest.java[R80-82]

+    HttpRequest request =
+        codec.encode(
+            new Command(sessionId, DriverCommand.FIND_CHILD_ELEMENTS("scope", "relative", value)));

Evidence

Compliance rule 5 requires focused coverage for new or changed behavior. The added test encodes only
FIND_CHILD_ELEMENTS, leaving the separately changed singular command path unverified.

AGENTS.md: Add Focused Tests and Avoid Contract-Misrepresenting Mocks
java/test/org/openqa/selenium/remote/codec/w3c/W3CHttpCommandCodecTest.java[69-89]
java/src/org/openqa/selenium/remote/DriverCommand.java[258-264]
java/src/org/openqa/selenium/remote/RemoteWebElement.java[210-212]

Agent prompt

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

## Issue description
The serialization test covers only plural child finds, while this PR changes both singular and plural command paths to preserve nested relative-locator maps.

## Fix Focus Areas
- java/test/org/openqa/selenium/remote/codec/w3c/W3CHttpCommandCodecTest.java[69-89]

## Recommended Fix
Parameterize the test over `FIND_CHILD_ELEMENT` and `FIND_CHILD_ELEMENTS`, or add a focused singular-path test that encodes the same nested map and verifies `value` remains a JSON object.

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

---

View high (4)

4. Shadow-root tag searches fail ✓ Resolved 🐞 Bug ≡ Correctness

Description

RelativeBy.findElements passes every non-WebDriver context as atomRoot, including Selenium's
ShadowRoot, but the FIND_ELEMENTS atom calls root.getElementsByTagName() for tag-name
locators. A relative By.tagName(...) lookup from a shadow root therefore throws in the browser
because ShadowRoot has no getElementsByTagName, while remote argument conversion preserves the
context as a shadow-root reference.

Code

java/src/org/openqa/selenium/support/locators/RelativeLocator.java[239]

+      Object atomRoot = context instanceof WebDriver ? null : context;

Evidence

ShadowRoot is a non-driver SearchContext, so the added condition supplies it as the atom root.
Script argument conversion serializes it as the W3C shadow-root reference, and the browser-side atom
then invokes an API absent from ShadowRoot for a tag-name locator.

java/src/org/openqa/selenium/remote/ShadowRoot.java[35-42]
java/src/org/openqa/selenium/remote/ShadowRoot.java[61-71]
java/src/org/openqa/selenium/remote/WebElementToJsonConverter.java[49-59]
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[578-582]
javascript/atoms/typescript/find-elements.ts[18-21]
javascript/atoms/typescript/find-elements.ts[91-96]

Agent prompt

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

## Issue description
Passing a `ShadowRoot` as the relative-locator atom root breaks tag-name relative lookups because the atom assumes every root implements `getElementsByTagName()`.

## Fix Focus Areas
- javascript/atoms/typescript/find-elements.ts[18-21]
- javascript/atoms/typescript/find-elements.ts[91-96]
- java/src/org/openqa/selenium/support/locators/RelativeLocator.java[238-244]

## Recommended Fix
Extend the atom root type and tag-name lookup to support `ShadowRoot`. For example, use `querySelectorAll(target)` for shadow-root tag-name searches (or use a common query mechanism valid for Document, Element, and ShadowRoot), then add a browser test that invokes a relative `By.tagName` locator from `element.getShadowRoot()`.

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

---

5. Scoped locators use outside anchors ✓ Resolved 🐞 Bug ≡ Correctness

Description

findElements passes context for candidate enumeration, but the atom's resolveAnchor still
calls findElements(selector) without that root. Element contexts can therefore resolve a duplicate
selector outside their subtree, while shadow-root contexts cannot resolve locator-based anchors
inside the shadow tree at all.

Code

java/src/org/openqa/selenium/support/locators/RelativeLocator.java[R243-244]

+          (List<WebElement>)
+              js.executeScript(FIND_ELEMENTS, asAtomLocatorParameter(this), atomRoot);

Evidence

The Java change supplies the context as the atom root, and relativeMany uses that root only to
find candidate elements. Every filter anchor and the final sorting anchor are resolved through
resolveAnchor, whose nested findElements call omits the root and consequently defaults to
document.

java/src/org/openqa/selenium/support/locators/RelativeLocator.java[235-244]
javascript/atoms/typescript/find-elements.ts[183-201]
javascript/atoms/typescript/find-elements.ts[251-288]
javascript/atoms/typescript/find-elements.ts[291-322]

Agent prompt

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

## Issue description
Relative-locator candidates are now searched beneath the supplied context, but locator-based anchors are still resolved against the document. This can select an anchor outside an element context and prevents document lookup from finding anchors inside a shadow root.

## Fix Focus Areas
- javascript/atoms/typescript/find-elements.ts[183-201]
- javascript/atoms/typescript/find-elements.ts[251-288]
- java/test/org/openqa/selenium/support/locators/RelativeLocatorTest.java[408-442]

## Recommended Fix
Thread the active atom root through `resolveAnchor`, proximity-filter construction, filtering, and final proximity sorting. Add coverage using a locator-based anchor with a conflicting outside match, plus a shadow-root case if supported by this test suite.

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

---

6. Wrapped contexts fail relative searches 📘 Rule violation ≡ Correctness

Description

atomRoot forwards every non-WebDriver SearchContext directly to executeScript, even though
its argument converter supports only script-serializable values rather than arbitrary
SearchContext or WrapsDriver implementations. When either relative-locator implementation
receives a custom driver-wrapping context accepted by getJavascriptExecutor, conversion throws
IllegalArgumentException before the atom runs, while the added browser coverage exercises only
WebElement and driver contexts.

Code

java/src/org/openqa/selenium/support/locators/RelativeLocator.java[239]

+      Object atomRoot = context instanceof WebDriver ? null : context;

Evidence

By.getWebDriver accepts any WrapsDriver, but both relative-locator implementations subsequently
pass the original non-driver context to executeScript. WebElementToJsonConverter handles remote
elements, shadow roots, wrapped elements, collections, and maps but throws for unsupported objects,
proving that custom search-context wrappers fail in both changed paths; the regression test does not
exercise this API condition because it covers only web-element and driver roots.

AGENTS.md: Include Focused Tests for Behavioral Changes and Avoid Misleading Mocks
java/src/org/openqa/selenium/support/locators/RelativeLocator.java[238-244]
java/src/org/openqa/selenium/support/locators/RelativeLocatorServerSide.java[66-72]
java/src/org/openqa/selenium/JavascriptExecutor.java[57-66]
java/test/org/openqa/selenium/support/locators/RelativeLocatorTest.java[426-441]
java/src/org/openqa/selenium/By.java[138-158]
java/src/org/openqa/selenium/support/locators/RelativeLocator.java[235-244]
java/src/org/openqa/selenium/support/locators/RelativeLocatorServerSide.java[58-72]
java/src/org/openqa/selenium/remote/WebElementToJsonConverter.java[44-85]

Agent prompt

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

## Issue description
Relative locators treat every non-driver search context as a DOM root and pass it to `executeScript`, but arbitrary custom `SearchContext` and `WrapsDriver` implementations cannot be serialized as JavaScript arguments. Such contexts are accepted for resolving the wrapped driver but then fail during argument conversion before the relative-locator atom executes.

## Fix Focus Areas
- java/src/org/openqa/selenium/support/locators/RelativeLocator.java[238-244]
- java/src/org/openqa/selenium/support/locators/RelativeLocatorServerSide.java[66-72]
- java/src/org/openqa/selenium/remote/WebElementToJsonConverter.java[44-85]
- java/test/org/openqa/selenium/support/locators/RelativeLocatorTest.java[408-442]

## Recommended Fix
Supply the context as the atom root only when it has a supported script representation, such as a serializable element, wrapped element, or shadow root. For other wrapped contexts, retain the document-root fallback or reject the context explicitly before script conversion rather than forwarding the arbitrary object. Add focused relative-locator coverage for a custom non-element implementation of both `SearchContext` and `WrapsDriver`, exercising both locator paths.

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

---

7. Element-scoped locators search the page ✓ Resolved 🐞 Bug ≡ Correctness

Description

RelativeBy.findElements executes FIND_ELEMENTS through the unwrapped driver but never supplies
context, so the atom defaults its root to document. When an element or shadow root invokes a
relative locator, candidates outside that context enter both the client-side and server-side paths.

Code

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

+    if (!(driver instanceof JavascriptExecutor)) {

Evidence

The search API requires results to remain within the current context, and
RemoteWebElement.findElements preserves that context when dispatching a locator. The
relative-locator implementation currently uses the context only to resolve an executor, while the
JavaScript atom explicitly defaults a missing root argument to the whole document; changing the
capability check activates this path for wrapped element and shadow-root contexts.

java/src/org/openqa/selenium/SearchContext.java[22-30]
java/src/org/openqa/selenium/remote/RemoteWebElement.java[203-208]
java/src/org/openqa/selenium/support/locators/RelativeLocator.java[233-240]
java/src/org/openqa/selenium/support/locators/RelativeLocatorServerSide.java[58-69]
javascript/atoms/typescript/find-elements.ts[251-266]
javascript/atoms/typescript/find-elements.ts[291-305]

Agent prompt

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

## Issue description
Accepting a wrapped JavaScript-capable driver now allows relative locators invoked from elements and shadow roots to execute, but both relative-locator implementations omit the original search context when calling the atom. The atom consequently searches from `document` rather than within the caller's context.

## Fix Focus Areas
- java/src/org/openqa/selenium/support/locators/RelativeLocator.java[233-240]
- java/src/org/openqa/selenium/support/locators/RelativeLocatorServerSide.java[58-69]
- javascript/atoms/typescript/find-elements.ts[251-266]
- java/test/org/openqa/selenium/support/locators/RelativeLocatorTest.java[193-203]

## Recommended Fix
Pass a script-compatible element or shadow-root context as the atom's root argument while retaining the document root for driver contexts. Apply the same behavior to both relative-locator execution paths, and add a scoped-search test with matching candidates inside and outside the context to prove that only the inner candidates are returned.

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

---

8. Prefixed tag searches miss elements 🐞 Bug ≡ Correctness

Description

The fallback in tagNameMany compares the requested name with Element.localName, which discards
any namespace prefix instead of matching the qualified name used by getElementsByTagName. Under a
shadow or synthetic root without getElementsByTagName, By.tagName("p:item") misses a p:item
descendant, while By.tagName("item") can incorrectly include it.

Code

javascript/atoms/typescript/find-elements.ts[R105-106]

+    return elements.filter(el =>
+      el.namespaceURI === html ? el.localName === folded : el.localName === target)

Evidence

The new fallback scans every descendant and compares only localName, whereas Selenium's
established tag-name locator delegates to getElementsByTagName(target). MDN documents that
getElementsByTagName accepts a qualified name, while localName is only the portion after the
colon, proving the fallback changes matching semantics for prefixed elements.

javascript/atoms/typescript/find-elements.ts[99-106]
javascript/atoms/locators/tag_name.js[45-53]
🌐 The method's tagName argument is the qualified name to look for.
🌐 localName returns only the local portion after the colon in a qualified name.

Agent prompt

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

## Issue description
The tag-name fallback compares against `localName`, so namespace prefixes are discarded and prefixed elements are matched differently from `getElementsByTagName`.

## Fix Focus Areas
- javascript/atoms/typescript/find-elements.ts[99-106]

## Recommended Fix
Compare non-HTML elements using their qualified name, including any prefix, while preserving the intended HTML case-folding behavior. Add fallback tests covering both a qualified lookup such as `p:item` and an unqualified `item` lookup against prefixed descendants.

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

---

Context sources

Review mode: ⚖️ Balanced: The push changes runtime locator scoping across element and shadow-root paths, creating real behavioral risk, but remains a focused single concern suitable for one careful review pass.

Tip of the day

💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[diemol]

This fix unblocks element/shadow-root-scoped relative locators, but neither call site (RelativeLocator.java nor RelativeLocatorServerSide.java) passes the context through to the FIND_ELEMENTS atom, so the atom's root always defaults to document.

That means calls like element.findElements(with(...)) will now silently search the entire page instead of just the element's subtree, whereas before they at least failed with a clear exception.

[Mochxd]

@diemol
yeah, that was a really good point, I pushed a follow-up that passes the search context into the atom when it isn't the driver (null still means document). Same change on both locator paths.

There's a new test in RelativeLocatorTest with one match inside the context and one outside. Searching from the element only gets the inner one; searching from the driver still returns both.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-code-review]

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

[diemol]

@Mochxd it seems the new added test is failing. Were you able to execute it on your end before submitting the code?

[Mochxd]

@diemol I went through the remote log before changing anything.

RelativeLocatorTest-remote is the one that fails, and only shouldOnlySearchWithinTheContextElement. The same test on Chrome macOS/Windows in that run is green.

Grid is calling executeScript with two args. The scope element is fine. The locator is not: relative is a Java toString(), not an object. A driver-level search in the same run sends { "relative": { "filters": ..., "root": ... } } and null, and that works. With a string the atom never sees filters/root.

On the client, RelativeLocator already JSON-encodes the locator (asAtomLocatorParameter) before the script. RelativeLocatorServerSide still passes the live map, which still has RemoteWebElements inside it. That is the path element.findElements(with(...)) takes on Grid.

I was going to convert that locator to a JSON-safe map first, same as the client, and keep the test. I have not pushed that yet. If you would rather keep relative locators document-scoped on the server, or do this a different way, say so and I will follow that.

[Mochxd]

@diemol I reproduced RelativeLocatorTest-remote / shouldOnlySearchWithinTheContextElement locally. Same ChromeDriver error as CI: call function result missing int 'status'.

The locator was going over the wire as Map.toString() (filters=[...]). RemoteWebElement was doing String.valueOf(value) on child find. Driver-level find already keeps the value as an object, so that path was fine.

I dropped the stringify so the nested map stays JSON. After that the scoped Grid test passes.

[Mochxd]

@diemol any updates?

[qodo-code-review]

Code review by qodo was updated up to the latest commit 279eafc

[diemol]

I still don't see a change in the shadowRoot.java because searching from a normal element will work, but if I search inside the shadowRoot, it won't work.

Also, in the findElements.ts atom, the resolveAnchor method won't work if we pass a by locator, so that also needs to be fixed to get the whole thing working.

And the last thing I saw is that it is very unlikely, but there might be someone extending the find child element and the find child elements, so we need to deprecate and, after that, remove.

[diemol]

I don't see how this comment is needed.

[diemol]

This also needs to be deprecated first. There might be someone who is using this outside the project. We need to mark this as deprecated and in two releases we remove it.

[diemol]

There might be someone who is using this outside the project. We need to mark this as deprecated and in two releases we remove it.

[Mochxd]

@diemol the review changes are in. Shadow root find passes the locator value through instead of turning it into a string. resolveAnchor looks up a By inside the root it was given. The String find child element and find child elements methods are deprecated and call the Object versions, and I did the same for the shadow root find methods because those signatures changed too.

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0a1b85c

[qodo-code-review]

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

[Mochxd]

@diemol about the qodo comment on the tag fallback: I am leaving that line as it is. The difference it describes is real for an element created with a namespace prefix, searched from a shadow root. For a normal HTML tag there is no prefix, so localName is already what getElementsByTagName compares, and the lowercase check stays. I do not think that edge belongs in this PR.

[diemol]

I pushed two commits on top of your changes:

1. [java] Route shadow root relative finds through Grid and fall back to the document for anchors

• Grid: CustomLocatorHandler now also handles /session/{sessionId}/shadow/{shadowId}/element(s). Before, shadowRoot.findElements(with(...)) failed on Grid with invalid argument: invalid locator once
  an earlier driver or element relative search had cached the remote path.
• Atom: resolveAnchor now looks for a locator anchor inside the search root first and falls back to the document. Anchors inside a shadow root work, and element-scoped searches can still use an anchor
  anywhere on the page. Driver-level calls are unchanged.
• Tests:
  • CustomLocatorHandlerTest.shouldBeAbleToRootASearchWithinAShadowRoot.
  • RelativeLocatorTest.shouldOnlySearchWithinTheShadowRoot, which also runs on Grid in the remote variant and fails without the Grid change.
  • Atom tests for an anchor outside the root and for an anchor inside a shadow root.

2. [js] Type the find-elements atom root as possibly a ShadowRoot

• Widened the atom's root type to Document | Element | ShadowRoot, which removes the cast in the tag-name fallback.
• Folded the fake-root tag-name test into the shadow-root test.

[qodo-code-review]

Code review by qodo was updated up to the latest commit 77b3d1c

[Mochxd]

@diemol the RBE job is red because of //dotnet/test/webdriver:DevTools/DevToolsTabsTests-chrome, ClosingTabDoesNotBreakDevToolsSession. It failed both attempts. The test closes the original tab and then Console.enable returns "Session with given id not found." This PR does not touch DevTools or the .NET bindings. Two Python BiDi tests failed once and passed on retry, so they are marked flaky and did not fail the job: browsingContext.locateNodes on Edge ("execution contexts cleared") and a Chrome network test that timed out waiting for a BiDi response.

I went through the two commits. Routing the shadow find endpoints through CustomLocatorHandler is the right fix for the cached remote path, and looking up the anchor in the root before the document matches the tests you added. RelativeLocatorTest, RelativeLocatorTest-remote, and CustomLocatorHandlerTest passed.

The qodo comment on shouldBeAbleToRootASearchWithinAShadowRoot is that the test only posts to /elements. The /element route uses the same shadowRoot() helper and leaves findMultiple false. I would not add a second test for that.

[diemol]

Thank you for your work on this, I believe this was long pending to be fixed.

I also pushed in this same PR a change for .NET since it was having the same issue.

[qodo-code-review]

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