Skip to content

[java] Fix: Convert By.className to css selector for Firefox inside Shadow DOM #16085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions java/src/org/openqa/selenium/remote/ShadowRoot.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.openqa.selenium.WebElement;
import org.openqa.selenium.WrapsDriver;
import org.openqa.selenium.internal.Require;
import org.openqa.selenium.remote.shadow.FirefoxClassNameWorkaround;

// Note: we want people to code against the SearchContext API, so we keep this class package private
class ShadowRoot implements SearchContext, WrapsDriver {
Expand All @@ -44,18 +45,20 @@ class ShadowRoot implements SearchContext, WrapsDriver {

@Override
public List<WebElement> findElements(By by) {
By resolved = FirefoxClassNameWorkaround.resolveForShadow(by, this);
return parent.findElements(
this,
(using, value) -> FIND_ELEMENTS_FROM_SHADOW_ROOT(id, using, String.valueOf(value)),
by);
resolved);
}

@Override
public WebElement findElement(By by) {
By resolved = FirefoxClassNameWorkaround.resolveForShadow(by, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would highly recommend not adding browser-specific logic here.

return parent.findElement(
this,
(using, value) -> FIND_ELEMENT_FROM_SHADOW_ROOT(id, using, String.valueOf(value)),
by);
resolved);
}

@Override
Expand All @@ -73,12 +76,8 @@ private Map<String, Object> toJson() {

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
if (this == o) return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid formatting changes as part of a PR. If required, create a separate PR for it.

Copy link
Author

@nityanandbb nityanandbb Jul 25, 2025

Choose a reason for hiding this comment

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

Thank you for the review @pujagani 🙏

  • I understand the concern about avoiding browser-specific logic in core classes like ShadowRoot. I'm working on isolating the logic into a utility class (e.g., SelectorResolver) to keep ShadowRoot clean and compliant with the architecture. I'll update the PR accordingly.

  • I'll also remove any unrelated formatting changes from this PR and keep it focused on the bug fix only.

  • Regarding tests: I’ll add test coverage specifically for Firefox + By.className inside Shadow DOM to verify the selector resolution behavior.

  • I'll update the linked issue to correctly point to this PR and add context for the changes.

  • For the CLA — I'll ensure it's signed shortly so the workflow can proceed.

Thanks again for the guidance. Will push the updates soon.

if (o == null || getClass() != o.getClass()) return false;
ShadowRoot that = (ShadowRoot) o;
return Objects.equals(parent, that.parent) && Objects.equals(id, that.id);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.openqa.selenium.remote.shadow;

import org.openqa.selenium.By;
import org.openqa.selenium.InvalidSelectorException;
import org.openqa.selenium.SearchContext;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WrapsDriver;
import org.openqa.selenium.remote.HasCapabilities;
import org.openqa.selenium.Capabilities;

final class FirefoxClassNameWorkaround {

static boolean shouldUseCssSelector(By by, SearchContext context) {
if (!(by instanceof By.ByClassName)) return false;
if (!(context instanceof WrapsDriver)) return false;

try {
WebDriver driver = ((WrapsDriver) context).getWrappedDriver();
Capabilities caps = ((HasCapabilities) driver).getCapabilities();
return "firefox".equalsIgnoreCase(caps.getBrowserName());
} catch (Exception ignored) {
return false;
}
}

static By convertToCss(By by) {
String className = extractClassName(by);
if (className.contains(" ")) {
throw new InvalidSelectorException(
"Compound class names not supported in Firefox Shadow DOM. Use single class name or By.cssSelector instead."
);
}
return By.cssSelector("." + className);
}

private static String extractClassName(By by) {
String raw = by.toString().replace("By.className:", "").trim();
return raw.replaceAll("\\s+", "");
}

public static By resolveForShadow(By by, SearchContext context) {
return shouldUseCssSelector(by, context) ? convertToCss(by) : by;
}
}
Loading