Skip to content

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

Merged
diemol merged 13 commits into
SeleniumHQ:trunkfrom
Mochxd:fix-by-javascript-executor
Sep 25, 2026
Merged

diemol merged 13 commits into
SeleniumHQ:trunkfrom
Mochxd:fix-by-javascript-executor

Conversation

@Mochxd

@Mochxd Mochxd commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

🔗 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)

@selenium-ci selenium-ci added the C-java Java Bindings label Aug 12, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

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

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

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

qodo-code-review Bot commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Remediation recommended

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


Grey Divider

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.

Grey Divider

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 ↗

Grey Divider

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)


Remediation recommended
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


Grey Divider

Qodo Logo

Comment thread java/test/org/openqa/selenium/ByTest.java Outdated
@Mochxd
Mochxd force-pushed the fix-by-javascript-executor branch from 9ce385e to bdc7fc4 Compare August 12, 2026 12:40
Comment thread java/src/org/openqa/selenium/By.java
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@Mochxd

Mochxd commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

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
Mochxd force-pushed the fix-by-javascript-executor branch from bdc7fc4 to 689eff9 Compare September 11, 2026 12:14
@Mochxd

Mochxd commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

@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

qodo-code-review Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Single shadow finds lack coverage 📘 Rule violation ☼ Reliability ⭐ New
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



Remediation recommended

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


Grey Divider

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.

Grey Divider

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 ↗

Grey Divider

Previous reviews

Review updated until commit b0f9598

Results up to commit 812ad2f ⚖️ Balanced


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


Action required
1. 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


Results up to commit eaeffda 🧠 Deep


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


Action required
1. 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


2. 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


3. 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


Results up to commit 279eafc 🧠 Deep


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


Action required
1. 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


Results up to commit 0a1b85c 🧠 Deep


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


Action required
1. 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


Results up to commit d834e30 🧠 Deep


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


Remediation recommended
1. 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


Grey Divider

Qodo Logo

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

@diemol diemol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Mochxd commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor Author

@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

Copy link
Copy Markdown
Contributor

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
Mochxd requested a review from diemol September 14, 2026 19:12
Comment thread java/src/org/openqa/selenium/support/locators/RelativeLocator.java
Comment thread java/src/org/openqa/selenium/support/locators/RelativeLocator.java
Comment thread java/src/org/openqa/selenium/support/locators/RelativeLocator.java
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@diemol

diemol commented Sep 16, 2026

Copy link
Copy Markdown
Member

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

@Mochxd

Mochxd commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@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.

String.valueOf() turned relative locator maps into Map.toString() on the wire, so Grid could not run element-scoped relative locators.
@Mochxd

Mochxd commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

@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
Mochxd requested a review from diemol September 17, 2026 14:31
@Mochxd

Mochxd commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

@diemol any updates?

This was referenced Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Sponsor
SponsoredKunjungi sekarang
Promo