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

rb/lib/selenium/webdriver/support/select.rb

@@ -21,6 +21,9 @@ module Selenium
   module WebDriver
     module Support
       class Select
+        HIDDEN_CSS_VALUES = %w[hidden none 0 0.0].to_set.freeze
+        VISIBILITY_PROPERTIES = %w[visibility display opacity].freeze
+
         #
         # @param [Element] element The select element to use
         #
@@ -161,11 +164,33 @@ def deselect_all
         private
 
         def select_by_text(text)
+          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
+        end
+
+        def assert_select_enabled
+          return if @element.enabled?
+
+          raise Error::UnsupportedOperationError,
+                'You may not select an option in a disabled select'
+        end
+
+        def assert_select_visible
+          return if option_visible?(@element)
+
+          raise Error::UnsupportedOperationError,
+                'You may not select an option in an invisible select'
         end
 
         def select_by_index(index)
@@ -266,6 +291,10 @@ def find_by_index(index)
         def find_by_value(value)
           @element.find_elements(xpath: ".//option[@value = #{Escaper.escape value}]")
         end
+
+        def option_visible?(element)
+          VISIBILITY_PROPERTIES.none? { |property| HIDDEN_CSS_VALUES.include?(element.css_value(property)) }
+        end
       end # Select
     end # Support
   end # WebDriver

rb/spec/integration/selenium/webdriver/select_spec.rb

@@ -27,6 +27,7 @@ module Support
         let(:multi_select) { described_class.new(driver.find_element(id: 'multi')) }
         let(:single_disabled) { described_class.new(driver.find_element(name: 'single_disabled')) }
         let(:multi_disabled) { described_class.new(driver.find_element(name: 'multi_disabled')) }
+        let(:multi_invisible) { described_class.new(driver.find_element(id: 'invisible_multi_select')) }
 
         before { driver.navigate.to url_for('formPage.html') }
         after { reset_driver! if GlobalTestEnv.rbe? && GlobalTestEnv.browser == :chrome }
@@ -120,6 +121,14 @@ module Support
               it 'errors when not found' do
                 expect { multi_select.select_by(:text, 'invalid') }.to raise_exception(Error::NoSuchElementError)
               end
+
+              it 'errors when option is invisible', :aggregate_failures do
+                %w[Apples Pears Oranges Lemons].each do |text|
+                  expect {
+                    multi_invisible.select_by(:text, text)
+                  }.to raise_exception(Error::NoSuchElementError)
+                end
+              end
             end
 
             context 'when by index' do
@@ -147,6 +156,11 @@ module Support
               it 'errors when not found' do
                 expect { multi_select.select_by(:index, 5) }.to raise_exception(Error::NoSuchElementError)
               end
+
+              it 'selects invisible option' do
+                multi_invisible.select_by(:index, 1)
+                expect(multi_invisible.selected_options).to include(driver.find_element(css: 'option[value=pears]'))
+              end
             end
 
             context 'when by value' do
@@ -176,6 +190,11 @@ module Support
               it 'errors when not found' do
                 expect { multi_select.select_by(:value, 'invalid') }.to raise_exception(Error::NoSuchElementError)
               end
+
+              it 'selects invisible option' do
+                multi_invisible.select_by(:value, 'pears')
+                expect(multi_invisible.selected_options).to include(driver.find_element(css: 'option[value=pears]'))
+              end
             end
           end
 
@@ -298,6 +317,11 @@ module Support
             it 'errors when not found' do
               expect { multi_select.deselect_by(:text, 'invalid') }.to raise_exception(Error::NoSuchElementError)
             end
+
+            it 'deselects invisible option' do
+              multi_invisible.deselect_by(:text, 'Apples')
+              expect(multi_invisible.selected_options).not_to include(driver.find_element(css: 'option[value=apples]'))
+            end
           end
 
           context 'when by index' do
@@ -320,6 +344,11 @@ module Support
             it 'errors when not found' do
               expect { multi_select.deselect_by(:index, 5) }.to raise_exception(Error::NoSuchElementError)
             end
+
+            it 'deselects invisible option' do
+              multi_invisible.deselect_by(:index, 0)
+              expect(multi_invisible.selected_options).not_to include(driver.find_element(css: 'option[value=apples]'))
+            end
           end
 
           context 'when by value' do
@@ -342,6 +371,11 @@ module Support
             it 'errors when not found' do
               expect { multi_select.deselect_by(:value, 'invalid') }.to raise_exception(Error::NoSuchElementError)
             end
+
+            it 'deselects invisible option' do
+              multi_invisible.deselect_by(:value, 'apples')
+              expect(multi_invisible.selected_options).not_to include(driver.find_element(css: 'option[value=apples]'))
+            end
           end
         end
 

rb/spec/unit/selenium/webdriver/support/select_spec.rb

@@ -24,14 +24,16 @@ module WebDriver
     module Support
       describe Select do
         let(:select) do
-          select_element = instance_double(Element, tag_name: 'select')
+          select_element = instance_double(Element, tag_name: 'select', enabled?: true)
           allow(select_element).to receive(:dom_attribute).with(:multiple)
+          allow(select_element).to receive(:css_value).and_return('')
           select_element
         end
 
         let(:multi_select) do
-          select_element = instance_double(Element, tag_name: 'select')
+          select_element = instance_double(Element, tag_name: 'select', enabled?: true)
           allow(select_element).to receive(:dom_attribute).with(:multiple).and_return 'multiple'
+          allow(select_element).to receive(:css_value).and_return('')
           select_element
         end
 
@@ -94,6 +96,7 @@ module Support
 
         it 'allows options to be selected by visible text' do
           option = instance_double(Element, selected?: false, enabled?: true, click: nil)
+          allow(option).to receive(:css_value).and_return('')
           allow(multi_select).to receive(:find_elements).and_return([option])
 
           described_class.new(multi_select).select_by(:text, 'fish')
@@ -185,6 +188,7 @@ module Support
 
         it 'falls back to slow lookups when "get by visible text fails" and there is a space' do
           first_option = instance_double(Element, selected?: false, enabled?: true, text: 'foo bar', click: nil)
+          allow(first_option).to receive(:css_value).and_return('')
           allow(select).to receive(:find_elements).and_return([], [first_option])
 
           described_class.new(select).select_by(:text, 'foo bar')