fixed 15265-Unifying Select Class Across All Bindings

[rrv40]

User description

🔗 Related Issues

Fixes #15265
This aligns Select.selectByVisibleText() behavior in Java with the expectation that it only interacts with visible options, consistent with other Selenium bindings.

💥 What does this PR do?

Updates the Java Select class selectByVisibleText() method to respect element visibility before selection.
Previously, the method could select <option> elements even if they were hidden (display:none, visibility:hidden, or opacity:0).
Now, it ensures only visible options are considered, throwing a NoSuchElementException for invisible matches.

Key changes:

• Added assertSelectIsVisible() at the start of the method.
• Introduced a visibility check using hasCssPropertyAndVisible() before selecting an option.
• Updated candidate search logic to work with the new visibility constraint.

🔧 Implementation Notes

• The new hasCssPropertyAndVisible() method verifies:
  • CSS display is not none
  • CSS visibility is not hidden
  • CSS opacity is not 0 or 0.0
• The check is applied both to exact-text matches and contains-text matches.
• The logic remains backwards compatible — only invisible options are now excluded.

💡 Additional Considerations

• Tests should be updated or added to verify:
  • Selecting a visible option works as before.
  • Attempting to select a hidden option throws NoSuchElementException.
• This change could be mirrored in other Select methods (e.g., selectByIndex, selectByValue) for complete consistency.
• Other language bindings (Python, Ruby, etc.) may already enforce similar checks.

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Enhancement

---

Description

• Enhanced Java Select class to respect element visibility

• Added visibility checks for selectByVisibleText method

• Throws NoSuchElementException for invisible options

• Aligns Java behavior with other Selenium bindings

---

Diagram Walkthrough

flowchart LR
  A["selectByVisibleText called"] --> B["assertSelectIsVisible added"]
  B --> C["Find option elements"]
  C --> D["Check visibility with hasCssPropertyAndVisible"]
  D --> E["Throw exception if invisible"]
  D --> F["Select if visible"]

Loading

File Walkthrough

	Relevant files
Enhancement	Select.java Enhanced selectByVisibleText with visibility checks            java/src/org/openqa/selenium/support/ui/Select.java Added assertSelectIsVisible() call at method start Introduced visibility check using hasCssPropertyAndVisible() Updated candidate search logic to use searchText variable Enhanced error handling for invisible options +9/-6

---

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[selenium-ci]

Thank you, @Roushan7970835758 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.

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 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 Behavior Change Throwing NoSuchElementException when encountering an invisible option (even if there are visible matches later) may be too strict; consider skipping invisible options instead of failing immediately to preserve selection when multiple options exist. if (!hasCssPropertyAndVisible(option)) throw new NoSuchElementException("Invisible option with text: " + text); setSelected(option, true); Inconsistent Matching Logic The previous logic only entered the contains-search path when text had spaces; the new logic always does when no exact match is found, which may alter behavior/perf for single-word queries. Confirm this intended change and its impact. if (!matched) { String searchText = text.contains(" ") ? getLongestSubstringWithoutSpace(text) : text; List<WebElement> candidates; if (searchText.isEmpty()) { // hmm, text is either empty or contains only spaces - get all options ... candidates = element.findElements(By.tagName("option")); } else { // get candidates via XPATH ... candidates = element.findElements( By.xpath(".//option[contains(., " + Quotes.escape(searchText) + ")]")); }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Skip invisible options, throw after Avoid throwing for invisible options mid-iteration; this prevents selecting other valid visible options and changes behavior. Instead, skip invisible options and only throw if no visible match was found after the loop. java/src/org/openqa/selenium/support/ui/Select.java [132-139] +boolean anyVisibleSelected = false; for (WebElement option : options) { - if (!hasCssPropertyAndVisible(option)) - throw new NoSuchElementException("Invisible option with text: " + text); + if (!hasCssPropertyAndVisible(option)) { + continue; + } setSelected(option, true); + anyVisibleSelected = true; if (!isMultiple()) { return; } } +if (!anyVisibleSelected && !options.isEmpty()) { + throw new NoSuchElementException("Only invisible options matched text: " + text); +} Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a logic flaw where the code throws an exception on the first invisible match, preventing it from finding other potentially visible matches with the same text, which is a significant bug.	High
General	Use normalized contains() matching Normalize whitespace consistently when building the XPath contains() fallback. Without trimming both the option text and the input, options with leading/trailing spaces may be missed or mismatched. Use normalize-space() in XPath and trimmed search text. java/src/org/openqa/selenium/support/ui/Select.java [142-154] if (!matched) { String searchText = text.contains(" ") ? getLongestSubstringWithoutSpace(text) : text; - List<WebElement> candidates; + List<WebElement> candidates; if (searchText.isEmpty()) { - // hmm, text is either empty or contains only spaces - get all options ... candidates = element.findElements(By.tagName("option")); } else { - // get candidates via XPATH ... + String normalizedSearch = searchText.trim(); candidates = element.findElements( - By.xpath(".//option[contains(., " + Quotes.escape(searchText) + ")]")); + By.xpath(".//option[contains(normalize-space(.), " + Quotes.escape(normalizedSearch) + ")]")); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out an inconsistency; the fallback search logic does not normalize whitespace, unlike the primary search, which could lead to failed selections for options with extra spacing.	Medium
	Trim input before exact match Ensure consistent normalization by trimming text before using it in the equality XPath with normalize-space(.). Otherwise, trailing or leading spaces in input prevent equality match although UI shows normalized text. java/src/org/openqa/selenium/support/ui/Select.java [123-130] @Override public void selectByVisibleText(String text) { assertSelectIsEnabled(); assertSelectIsVisible(); + String normalizedText = text == null ? "" : text.trim(); // try to find the option via XPATH ... List<WebElement> options = element.findElements( - By.xpath(".//option[normalize-space(.) = " + Quotes.escape(text) + "]")); + By.xpath(".//option[normalize-space(.) = " + Quotes.escape(normalizedText) + "]")); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: This is a good suggestion for robustness, as trimming the input text before the XPath query makes the matching more resilient to leading/trailing whitespace, aligning with user expectations.	Medium
Learned best practice	Add null check for text Validate that text is non-null and non-empty before building XPath and using it in matching logic to avoid NullPointerException and unintended behavior. Return early with a clear error message if invalid input is provided. java/src/org/openqa/selenium/support/ui/Select.java [124-130] public void selectByVisibleText(String text) { + if (text == null) { + throw new IllegalArgumentException("Visible text to select must not 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 before use to prevent NullPointerException.	Low
More

[diemol]

There is another PR with similar changes but tests are failing due to the changes. The PR also needs to fix tests that will eventually fail due to this changes.