Merge pull request #230 from rubycdp/bugfix/issue#229-node-select-picks-option-with-empty-value-instead-provided-value

Author: Mifrill

CHANGELOG.md

@@ -13,8 +13,8 @@ a block with this page, after which the page is closed.
 - `Ferrum::Browser` supports new option `:proxy` https://github.com/rubycdp/ferrum#proxy
 - `Ferrum::Network::Response#params` attr_reader added
 - `Ferrum::Node`
-  - `#selected` check selected option
-  - `#select` select option
+  - `#selected` checks selected options. Returns an array of `Ferrum::Node` that's selected.
+  - `#select` selects options by passed attribute, default: `:value`.
   - `#computed_style` returns hash of computed styles
 - `Ferrum::JavaScriptError#stack_trace` attr_reader
 - Windows support

README.md

@@ -1098,9 +1098,23 @@ frame.at_css("//a[text() = 'Log in']") # => Node
 #### property
 #### attribute
 #### evaluate
-#### selected : `Array`
+#### selected : `Array<Node>`
 #### select
 
+(chainable) Selects options by passed attribute.
+
+```ruby
+browser.at_xpath("//*[select]").select(["1"]) # => Node (select)
+browser.at_xpath("//*[select]").select(["text"], by: :text) # => Node (select)
+```
+
+Accept string, array or strings:
+```ruby
+browser.at_xpath("//*[select]").select("1")
+browser.at_xpath("//*[select]").select("1", "2")
+browser.at_xpath("//*[select]").select(["1", "2"])
+```
+
 
 ## Thread safety ##
 

lib/ferrum/node.rb

@@ -136,33 +136,30 @@ def selected
           if (element.nodeName.toLowerCase() !== 'select') {
             throw new Error('Element is not a <select> element.');
           }
-          return Array.from(element).filter(option => option.selected).map((option) => option.text);
+          return Array.from(element).filter(option => option.selected);
         }
       JS
       page.evaluate_func(function, self)
     end
 
-    def select(*values)
+    def select(*values, by: :value)
       tap do
         function = <<~JS
-          function(element, values) {
+          function(element, values, by) {
             if (element.nodeName.toLowerCase() !== 'select') {
               throw new Error('Element is not a <select> element.');
             }
             const options = Array.from(element.options);
             element.value = undefined;
             for (const option of options) {
-              option.selected = values.includes(option.value);
+              option.selected = values.some((value) => option[by] === value);
               if (option.selected && !element.multiple) break;
             }
             element.dispatchEvent(new Event('input', { bubbles: true }));
             element.dispatchEvent(new Event('change', { bubbles: true }));
-            return options
-              .filter((option) => option.selected)
-              .map((option) => option.value);
           }
         JS
-        page.evaluate_func(function, self, values.join(","))
+        page.evaluate_func(function, self, values.flatten, by)
       end
     end
 

spec/node_spec.rb

@@ -89,24 +89,24 @@ module Ferrum
       end
 
       it "returns texts of selected options" do
-        expect(browser.at_xpath("//*[@id='form_region']").selected).to eq(["Norway"])
+        expect(browser.at_xpath("//*[@id='form_region']").selected.map(&:text)).to eq(["Norway"])
       end
 
       context "when options exists but no selected option" do
         it "returns first option text as default value" do
-          expect(browser.at_xpath("//*[@id='form_title']").selected).to eq(["Mrs"])
+          expect(browser.at_xpath("//*[@id='form_title']").selected.map(&:text)).to eq(["Mrs"])
         end
       end
 
       context "when no selected options" do
         it "returns empty array" do
-          expect(browser.at_xpath("//*[@id='form_tendency']").selected).to eq([])
+          expect(browser.at_xpath("//*[@id='form_tendency']").selected.map(&:text)).to eq([])
         end
       end
 
       context "when selector is not <select>" do
         it "raises JavaScriptError with proper message" do
-          expect { browser.at_xpath("//*[@id='customer_name']").selected }
+          expect { browser.at_xpath("//*[@id='customer_name']").selected.map(&:text) }
             .to raise_exception(Ferrum::JavaScriptError, /Element is not a <select> element/)
         end
       end
@@ -118,13 +118,13 @@ module Ferrum
       end
 
       it "picks option in select by match string argument" do
-        expect(browser.at_xpath("//*[@id='form_title']").select("Miss").selected).to eq(["Miss"])
+        expect(browser.at_xpath("//*[@id='form_title']").select("Miss").selected.map(&:text)).to eq(["Miss"])
       end
 
       shared_examples "clears selected options with no exception" do |options|
         it "clears selected options with no exception" do
-          expect(browser.at_xpath("//*[@id='form_title']").selected).to eq(["Mrs"])
-          expect(browser.at_xpath("//*[@id='form_title']").select(options).selected).to eq([])
+          expect(browser.at_xpath("//*[@id='form_title']").selected.map(&:text)).to eq(["Mrs"])
+          expect(browser.at_xpath("//*[@id='form_title']").select(options).selected.map(&:text)).to eq([])
         end
       end
 
@@ -142,18 +142,20 @@ module Ferrum
 
       context "when one of option with provided texts does not exist" do
         it "picks only existed options with no exception" do
-          expect(browser.at_xpath("//*[@id='form_title']").selected).to eq(["Mrs"])
-          expect(browser.at_xpath("//*[@id='form_title']").select(%w[Mrs SQL]).selected).to eq(["Mrs"])
+          expect(browser.at_xpath("//*[@id='form_title']").selected.map(&:text)).to eq(["Mrs"])
+          expect(browser.at_xpath("//*[@id='form_title']").select(%w[Mrs SQL]).selected.map(&:text)).to eq(["Mrs"])
         end
       end
 
       context "when select has multiple property" do
         it "picks options in select by match arguments as array" do
-          expect(browser.at_xpath("//*[@id='form_languages']").select(%w[SQL Ruby]).selected).to eq(%w[Ruby SQL])
+          expect(browser.at_xpath("//*[@id='form_languages']").select(%w[SQL Ruby]).selected.map(&:text))
+            .to eq(%w[Ruby SQL])
         end
 
-        it "picks options in select by match arguments as string" do
-          expect(browser.at_xpath("//*[@id='form_languages']").select("SQL, Ruby").selected).to eq(%w[Ruby SQL])
+        it "picks options in select by match arguments as strings" do
+          expect(browser.at_xpath("//*[@id='form_languages']").select("SQL", "Ruby").selected.map(&:text))
+            .to eq(%w[Ruby SQL])
         end
       end
 
@@ -166,7 +168,29 @@ module Ferrum
 
       context "when provided texts of disabled option" do
         it "picks disabled option with no exception" do
-          expect(browser.at_xpath("//*[@id='form_title']").select(["Other"]).selected).to eq(["Other"])
+          expect(browser.at_xpath("//*[@id='form_title']").select(["Other"]).selected.map(&:text)).to eq(["Other"])
+        end
+      end
+
+      context "when option with text and value" do
+        it "picks option in select by matched text" do
+          expect(browser.at_xpath("//select[@id='form_locale']").select("Swedish", by: :text).selected.map(&:value))
+            .to eq(["sv"])
+        end
+      end
+
+      context "when option with empty text/value" do
+        it "picks option in select by match string argument" do
+          expect(browser.at_xpath("//select[@id='empty_option']").select("AU").selected.map(&:value)).to eq(["AU"])
+        end
+
+        it "picks empty option by match empty value argument" do
+          expect(browser.at_xpath("//select[@id='empty_option']").select("").selected.map(&:value)).to eq([""])
+        end
+
+        it "picks empty option by match empty text argument" do
+          expect(browser.at_xpath("//select[@id='empty_option']").select("", by: :text).selected.map(&:text))
+            .to eq([""])
         end
       end
     end

spec/support/views/form.erb

@@ -672,3 +672,8 @@ New line after and before textarea tag
 <p>
   <input id="hidden_input" style="display: none"/>
 </p>
+
+<select id="empty_option">
+  <option value=""></option>
+  <option value="AU">Australia</option>
+</select>