From 72ec06b0c49cb68bf2003772110e9c72d301e5bc Mon Sep 17 00:00:00 2001 From: Thomas Walpole Date: Tue, 30 Apr 2019 09:49:01 -0700 Subject: [PATCH] Optimize wirecalls in selenium driver --- lib/capybara/node/element.rb | 4 +-- lib/capybara/selenium/node.rb | 32 ++++++++++++--------- lib/capybara/selenium/nodes/chrome_node.rb | 8 ++++++ lib/capybara/selenium/nodes/firefox_node.rb | 8 ++++++ lib/capybara/selenium/nodes/safari_node.rb | 8 ++++-- lib/capybara/spec/session/select_spec.rb | 5 ---- 6 files changed, 43 insertions(+), 22 deletions(-) diff --git a/lib/capybara/node/element.rb b/lib/capybara/node/element.rb index bee19ea2..7be31580 100644 --- a/lib/capybara/node/element.rb +++ b/lib/capybara/node/element.rb @@ -126,7 +126,6 @@ module Capybara # # @return [Capybara::Node::Element] The element def select_option(wait: nil) - warn "Attempt to select disabled option: #{value || text}" if disabled? synchronize(wait) { base.select_option } self end @@ -276,7 +275,8 @@ module Capybara # @return [String] The tag name of the element # def tag_name - synchronize { base.tag_name } + # Element type is immutable so cache it + @tag_name ||= synchronize { base.tag_name } end ## diff --git a/lib/capybara/selenium/node.rb b/lib/capybara/selenium/node.rb index 62591cdc..e8429bac 100644 --- a/lib/capybara/selenium/node.rb +++ b/lib/capybara/selenium/node.rb @@ -56,10 +56,12 @@ class Capybara::Selenium::Node < Capybara::Driver::Node def set(value, **options) raise ArgumentError, "Value cannot be an Array when 'multiple' attribute is not present. Not a #{value.class}" if value.is_a?(Array) && !multiple? - tag_name, type = attrs(:tagName, :type) - case tag_name.downcase + tag_name, type = attrs(:tagName, :type).map { |val| val&.downcase } + @tag_name ||= tag_name + + case tag_name when 'input' - case type.downcase + case type when 'radio' click when 'checkbox' @@ -78,7 +80,7 @@ class Capybara::Selenium::Node < Capybara::Driver::Node when 'textarea' set_text(value, options) else - set_content_editable(value) if content_editable? + set_content_editable(value) end end @@ -136,7 +138,7 @@ class Capybara::Selenium::Node < Capybara::Driver::Node end def tag_name - native.tag_name.downcase + @tag_name ||= native.tag_name.downcase end def visible?; boolean_attr(native.displayed?); end @@ -291,20 +293,24 @@ private # Ensure we are focused on the element click - driver.execute_script <<-JS, self - var range = document.createRange(); - var sel = window.getSelection(); - arguments[0].focus(); - range.selectNodeContents(arguments[0]); - sel.removeAllRanges(); - sel.addRange(range); + editable = driver.execute_script <<-JS, self + if (arguments[0].isContentEditable) { + var range = document.createRange(); + var sel = window.getSelection(); + arguments[0].focus(); + range.selectNodeContents(arguments[0]); + sel.removeAllRanges(); + sel.addRange(range); + return true; + } + return false; JS # The action api has a speed problem but both chrome and firefox 58 raise errors # if we use the faster direct send_keys. For now just send_keys to the element # we've already focused. # native.send_keys(value.to_s) - browser_action.send_keys(value.to_s).perform + browser_action.send_keys(value.to_s).perform if editable end def action_with_modifiers(click_options) diff --git a/lib/capybara/selenium/nodes/chrome_node.rb b/lib/capybara/selenium/nodes/chrome_node.rb index 7051239d..18d6e7a7 100644 --- a/lib/capybara/selenium/nodes/chrome_node.rb +++ b/lib/capybara/selenium/nodes/chrome_node.rb @@ -39,6 +39,14 @@ class Capybara::Selenium::ChromeNode < Capybara::Selenium::Node driver.evaluate_script("arguments[0].matches(':disabled, select:disabled *')", self) end + def select_option + # To optimize to only one check and then click + selected_or_disabled = driver.evaluate_script(<<~JS, self) + arguments[0].matches(':disabled, select:disabled *, :checked') + JS + click unless selected_or_disabled + end + private def file_errors diff --git a/lib/capybara/selenium/nodes/firefox_node.rb b/lib/capybara/selenium/nodes/firefox_node.rb index 44c823dc..b6803574 100644 --- a/lib/capybara/selenium/nodes/firefox_node.rb +++ b/lib/capybara/selenium/nodes/firefox_node.rb @@ -55,6 +55,14 @@ class Capybara::Selenium::FirefoxNode < Capybara::Selenium::Node scroll_if_needed { browser_action.move_to(native, 0, 0).move_to(native).perform } end + def select_option + # To optimize to only one check and then click + selected_or_disabled = driver.evaluate_script(<<~JS, self) + arguments[0].matches(':disabled, select:disabled *, :checked') + JS + click unless selected_or_disabled + end + private def click_with_options(click_options) diff --git a/lib/capybara/selenium/nodes/safari_node.rb b/lib/capybara/selenium/nodes/safari_node.rb index 1b316eb1..a57b27d9 100644 --- a/lib/capybara/selenium/nodes/safari_node.rb +++ b/lib/capybara/selenium/nodes/safari_node.rb @@ -19,8 +19,12 @@ class Capybara::Selenium::SafariNode < Capybara::Selenium::Node end def select_option - driver.execute_script("arguments[0].closest('select').scrollIntoView()", self) - super + # To optimize to only one check and then click + selected_or_disabled = driver.execute_script(<<~JS, self) + arguments[0].closest('select').scrollIntoView(); + return arguments[0].matches(':disabled, select:disabled *, :checked'); + JS + click unless selected_or_disabled end def unselect_option diff --git a/lib/capybara/spec/session/select_spec.rb b/lib/capybara/spec/session/select_spec.rb index cb6c5ae5..6d14a25d 100644 --- a/lib/capybara/spec/session/select_spec.rb +++ b/lib/capybara/spec/session/select_spec.rb @@ -141,11 +141,6 @@ Capybara::SpecHelper.spec '#select' do @session.select('Other', from: 'form_title') expect(@session.find_field('form_title').value).not_to eq 'Other' end - - it 'should warn' do - expect { @session.select('Other', from: 'form_title') }.to \ - output(/^Attempt to select disabled option: Other/).to_stderr - end end context 'with multiple select' do