[java] Refactored selectByContainsVisibleText and selectByVisibleText methods to remove code duplication

[vicky-iv]

User description

🔗 Related Issues

[🚀 Feature]: Unifying Select Class Across All Bindings #15265

💥 What does this PR do?

This is a tiny addition to my previous PR #16220
Refactored selectByContainsVisibleText and selectByVisibleText methods to remove code duplication

🔧 Implementation Notes

This change was inspired by the current .NET implementation, where there is only one method for both cases, using a boolean switch to toggle between partial and exact matches.
I preserved the existing Java interface (and existing implementation of both methods) by extracting the common selection logic into a new private method selectByVisibleText(String text, boolean isPartialMatch), and reused it across the existing selectByVisibleText and selectByContainsVisibleText methods.

All tests from the following packages are passed locally:
//java/test/org/openqa/selenium/support/ui/...

💡 Additional Considerations

--

🔄 Types of changes

• Cleanup (refactoring)

---

PR Type

Other

---

Description

• Refactored selectByVisibleText and selectByContainsVisibleText methods

• Extracted common logic into private method with boolean parameter

• Eliminated code duplication while preserving existing interface

• Improved maintainability by consolidating selection logic

---

Diagram Walkthrough

flowchart LR
  A["selectByVisibleText(text)"] --> C["selectByVisibleText(text, false)"]
  B["selectByContainsVisibleText(text)"] --> C
  C --> D["Common selection logic"]

Loading

File Walkthrough

	Relevant files
Enhancement	Select.java Refactor text selection methods to eliminate duplication  java/src/org/openqa/selenium/support/ui/Select.java Replaced duplicate code in selectByVisibleText and selectByContainsVisibleText methods Added private overloaded method with isPartialMatch boolean parameter Consolidated common selection logic into single implementation Maintained existing public API interface +69/-96

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Behavioral Change The new shared logic uses substring extraction for spaced text and falls back to contains/equals on candidates, which might slightly alter edge-case behavior between exact vs partial matching compared to the previous separate paths. Validate that both methods still match legacy behavior, especially for texts with leading/trailing spaces, multiple spaces, and empty strings. private void selectByVisibleText(String text, boolean isPartialMatch) { assertSelectIsEnabled(); assertSelectIsVisible(); // try to find the option via XPATH ... List<WebElement> options = element.findElements( By.xpath(".//option[normalize-space(.) = " + Quotes.escape(text) + "]")); for (WebElement option : options) { if (!hasCssPropertyAndVisible(option)) throw new NoSuchElementException("Invisible option with text: " + text); setSelected(option, true); if (!isMultiple()) { return; } } boolean matched = !options.isEmpty(); if (!matched) { String searchText = text.contains(" ") ? getLongestSubstringWithoutSpace(text) : text; List<WebElement> candidates; if (searchText.isEmpty()) { candidates = element.findElements(By.tagName("option")); } else { candidates = element.findElements( By.xpath(".//option[contains(., " + Quotes.escape(searchText) + ")]")); } String trimmed = text.trim(); for (WebElement option : candidates) { boolean isMatchedOptionFound = isPartialMatch ? option.getText().contains(trimmed) : option.getText().trim().equals(trimmed); if (isMatchedOptionFound) { if (!hasCssPropertyAndVisible(option)) throw new NoSuchElementException("Invisible option with text: " + text); setSelected(option, true); if (!isMultiple()) { return; } matched = true; } } } if (!matched) { throw new NoSuchElementException("Cannot locate option with text: " + text); } } Visibility Exception The code throws NoSuchElementException when encountering an invisible option that otherwise matches. Confirm this mirrors prior behavior and is the desired contract; otherwise consider a more specific exception or skipping invisible matches until none remain. for (WebElement option : options) { if (!hasCssPropertyAndVisible(option)) throw new NoSuchElementException("Invisible option with text: " + text); setSelected(option, true); if (!isMultiple()) { return; } } boolean matched = !options.isEmpty(); if (!matched) { String searchText = text.contains(" ") ? getLongestSubstringWithoutSpace(text) : text; List<WebElement> candidates; if (searchText.isEmpty()) { candidates = element.findElements(By.tagName("option")); } else { candidates = element.findElements( By.xpath(".//option[contains(., " + Quotes.escape(searchText) + ")]")); } String trimmed = text.trim(); for (WebElement option : candidates) { boolean isMatchedOptionFound = isPartialMatch ? option.getText().contains(trimmed) : option.getText().trim().equals(trimmed); if (isMatchedOptionFound) { if (!hasCssPropertyAndVisible(option)) throw new NoSuchElementException("Invisible option with text: " + text); setSelected(option, true);

[selenium-ci]

Thank you, @vicky-iv for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Learned best practice	Add null parameter validation The method should validate that the text parameter is not null before using it in XPath queries and string operations. This prevents potential NullPointerException when the method is called with null arguments. java/src/org/openqa/selenium/support/ui/Select.java [342-349] private void selectByVisibleText(String text, boolean isPartialMatch) { + if (text == null) { + throw new IllegalArgumentException("Text cannot be null"); + } assertSelectIsEnabled(); assertSelectIsVisible(); // try to find the option via XPATH ... List<WebElement> options = element.findElements( By.xpath(".//option[normalize-space(.) = " + Quotes.escape(text) + "]")); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add null checks and validation for parameters and variables before using them to prevent NullReferenceException, KeyError, or similar runtime errors.	Low
General	Fix inconsistent text matching logic The exact match logic is inconsistent with the initial XPath query which uses normalize-space(.). Use option.getText().trim().equals(trimmed) for exact match to maintain consistency with the XPath normalization behavior. java/src/org/openqa/selenium/support/ui/Select.java [377-380] boolean isMatchedOptionFound = isPartialMatch ? option.getText().contains(trimmed) - : option.getText().trim().equals(trimmed); + : trimmed.equals(option.getText().trim()); Apply / Chat Suggestion importance[1-10]: 2 __ Why: The suggestion proposes a stylistic change to a "Yoda condition" (trimmed.equals(...) instead of option.getText().trim().equals(...)) for null safety, which is a minor improvement as option.getText() is not expected to be null.	Low
Update

[cgoldberg]

That's weird.. CI failed with an HTTP 403 trying to download a package from npm. I just kicked it off again.

[vicky-iv]

@cgoldberg Looks like only the Ruby tests are failing now, and they're not related to the changes in this PR. Is there anything else I need to do to get this merged?

[cgoldberg]

@vicky-iv yea, CI looks good. I'll ping the Slack channel to see if someone more familiar with the Java bindings can give it a final review. I usually only merge Python stuff :)

[asolntsev]

Generally OK, but have a suggestion. :)

[diemol]

Thank you!