[rb] fix select being able to select options hidden by css rules

[FFederi]

🔗 Related Issues

Related to issue: [🚀 Feature]: Unifying Select Class Across All Bindings

💥 What does this PR do?

Modifies select behaviour to make it the same as python and Java bindings.
In particular, makes it so selecting an option hidden by CSS rules raises an exception.

🔧 Implementation Notes

I couldn't find (or understand) a general way to check visibility of an element, so I added a simple check on CSS values like it's been done with Java and python.

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Type

Bug fix

---

Description

• Prevents selecting/deselecting options hidden by CSS rules

• Adds visibility check using CSS properties (visibility, display, opacity)

• Raises ElementNotInteractableError for invisible options

• Aligns Ruby bindings behavior with Python and Java implementations

---

File Walkthrough

	Relevant files
Bug fix	select.rb Add visibility checks to select/deselect operations            rb/lib/selenium/webdriver/support/select.rb Added css_property_and_visible? helper method to check element visibility Modified select_option to raise ElementNotInteractableError for invisible options Modified deselect_option to raise ElementNotInteractableError for invisible options Checks CSS properties (visibility, display, opacity) against hidden values +19/-0
Tests	select_spec.rb Add tests for invisible option selection behavior                rb/spec/integration/selenium/webdriver/select_spec.rb Added multi_invisible fixture for testing invisible select elements Added test case for selecting invisible options by text Added test case for deselecting invisible options by text Both test cases verify ElementNotInteractableError is raised +17/-0

---

[selenium-ci]

Thank you, @FFederi 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-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡	🎫 #15265 🟢 Make `Select` class behavior consistent across language bindings (tracking item: Ruby). Ensure selection by visible text (and "contains visible text" where applicable) does not allow selecting options hidden by CSS, considering the CSS properties visibility, display, and opacity and the values hidden, none, 0, 0.0. ⚪ Confirm that Ruby’s "contains visible text" selection path (if supported in this binding/version) also routes through the same option selection logic and therefore raises the same exception for CSS-hidden options in real browser runs.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Potentially misleading name: The method css_property_and_visible? implies a full visibility check but only tests a small set of CSS property values, which may confuse readers about its actual behavior. Referred Code def css_property_and_visible?(element) css_value_candidates = %w[hidden none 0 0.0].to_set css_property_candidates = %w[visibility display opacity] css_property_candidates.none? do |property| css_value_candidates.include?(element.css_value(property)) end end Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Incomplete visibility checks: The new CSS-based invisibility detection in css_property_and_visible? may miss edge cases (e.g., other opacity string formats, inherited styles, or other visibility-affecting CSS values), potentially allowing still-non-interactable options through or blocking valid ones. Referred Code def css_property_and_visible?(element) css_value_candidates = %w[hidden none 0 0.0].to_set css_property_candidates = %w[visibility display opacity] css_property_candidates.none? do |property| css_value_candidates.include?(element.css_value(property)) end end Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Improve visibility check for correctness Improve the css_property_and_visible? method by checking each CSS property (display, visibility, opacity) against its specific "hiding" value for a more correct and robust visibility check. rb/lib/selenium/webdriver/support/select.rb [280-287] def css_property_and_visible?(element) - css_value_candidates = %w[hidden none 0 0.0].to_set - css_property_candidates = %w[visibility display opacity] + return false if element.css_value('display') == 'none' + return false if element.css_value('visibility') == 'hidden' + return false if element.css_value('opacity').to_f.zero? - css_property_candidates.none? do |property| - css_value_candidates.include?(element.css_value(property)) - end + true end Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the visibility check logic is flawed by using a single set of values for different CSS properties, and proposes a more robust and correct implementation.	Medium
Update

[FFederi]

@aguspe
Sorry for the delay, thanks for your review!
I tried addressing the points you raised trying to prioritizing making the logic similar to Java's, what do you think?

[aguspe]

@aguspe Sorry for the delay, thanks for your review! I tried addressing the points you raised trying to prioritizing making the logic similar to Java's, what do you think?

@FFederi thank you for the awesome work! I just have one small comment in Java selectByVisibleText also calls assertSelectIsEnabled() and assertSelectIsVisible() on the parent select before iterating options, want to add the same here?

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Tickets: 🎫 [🚀 Feature]: Unifying Select Class Across All Bindings

1. Missing Set require 🐞 Bug ☼ Reliability

Description

Select initializes HIDDEN_CSS_VALUES using .to_set during file load, but the codebase does not
require Ruby’s stdlib set, so loading this file can raise NoMethodError for to_set. This is a
new hard-failure path because the constant is evaluated immediately when the file is required.

Code

rb/lib/selenium/webdriver/support/select.rb[24]

+        HIDDEN_CSS_VALUES = %w[hidden none 0 0.0].to_set.freeze

Evidence

The PR adds a constant initialized via .to_set in support/select.rb; .to_set is provided by
Ruby’s stdlib set and isn’t available unless set is loaded. The Selenium Ruby entrypoint
requires various stdlibs but does not require set, and there is no local require in
support/select.rb, so requiring Support/Select can fail at load time.

rb/lib/selenium/webdriver/support/select.rb[20-26]
rb/lib/selenium/webdriver.rb[20-30]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rb/lib/selenium/webdriver/support/select.rb` now calls `.to_set` while defining `HIDDEN_CSS_VALUES`. In Ruby, `Enumerable#to_set` is defined by the stdlib `set` and is not available unless `set` has been required, so loading this file can raise `NoMethodError` at require-time.

### Issue Context
This is particularly risky because the `.to_set` call happens during constant initialization (file load), so it can break any consumer that loads `Selenium::WebDriver::Support` / `Select` without having previously loaded `set`.

### Fix Focus Areas
- rb/lib/selenium/webdriver/support/select.rb[20-26]
- rb/lib/selenium/webdriver.rb[20-30]

### Suggested fix
Add `require 'set'` either:
1) locally at the top of `rb/lib/selenium/webdriver/support/select.rb`, or
2) centrally in `rb/lib/selenium/webdriver.rb` (preferred if you want to support other existing `.to_set` usages across the library).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. select_by(:text) now raises 📘 Rule violation ⚙ Maintainability ⭐ New

Description

select_by(:text, ...) now raises exceptions for previously-selectable cases (disabled/invisible
select or invisible option), which is a user-facing behavior change that can break existing
consumers upgrading without code changes. This may require a deprecation period and/or release-note
documentation to maintain API compatibility expectations.

Code

rb/lib/selenium/webdriver/support/select.rb[R167-179]

+          assert_select_enabled
+          assert_select_visible
+
          opts = find_by_text text

-          return select_options(opts) unless opts.empty?
+          raise Error::NoSuchElementError, "cannot locate element with text: #{text.inspect}" if opts.empty?

-          raise Error::NoSuchElementError, "cannot locate element with text: #{text.inspect}"
+          opts.each do |opt|
+            raise Error::NoSuchElementError, "invisible option with text: #{text.inspect}" unless option_visible?(opt)
+
+            select_option(opt)
+            break unless multiple?
+          end

Evidence

PR Compliance ID 5 requires avoiding breaking changes to public user-facing interfaces. The updated
select_by_text implementation adds new precondition checks and raises (NoSuchElementError /
UnsupportedOperationError) in cases that could previously succeed, changing the public behavior of
selection by visible text.

AGENTS.md
rb/lib/selenium/webdriver/support/select.rb[167-179]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Ruby `Select` behavior for `select_by(:text, ...)` is now stricter and raises exceptions for disabled/invisible selects and invisible options, which can be a breaking change for users who previously relied on selecting such options.

## Issue Context
Compliance requires maintaining user-facing API/behavior compatibility where possible, or clearly managing breaking changes (e.g., deprecation + documentation) so users can upgrade by changing only the version number.

## Fix Focus Areas
- rb/lib/selenium/webdriver/support/select.rb[167-179]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Hidden options still selectable 🐞 Bug ≡ Correctness ⭐ New

Description

The new CSS visibility guard is only applied in select_by_text, so callers can still select
CSS-hidden options via select_by_index/select_by_value (and select_all) on the same Select
instance. This creates inconsistent semantics and can either undermine the intended restriction or
surface driver-specific click/interactability errors when a hidden option is clicked.

Code

rb/lib/selenium/webdriver/support/select.rb[R167-179]

+          assert_select_enabled
+          assert_select_visible
+
          opts = find_by_text text

-          return select_options(opts) unless opts.empty?
+          raise Error::NoSuchElementError, "cannot locate element with text: #{text.inspect}" if opts.empty?

-          raise Error::NoSuchElementError, "cannot locate element with text: #{text.inspect}"
+          opts.each do |opt|
+            raise Error::NoSuchElementError, "invisible option with text: #{text.inspect}" unless option_visible?(opt)
+
+            select_option(opt)
+            break unless multiple?
+          end

Evidence

select_by_text now asserts the select is enabled/visible and rejects invisible options before
clicking, but select_by_index and select_by_value do not perform any visibility checks and still
click the found option(s). The integration spec changes in this PR also codify that hidden options
should error by text but still be selectable by index/value, demonstrating the inconsistent behavior
for the same hidden DOM options.

rb/lib/selenium/webdriver/support/select.rb[166-210]
rb/lib/selenium/webdriver/support/select.rb[146-150]
rb/spec/integration/selenium/webdriver/select_spec.rb[88-198]
common/src/web/formPage.html[78-88]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`select_by_text` now checks select/option visibility, but `select_by_index`, `select_by_value`, and `select_all` still allow selecting the same CSS-hidden options. This inconsistency means users can bypass the new restriction by choosing a different selector strategy.

### Issue Context
Integration specs added in this PR explicitly show `:text` raising for hidden options, while `:index` and `:value` still successfully select those hidden options.

### Fix Focus Areas
- rb/lib/selenium/webdriver/support/select.rb[166-210]
- rb/lib/selenium/webdriver/support/select.rb[196-210]
- rb/lib/selenium/webdriver/support/select.rb[146-150]
- rb/spec/integration/selenium/webdriver/select_spec.rb[125-198]

### Proposed change
- Decide on one consistent policy (either:
 - enforce the same `assert_select_enabled`/`assert_select_visible` + `option_visible?` checks for `select_by_index`, `select_by_value`, and `select_all`; **or**
 - if the intended behavior is "only visible-text selection enforces visibility" (matching other bindings), update the PR description/spec expectations to avoid implying a general restriction).
- If enforcing visibility broadly:
 - Call `assert_select_enabled` + `assert_select_visible` at the start of `select_by_index`, `select_by_value`, and `select_all`.
 - Before clicking, raise a consistent exception when `option_visible?` is false (matching `select_by_text`).
 - Update/replace the integration specs that currently assert invisible options can be selected by index/value.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit b0a4c10

Results up to commit 0836333

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

1. Missing Set require 🐞 Bug ☼ Reliability

Description

Select initializes HIDDEN_CSS_VALUES using .to_set during file load, but the codebase does not
require Ruby’s stdlib set, so loading this file can raise NoMethodError for to_set. This is a
new hard-failure path because the constant is evaluated immediately when the file is required.

Code

rb/lib/selenium/webdriver/support/select.rb[24]

+        HIDDEN_CSS_VALUES = %w[hidden none 0 0.0].to_set.freeze

Evidence

The PR adds a constant initialized via .to_set in support/select.rb; .to_set is provided by
Ruby’s stdlib set and isn’t available unless set is loaded. The Selenium Ruby entrypoint
requires various stdlibs but does not require set, and there is no local require in
support/select.rb, so requiring Support/Select can fail at load time.

rb/lib/selenium/webdriver/support/select.rb[20-26]
rb/lib/selenium/webdriver.rb[20-30]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rb/lib/selenium/webdriver/support/select.rb` now calls `.to_set` while defining `HIDDEN_CSS_VALUES`. In Ruby, `Enumerable#to_set` is defined by the stdlib `set` and is not available unless `set` has been required, so loading this file can raise `NoMethodError` at require-time.

### Issue Context
This is particularly risky because the `.to_set` call happens during constant initialization (file load), so it can break any consumer that loads `Selenium::WebDriver::Support` / `Select` without having previously loaded `set`.

### Fix Focus Areas
- rb/lib/selenium/webdriver/support/select.rb[20-26]
- rb/lib/selenium/webdriver.rb[20-30]

### Suggested fix
Add `require 'set'` either:
1) locally at the top of `rb/lib/selenium/webdriver/support/select.rb`, or
2) centrally in `rb/lib/selenium/webdriver.rb` (preferred if you want to support other existing `.to_set` usages across the library).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Persistent review updated to latest commit b0a4c10

[FFederi]

@aguspe

I just have one small comment in Java selectByVisibleText also calls assertSelectIsEnabled() and assertSelectIsVisible() on the parent select before iterating options, want to add the same here?

Sorry for the back and forth, I addressed this on b0a4c10 !