From 2ffbb3c6a55b9b3de13819b171c331ef9d125d4e Mon Sep 17 00:00:00 2001
From: Thomas Walpole <twalpole@gmail.com>
Date: Thu, 3 Jan 2019 13:00:38 -0800
Subject: [PATCH] Allow drivers to provide initial element visibility with
 found elements and receive other optional filtering options for
 find_xpath/css. Implement initial visibility return for Selenium Use text
 content to pre-filter XPath searches

---
 Gemfile                                       |  2 +-
 capybara.gemspec                              |  2 +
 gemfiles/Gemfile.gumbo                        |  2 +-
 lib/capybara/driver/base.rb                   |  4 +-
 lib/capybara/driver/node.rb                   |  5 +-
 lib/capybara/node/base.rb                     | 16 ++++--
 lib/capybara/node/element.rb                  |  8 +++
 lib/capybara/node/simple.rb                   |  8 ++-
 lib/capybara/queries/selector_query.rb        | 56 ++++++++++++++++---
 lib/capybara/selenium/driver.rb               | 18 +++---
 .../driver_specializations/chrome_driver.rb   |  4 +-
 .../driver_specializations/firefox_driver.rb  |  4 +-
 lib/capybara/selenium/extensions/find.rb      | 50 +++++++++++++++++
 lib/capybara/selenium/node.rb                 | 24 +++++---
 .../spec/session/has_selector_spec.rb         |  7 +++
 lib/capybara/spec/views/with_html.erb         |  2 +-
 spec/result_spec.rb                           |  4 +-
 spec/spec_helper.rb                           |  2 +
 18 files changed, 172 insertions(+), 46 deletions(-)
 create mode 100644 lib/capybara/selenium/extensions/find.rb

diff --git a/Gemfile b/Gemfile
index b4a04fc1..b1dd4737 100644
--- a/Gemfile
+++ b/Gemfile
@@ -5,7 +5,7 @@ source 'https://rubygems.org'
 gem 'bundler', '< 3.0'
 gemspec
 
-gem 'xpath', git: 'git://github.com/teamcapybara/xpath.git'
+gem 'xpath', github: 'teamcapybara/xpath'
 
 group :doc do
   gem 'redcarpet', platforms: :mri
diff --git a/capybara.gemspec b/capybara.gemspec
index e356014f..91f25a2e 100644
--- a/capybara.gemspec
+++ b/capybara.gemspec
@@ -37,6 +37,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency('cucumber', ['>= 2.3.0'])
   s.add_development_dependency('erubi') # dependency specification needed by rbx
   s.add_development_dependency('fuubar', ['>= 1.0.0'])
+  s.add_development_dependency('irb')
   s.add_development_dependency('launchy', ['>= 2.0.4'])
   s.add_development_dependency('minitest')
   s.add_development_dependency('puma')
@@ -45,6 +46,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency('rubocop')
   s.add_development_dependency('rubocop-rspec')
   s.add_development_dependency('selenium-webdriver', ['~>3.5'])
+  s.add_development_dependency('selenium_statistics')
   s.add_development_dependency('sinatra', ['>= 1.4.0'])
   s.add_development_dependency('webdrivers', ['>=3.6.0']) if ENV['CI']
   s.add_development_dependency('yard', ['>= 0.9.0'])
diff --git a/gemfiles/Gemfile.gumbo b/gemfiles/Gemfile.gumbo
index c5d5a78a..93164ad8 100644
--- a/gemfiles/Gemfile.gumbo
+++ b/gemfiles/Gemfile.gumbo
@@ -3,5 +3,5 @@ source 'https://rubygems.org'
 gem 'bundler', '< 3.0'
 gemspec path: '..'
 
-gem 'xpath', :git => 'git://github.com/teamcapybara/xpath.git'
+gem 'xpath', github: 'teamcapybara/xpath'
 gem 'nokogumbo'
\ No newline at end of file
diff --git a/lib/capybara/driver/base.rb b/lib/capybara/driver/base.rb
index 1b351ac4..f2d1fb4f 100644
--- a/lib/capybara/driver/base.rb
+++ b/lib/capybara/driver/base.rb
@@ -15,11 +15,11 @@ class Capybara::Driver::Base
     raise NotImplementedError
   end
 
-  def find_xpath(query)
+  def find_xpath(query, **options)
     raise NotImplementedError
   end
 
-  def find_css(query)
+  def find_css(query, **options)
     raise NotImplementedError
   end
 
diff --git a/lib/capybara/driver/node.rb b/lib/capybara/driver/node.rb
index b761319d..0653f604 100644
--- a/lib/capybara/driver/node.rb
+++ b/lib/capybara/driver/node.rb
@@ -3,11 +3,12 @@
 module Capybara
   module Driver
     class Node
-      attr_reader :driver, :native
+      attr_reader :driver, :native, :initial_visibility
 
-      def initialize(driver, native)
+      def initialize(driver, native, initial_visibility = nil)
         @driver = driver
         @native = native
+        @initial_visibility = initial_visibility
       end
 
       def all_text
diff --git a/lib/capybara/node/base.rb b/lib/capybara/node/base.rb
index 0b9684f2..3b21d559 100644
--- a/lib/capybara/node/base.rb
+++ b/lib/capybara/node/base.rb
@@ -95,13 +95,21 @@ module Capybara
       end
 
       # @api private
-      def find_css(css)
-        base.find_css(css)
+      def find_css(css, **options)
+        if base.method(:find_css).arity != 1
+          base.find_css(css, **options)
+        else
+          base.find_css(css)
+        end
       end
 
       # @api private
-      def find_xpath(xpath)
-        base.find_xpath(xpath)
+      def find_xpath(xpath, **options)
+        if base.method(:find_css).arity != 1
+          base.find_xpath(xpath, **options)
+        else
+          base.find_xpath(xpath)
+        end
       end
 
       # @api private
diff --git a/lib/capybara/node/element.rb b/lib/capybara/node/element.rb
index d8889784..4c111107 100644
--- a/lib/capybara/node/element.rb
+++ b/lib/capybara/node/element.rb
@@ -465,6 +465,14 @@ module Capybara
         %(Obsolete #<Capybara::Node::Element>)
       end
 
+      def initial_visibility
+        if base.respond_to? :initial_visibility
+          base.initial_visibility
+        else
+          nil
+        end
+      end
+
       STYLE_SCRIPT = <<~JS
         (function(){
           var s = window.getComputedStyle(this);
diff --git a/lib/capybara/node/simple.rb b/lib/capybara/node/simple.rb
index f2e0f7c6..270a6106 100644
--- a/lib/capybara/node/simple.rb
+++ b/lib/capybara/node/simple.rb
@@ -164,12 +164,12 @@ module Capybara
       end
 
       # @api private
-      def find_css(css)
+      def find_css(css, **_options)
         native.css(css)
       end
 
       # @api private
-      def find_xpath(xpath)
+      def find_xpath(xpath, **_options)
         native.xpath(xpath)
       end
 
@@ -178,6 +178,10 @@ module Capybara
         Capybara.session_options
       end
 
+      def initial_visibility
+        nil
+      end
+
     private
 
       def option_value(option)
diff --git a/lib/capybara/queries/selector_query.rb b/lib/capybara/queries/selector_query.rb
index 634a31d7..b7e86e6f 100644
--- a/lib/capybara/queries/selector_query.rb
+++ b/lib/capybara/queries/selector_query.rb
@@ -14,6 +14,7 @@ module Capybara
                      **options,
                      &filter_block)
         @resolved_node = nil
+        @resolved_count = 0
         @options = options.dup
         super(@options)
         self.session_options = session_options
@@ -106,7 +107,9 @@ module Capybara
         exact = exact? if exact.nil?
         expr = apply_expression_filters(@expression)
         expr = exact ? expr.to_xpath(:exact) : expr.to_s if expr.respond_to?(:to_xpath)
-        filtered_expression(expr)
+        expr = filtered_expression(expr)
+        expr = "(#{expr})[#{xpath_text_conditions}]" if try_text_match_in_expression?
+        expr
       end
 
       def css
@@ -117,6 +120,7 @@ module Capybara
       def resolve_for(node, exact = nil)
         applied_filters.clear
         @resolved_node = node
+        @resolved_count += 1
         node.synchronize do
           children = find_nodes_by_selector_format(node, exact).map(&method(:to_element))
           Capybara::Result.new(children, self)
@@ -138,6 +142,26 @@ module Capybara
 
     private
 
+      def text_fragments
+        text = (options[:text] || options[:exact_text])
+        text.is_a?(String) ? text.split : []
+      end
+
+      def xpath_text_conditions
+        (options[:text] || options[:exact_text]).split.map { |txt| XPath.contains(txt) }.reduce(&:&)
+      end
+
+      def try_text_match_in_expression?
+        first_try? &&
+          (options[:text] || options[:exact_text]).is_a?(String) &&
+          @resolved_node&.respond_to?(:session) &&
+          @resolved_node.session.driver.wait?
+      end
+
+      def first_try?
+        @resolved_count == 1
+      end
+
       def show_for_stage(only_applied)
         lambda do |stage = :any|
           !only_applied || (stage == :any ? applied_filters.any? : applied_filters.include?(stage))
@@ -156,10 +180,24 @@ module Capybara
       end
 
       def find_nodes_by_selector_format(node, exact)
+        options = {}
+        options[:uses_visibility] = true unless visible == :all
+        options[:texts] = text_fragments unless selector.format == :xpath
+
         if selector.format == :css
-          node.find_css(css)
+          if node.method(:find_css).arity != 1
+            node.find_css(css, **options)
+          else
+            node.find_css(css)
+          end
+        elsif selector.format == :xpath
+          if node.method(:find_xpath).arity != 1
+            node.find_xpath(xpath(exact), **options)
+          else
+            node.find_xpath(xpath(exact))
+          end
         else
-          node.find_xpath(xpath(exact))
+          raise ArgumentError, "Unknown format: #{selector.format}"
         end
       end
 
@@ -318,12 +356,12 @@ module Capybara
       def matches_system_filters?(node)
         applied_filters << :system
 
-        matches_id_filter?(node) &&
+        matches_visible_filter?(node) &&
+          matches_id_filter?(node) &&
           matches_class_filter?(node) &&
           matches_style_filter?(node) &&
           matches_text_filter?(node) &&
-          matches_exact_text_filter?(node) &&
-          matches_visible_filter?(node)
+          matches_exact_text_filter?(node)
       end
 
       def matches_id_filter?(node)
@@ -377,8 +415,10 @@ module Capybara
 
       def matches_visible_filter?(node)
         case visible
-        when :visible then node.visible?
-        when :hidden then !node.visible?
+        when :visible then
+          node.initial_visibility || (node.initial_visibility.nil? && node.visible?)
+        when :hidden then
+          (node.initial_visibility == false) || (node.initial_visibility.nil? && !node.visible?)
         else true
         end
       end
diff --git a/lib/capybara/selenium/driver.rb b/lib/capybara/selenium/driver.rb
index 66c9a3bb..31819e33 100644
--- a/lib/capybara/selenium/driver.rb
+++ b/lib/capybara/selenium/driver.rb
@@ -4,6 +4,8 @@ require 'uri'
 require 'English'
 
 class Capybara::Selenium::Driver < Capybara::Driver::Base
+  include Capybara::Selenium::Find
+
   DEFAULT_OPTIONS = {
     browser: :firefox,
     clear_local_storage: nil,
@@ -73,14 +75,6 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
     browser.current_url
   end
 
-  def find_xpath(selector)
-    browser.find_elements(:xpath, selector).map(&method(:build_node))
-  end
-
-  def find_css(selector)
-    browser.find_elements(:css, selector).map(&method(:build_node))
-  end
-
   def wait?; true; end
   def needs_server?; true; end
 
@@ -348,8 +342,12 @@ private
     end
   end
 
-  def build_node(native_node)
-    ::Capybara::Selenium::Node.new(self, native_node)
+  def find_context
+    browser
+  end
+
+  def build_node(native_node, initial_visibility = nil)
+    ::Capybara::Selenium::Node.new(self, native_node, initial_visibility)
   end
 
   def specialize_driver(sel_driver)
diff --git a/lib/capybara/selenium/driver_specializations/chrome_driver.rb b/lib/capybara/selenium/driver_specializations/chrome_driver.rb
index f0d73dab..97b059b5 100644
--- a/lib/capybara/selenium/driver_specializations/chrome_driver.rb
+++ b/lib/capybara/selenium/driver_specializations/chrome_driver.rb
@@ -51,8 +51,8 @@ private
     result['value']
   end
 
-  def build_node(native_node)
-    ::Capybara::Selenium::ChromeNode.new(self, native_node)
+  def build_node(native_node, initial_visibility = nil)
+    ::Capybara::Selenium::ChromeNode.new(self, native_node, initial_visibility)
   end
 
   def bridge
diff --git a/lib/capybara/selenium/driver_specializations/firefox_driver.rb b/lib/capybara/selenium/driver_specializations/firefox_driver.rb
index 124cc3fe..6b4d298c 100644
--- a/lib/capybara/selenium/driver_specializations/firefox_driver.rb
+++ b/lib/capybara/selenium/driver_specializations/firefox_driver.rb
@@ -44,7 +44,7 @@ module Capybara::Selenium::Driver::FirefoxDriver
 
 private
 
-  def build_node(native_node)
-    ::Capybara::Selenium::FirefoxNode.new(self, native_node)
+  def build_node(native_node, initial_visibility = nil)
+    ::Capybara::Selenium::FirefoxNode.new(self, native_node, initial_visibility)
   end
 end
diff --git a/lib/capybara/selenium/extensions/find.rb b/lib/capybara/selenium/extensions/find.rb
new file mode 100644
index 00000000..6abe6950
--- /dev/null
+++ b/lib/capybara/selenium/extensions/find.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+module Capybara
+  module Selenium
+    module Find
+      def find_xpath(selector, uses_visibility: false, **_options)
+        find_by(:xpath, selector, uses_visibility: uses_visibility, texts: [])
+      end
+
+      def find_css(selector, uses_visibility: false, texts: [], **_options)
+        find_by(:css, selector, uses_visibility: uses_visibility, texts: texts)
+      end
+
+    private
+
+      def find_by(format, selector, uses_visibility:, texts:)
+        els = find_context.find_elements(format, selector)
+        els = filter_by_text(els, texts) if (els.size > 1) && !texts.empty?
+        visibilities = if uses_visibility && els.size > 1
+          element_visibilities(els)
+        else
+          []
+        end
+        els.map.with_index { |el, idx| build_node(el, visibilities[idx]) }
+      end
+
+      def element_visibilities(elements)
+        es_context = respond_to?(:execute_script) ? self : driver
+        es_context.execute_script <<~JS, elements
+          return arguments[0].map(#{is_displayed_atom})
+        JS
+      end
+
+      def filter_by_text(elements, texts)
+        es_context = respond_to?(:execute_script) ? self : driver
+        es_context.execute_script <<~JS, elements, texts
+          var texts = arguments[1]
+          return arguments[0].filter(function(el){
+            var content = el.textContent.toLowerCase();
+            return texts.every(function(txt){ return content.includes(txt.toLowerCase()) });
+          })
+        JS
+      end
+
+      def is_displayed_atom # rubocop:disable Naming/PredicateName
+        @is_displayed_atom ||= browser.send(:bridge).send(:read_atom, 'isDisplayed')
+      end
+    end
+  end
+end
diff --git a/lib/capybara/selenium/node.rb b/lib/capybara/selenium/node.rb
index 87551018..4f58cda8 100644
--- a/lib/capybara/selenium/node.rb
+++ b/lib/capybara/selenium/node.rb
@@ -2,9 +2,11 @@
 
 # Selenium specific implementation of the Capybara::Driver::Node API
 
+require 'capybara/selenium/extensions/find'
 require 'capybara/selenium/extensions/scroll'
 
 class Capybara::Selenium::Node < Capybara::Driver::Node
+  include Capybara::Selenium::Find
   include Capybara::Selenium::Scroll
 
   def visible_text
@@ -153,14 +155,6 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
     native.attribute('isContentEditable')
   end
 
-  def find_xpath(locator)
-    native.find_elements(:xpath, locator).map { |el| self.class.new(driver, el) }
-  end
-
-  def find_css(locator)
-    native.find_elements(:css, locator).map { |el| self.class.new(driver, el) }
-  end
-
   def ==(other)
     native == other.native
   end
@@ -331,8 +325,12 @@ private
     each_key(keys) { |key| actions.key_up(key) }
   end
 
+  def browser
+    driver.browser
+  end
+
   def browser_action
-    driver.browser.action
+    browser.action
   end
 
   def each_key(keys)
@@ -347,6 +345,14 @@ private
     end
   end
 
+  def find_context
+    native
+  end
+
+  def build_node(native_node, initial_visibility = nil)
+    self.class.new(driver, native_node, initial_visibility)
+  end
+
   GET_XPATH_SCRIPT = <<~'JS'
     (function(el, xml){
       var xpath = '';
diff --git a/lib/capybara/spec/session/has_selector_spec.rb b/lib/capybara/spec/session/has_selector_spec.rb
index e4f0393e..1f57775b 100644
--- a/lib/capybara/spec/session/has_selector_spec.rb
+++ b/lib/capybara/spec/session/has_selector_spec.rb
@@ -56,6 +56,7 @@ Capybara::SpecHelper.spec '#has_selector?' do
   context 'with text' do
     it 'should discard all matches where the given string is not contained' do
       expect(@session).to have_selector('//p//a', text: 'Redirect', count: 1)
+      expect(@session).to have_selector(:css, 'p a', text: 'Redirect', count: 1)
       expect(@session).not_to have_selector('//p', text: 'Doesnotexist')
     end
 
@@ -191,5 +192,11 @@ Capybara::SpecHelper.spec '#has_no_selector?' do
       expect(@session).not_to have_no_selector('//p//a', text: /re[dab]i/i, count: 1)
       expect(@session).to have_no_selector('//p//a', text: /Red$/)
     end
+
+    it 'should error when matching element exists' do
+      expect do
+        expect(@session).to have_no_selector('//h2', text: 'Header Class Test Five')
+      end.to raise_error RSpec::Expectations::ExpectationNotMetError
+    end
   end
 end
diff --git a/lib/capybara/spec/views/with_html.erb b/lib/capybara/spec/views/with_html.erb
index 2fcf270d..58072b69 100644
--- a/lib/capybara/spec/views/with_html.erb
+++ b/lib/capybara/spec/views/with_html.erb
@@ -12,7 +12,7 @@
 <h2 class="head" id="h2two">Header Class Test Two</h2>
 <h2 class="head" id="h2_">Header Class Test Three</h2>
 <h2 class="head">Header Class Test Four</h2>
-<h2 class="head">Header Class Test Five</h2>
+<h2 class="head">Header Class <span style="display: none">Random</span>Test Five</h2>
 
 <span class="number">42</span>
 <span>Other span</span>
diff --git a/spec/result_spec.rb b/spec/result_spec.rb
index cc3ef8c3..358bcf38 100644
--- a/spec/result_spec.rb
+++ b/spec/result_spec.rb
@@ -88,7 +88,7 @@ RSpec.describe Capybara::Result do
   it 'should catch invalid element errors during filtering' do
     allow_any_instance_of(Capybara::Node::Simple).to receive(:text).and_raise(StandardError)
     allow_any_instance_of(Capybara::Node::Simple).to receive(:session).and_return(
-      instance_double('Capybara::Session', driver: instance_double('Capybara::Driver::Base', invalid_element_errors: [StandardError]))
+      instance_double('Capybara::Session', driver: instance_double('Capybara::Driver::Base', invalid_element_errors: [StandardError], wait?: false))
     )
     result = string.all('//li', text: 'Alpha')
     expect(result.size).to eq 0
@@ -97,7 +97,7 @@ RSpec.describe Capybara::Result do
   it 'should return non-invalid element errors during filtering' do
     allow_any_instance_of(Capybara::Node::Simple).to receive(:text).and_raise(StandardError)
     allow_any_instance_of(Capybara::Node::Simple).to receive(:session).and_return(
-      instance_double('Capybara::Session', driver: instance_double('Capybara::Driver::Base', invalid_element_errors: [ArgumentError]))
+      instance_double('Capybara::Session', driver: instance_double('Capybara::Driver::Base', invalid_element_errors: [ArgumentError], wait?: false))
     )
     expect do
       string.all('//li', text: 'Alpha').to_a
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index c201d039..d4e1ac36 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -3,6 +3,7 @@
 require 'rspec/expectations'
 require 'capybara/spec/spec_helper'
 require 'webdrivers' if ENV['CI']
+require 'selenium_statistics'
 
 module Capybara
   module SpecHelper
@@ -53,4 +54,5 @@ RSpec.configure do |config|
   Capybara::SpecHelper.configure(config)
   config.filter_run_including focus_: true unless ENV['CI']
   config.run_all_when_everything_filtered = true
+  config.after(:suite) { SeleniumStatistics.print_results }
 end