[Java] Add visibility check to selectByVisibleText in Select class

[SantiagoCecena]

User description

🔗 Related Issues

Fixes #15265

💥 What does this PR do?

This PR updates the selectByVisibleText method in the Java Select class to prevent selecting <option> elements that are hidden via CSS.

Specifically, the method now checks for CSS properties:

• display: none
• visibility: hidden
• opacity: 0 or 0.0

An existing private method, hasCssPropertyAndVisible, is used to verify visibility.

🔧 Implementation Notes

The fix was done by calling assertSelectIsVisible() at the beginning of the method, following the same pattern already used in selectByContainsVisibleText.

No new logic or dependencies were introduced, and behavior is now consistent across both selection methods.

💡 Additional Considerations

The change was manually tested in a local environment using WebDriver, since I currently cannot run Bazel locally due to company machine restrictions.

If needed, I'm happy to follow up with a Bazel-compatible test or collaborate with a maintainer to integrate one.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Add visibility check to selectByVisibleText method in Java Select class
• Prevent selection of hidden options with CSS properties like display: none
• Make behavior consistent with selectByContainsVisibleText method

---

Changes walkthrough 📝

	Relevant files
Bug fix	Select.java Add visibility validation to selectByVisibleText method    java/src/org/openqa/selenium/support/ui/Select.java • Added assertSelectIsVisible() call at method start • Added visibility check for each option using hasCssPropertyAndVisible() • Throw NoSuchElementException for invisible options with descriptive message +4/-1

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[CLAassistant]

All committers have signed the CLA.

[selenium-ci]

Thank you, @SantiagoCecena 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 🔍

(Review updated until commit b4475b0)

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 Logic Issue The visibility check throws an exception immediately when the first invisible option is found, but doesn't continue checking other options with the same text that might be visible. This could prevent selection of valid visible options when multiple options have the same text. if(!hasCssPropertyAndVisible(option)) { throw new NoSuchElementException("Invisible option with text: " + text); } Inconsistent Behavior The method throws NoSuchElementException for invisible options, but this exception type is typically used when elements are not found, not when they exist but are invisible. Consider using a more specific exception type or adjusting the behavior to skip invisible options instead. throw new NoSuchElementException("Invisible option with text: " + text); }

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to b4475b0
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Continue loop for visible options The current logic throws an exception immediately when encountering the first invisible option, preventing selection of visible options with the same text. Consider continuing the loop to find visible options instead. java/src/org/openqa/selenium/support/ui/Select.java [132-139] for (WebElement option : options) { - if (!hasCssPropertyAndVisible(option)) { - throw new NoSuchElementException("Invisible option with text: " + text); - } - setSelected(option, true); - if (!isMultiple()) { - return; + if (hasCssPropertyAndVisible(option)) { + setSelected(option, true); + if (!isMultiple()) { + return; + } } } +throw new NoSuchElementException("No visible option with text: " + text); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a significant logical flaw in the PR. The current implementation would fail if the first matching option is invisible, even if other visible options with the same text exist. The proposed change fixes this bug by iterating through all options and only selecting the visible ones.	High
General	✅ Fix spacing after if keyword Suggestion Impact: The suggestion was directly implemented - a space was added after the if keyword in line 5, changing "if(!hasCssPropertyAndVisible(option))" to "if (!hasCssPropertyAndVisible(option))" code diff: - if(!hasCssPropertyAndVisible(option)) { + if (!hasCssPropertyAndVisible(option)) { Add a space after the if keyword to follow Java coding conventions. This improves code readability and maintains consistency with standard formatting practices. java/src/org/openqa/selenium/support/ui/Select.java [133-135] -if(!hasCssPropertyAndVisible(option)) { +if (!hasCssPropertyAndVisible(option)) { throw new NoSuchElementException("Invisible option with text: " + text); } [Suggestion processed] Suggestion importance[1-10]: 3 __ Why: The suggestion correctly points out a minor style violation. Adding a space after the if keyword aligns with standard Java coding conventions, which improves code readability and consistency.	Low
Update

---

Previous suggestions

Suggestions up to commit b4475b0

Category	Suggestion	Impact
Possible issue	Check all options before throwing exception The current logic throws an exception for the first invisible option found, but doesn't check if there are other visible options with the same text. This could prevent selecting valid visible options when invisible ones exist with the same text. java/src/org/openqa/selenium/support/ui/Select.java [132-140] +boolean foundVisibleOption = false; for (WebElement option : options) { - if(!hasCssPropertyAndVisible(option)) { - throw new NoSuchElementException("Invisible option with text: " + text); - } - setSelected(option, true); - if (!isMultiple()) { - return; + if(hasCssPropertyAndVisible(option)) { + setSelected(option, true); + foundVisibleOption = true; + if (!isMultiple()) { + return; + } } } +if (!foundVisibleOption) { + throw new NoSuchElementException("No visible option with text: " + text); +} Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a logic flaw in the PR. The current implementation throws an exception on the first invisible option it encounters, which could prevent the selection of a valid, visible option that appears later in the list. The proposed change fixes this bug by ensuring all options are checked for visibility before an exception is thrown.	High

[cgoldberg]

@SantiagoCecena please fix the formatting and update your branch (missing space after if)

[cgoldberg]

Also, please sign the CLA: #15912 (comment)