[java] Fix By.name() double String.format on names containing '%'

[titusfortner]

🔗 Related Issues

Fixes #17807

💥 What does this PR do?

By.name(name) pre-substituted the name into the CSS pattern with String.format, then
PreW3CLocator ran String.format over the result a second time. A % in the name (valid in
HTML name attributes, e.g. name="50%off") was then read as a format conversion on the second
pass — throwing IllegalFormatConversionException / UnknownFormatConversionException at
construction, or silently corrupting the fallback selector for inputs like foo%sbar
(*[name='foofoo\%sbarbar']). The name is now concatenated in and any % is doubled so the
second format pass treats it as a literal.

🔧 Implementation Notes

Kept the fix minimal and behavior-preserving rather than routing the name through cssEscape.
cssEscape is identifier escaping (for .class / #id) and would over-escape an attribute-value
string — e.g. By.name("5foo") would become *[name='\35 foo']. Doubling % only changes
behavior for names containing %, which previously threw or corrupted, so no valid existing
selector changes.

This is Java-specific: it stems from Java's String.format. Other bindings build the same
selector without a printf pass and are unaffected. Follow-up (separate issue): the W3C remote
codec's name handler in W3CHttpCommandCodec does no escaping of the name at all (not even
').

🤖 AI assistance

• AI assisted
  • Tool(s): Claude Code
  • What was generated: reproduced the defect against trunk, wrote fail-before/pass-after
    regression tests, authored the fix, verified via Bazel and google-java-format.
  • I reviewed all AI output and can explain the change

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Fix By.name() CSS fallback formatting for names containing '%'

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Prevent double String.format parsing when building By.name() fallback CSS selectors.
• Treat '%' in name attribute values as a literal to avoid format exceptions/corruption.
• Add regression tests for percent and printf-like substrings in names.

Diagram

graph TD
  A["By.name(value)"] --> B["ByName ctor"] --> C["Escape ' and %"] --> D["PreW3CLocator"] --> E["String.format (2nd pass)"] --> F["ByCssSelector fallback"] --> G["Json serialization"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Refactor PreW3CLocator to avoid String.format for literal selectors

• ➕ Eliminates the need to double '%' in callers
• ➕ Makes escaping rules explicit per locator (attribute value vs identifier)
• ➖ Broader behavioral surface area; risks changing existing fallback selector semantics
• ➖ Requires touching multiple locators and potentially adding new constructor/API variants

2. Make ByName a BaseW3CLocator with explicit CSS fallback logic

• ➕ Avoids PreW3CLocator's formatting/escaping path entirely for name selectors
• ➕ Keeps attribute-value escaping rules localized to ByName
• ➖ Potentially duplicates behavior and diverges from other pre-W3C locators
• ➖ May affect remote parameter behavior if not carefully mirrored

Recommendation: Keep the PR’s minimal, behavior-preserving fix: doubling '%' only changes previously broken inputs and avoids broader refactors. Consider a follow-up refactor of PreW3CLocator formatting/escaping if more locators hit similar issues.

Files changed (2) +25 / -1

Bug fix (1) +1 / -1

By.javaEscape '%' in ByName fallback selector to survive PreW3CLocator formatting +1/-1

Escape '%' in ByName fallback selector to survive PreW3CLocator formatting

• Stops pre-formatting the name selector with String.format and instead concatenates the selector string directly. Doubles '%' characters in the constructed selector so the subsequent String.format inside PreW3CLocator treats them as literals, avoiding exceptions and selector corruption.

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

Tests (1) +24 / -0

ByTest.javaAdd regression tests for '%' and '%s' in By.name() serialization +24/-0

Add regression tests for '%' and '%s' in By.name() serialization

• Adds unit tests asserting that By.name("50%off") and By.name("foo%sbar") serialize to a css selector without throwing and preserve the literal percent content. These tests cover both the exception case and the silent double-format corruption scenario.

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

[qodo-code-review]

Code Review by Qodo

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

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Previous review results

Review updated until commit 2b716ca ⚖️ Balanced

Results up to commit 671f528 ⚖️ Balanced

No changes from previous review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 2b716ca