Move property filters back to the selectors

(this will all be squashed once it's ready)

lib/capybara/node/finders.rb

@@ -109,12 +109,12 @@ module Capybara
       # @return [Array[Capybara::Element]]           The found elements
       #
       def all(*args)
-        args, property_options = extract_property_options(args)
+        args, options = extract_normalized_options(args)
 
         selector = Capybara::Selector.normalize(*args)
         selector.xpaths.
           map    { |path| find_in_base(selector, path) }.flatten.
-          select { |node| filter_by_properties(node, property_options) }
+          select { |node| selector.filter(node) }
       end
 
       ##
@@ -132,13 +132,13 @@ module Capybara
       # @return [Capybara::Element]                  The found element or nil
       #
       def first(*args)
-        args, property_options  = extract_property_options(args)
+        args, options  = extract_normalized_options(args)
         found_elements = []
 
         selector = Capybara::Selector.normalize(*args)
         selector.xpaths.each do |path|
           find_in_base(selector, path).each do |node|
-            if filter_by_properties(node, property_options)
+            if selector.filter(node)
               found_elements << node
               return found_elements.last if not Capybara.prefer_visible_elements or node.visible?
             end
@@ -150,10 +150,10 @@ module Capybara
     protected
 
       def raise_find_error(*args)
-        args, property_options = extract_property_options(args)
-        normalized             = Capybara::Selector.normalize(*args)
-        message                = args.last[:message] || "Unable to find #{normalized.name} #{normalized.locator.inspect}"
-        message                = normalized.failure_message.call(self, normalized) if normalized.failure_message
+        args, options = extract_normalized_options(args)
+        normalized    = Capybara::Selector.normalize(*args)
+        message       = options[:message] || "Unable to find #{normalized.name} #{normalized.locator.inspect}"
+        message       = normalized.failure_message.call(self, normalized) if normalized.failure_message
 
         raise Capybara::ElementNotFound, message
       end
@@ -164,44 +164,24 @@ module Capybara
         end
       end
 
-      def extract_property_options(args)
-        xpath_options    = if args.last.is_a?(Hash) then args.pop.dup else {} end
-        property_options = [:text, :visible, :with, :checked, :unchecked, :selected].inject({}) do |opts, key|
-          opts[key] = xpath_options.delete(key) if xpath_options.has_key?(key)
-          opts
+      def extract_normalized_options(args)
+        options = if args.last.is_a?(Hash) then args.pop.dup else {} end
+
+        if text = options[:text]
+          options[:text] = Regexp.escape(text) unless text.kind_of?(Regexp)
         end
 
-        if text = property_options[:text]
-          property_options[:text] = Regexp.escape(text) unless text.kind_of?(Regexp)
+        if !options.has_key?(:visible)
+          options[:visible] = Capybara.ignore_hidden_elements
         end
 
-        if !property_options.has_key?(:visible)
-          property_options[:visible] = Capybara.ignore_hidden_elements
+        if selected = options[:selected]
+          options[:selected] = [selected].flatten
         end
 
-        if selected = property_options[:selected]
-          property_options[:selected] = [selected].flatten
-        end
-
-        [args.push(xpath_options), property_options]
+        [args.push(options), options]
       end
 
-      def filter_by_properties(node, property_options)
-        return false if property_options[:text]      and not node.text.match(property_options[:text])
-        return false if property_options[:visible]   and not node.visible?
-        return false if property_options[:with]      and not node.value == property_options[:with]
-        return false if property_options[:checked]   and not node.checked?
-        return false if property_options[:unchecked] and node.checked?
-        return false if property_options[:selected]  and not has_selected_options?(node, property_options[:selected])
-        true
-      end
-
-      private
-
-      def has_selected_options?(node, expected)
-        actual = node.all(:xpath, './/option').select { |option| option.selected? }.map { |option| option.text }
-        (expected - actual).empty?
-      end
     end
   end
 end

lib/capybara/selector.rb

@@ -1,12 +1,31 @@
 module Capybara
   class Selector
+    PROPERTY_OPTION_KEYS = [:text, :visible, :with, :checked, :unchecked, :selected]
+
     attr_reader :name
 
     class Normalized
-      attr_accessor :selector, :locator, :options, :xpaths
+      attr_accessor :selector, :locator, :options, :xpath_options, :property_options, :xpaths
 
       def failure_message; selector.failure_message; end
       def name; selector.name; end
+
+      def filter(node)
+        return false if property_options[:text]      and not node.text.match(property_options[:text])
+        return false if property_options[:visible]   and not node.visible?
+        return false if property_options[:with]      and not node.value == property_options[:with]
+        return false if property_options[:checked]   and not node.checked?
+        return false if property_options[:unchecked] and node.checked?
+        return false if property_options[:selected]  and not has_selected_options?(node, property_options[:selected])
+        true
+      end
+
+      private
+
+      def has_selected_options?(node, expected)
+        actual = node.all(:xpath, './/option').select { |option| option.selected? }.map { |option| option.text }
+        (expected - actual).empty?
+      end
     end
 
     class << self
@@ -25,6 +44,7 @@ module Capybara
       def normalize(*args)
         normalized = Normalized.new
         normalized.options = if args.last.is_a?(Hash) then args.pop else {} end
+        normalized.xpath_options, normalized.property_options = split_options(normalized.options)
 
         if args[1]
           normalized.selector = all[args[0]]
@@ -35,7 +55,7 @@ module Capybara
         end
         normalized.selector ||= all[Capybara.default_selector]
 
-        xpath = normalized.selector.call(normalized.locator, normalized.options)
+        xpath = normalized.selector.call(normalized.locator, normalized.xpath_options)
         if xpath.respond_to?(:to_xpaths)
           normalized.xpaths = xpath.to_xpaths
         else
@@ -43,6 +63,18 @@ module Capybara
         end
         normalized
       end
+
+      private
+
+      def split_options(options)
+        xpath_options = options.dup
+        property_options = PROPERTY_OPTION_KEYS.inject({}) do |opts, key|
+          opts[key] = xpath_options.delete(key) if xpath_options[key]
+          opts
+        end
+
+        [ xpath_options, property_options ]
+      end
     end
 
     def initialize(name, &block)
@@ -73,8 +105,8 @@ module Capybara
       @failure_message
     end
 
-    def call(locator, options={})
-      @xpath.call(locator, options)
+    def call(locator, xpath_options={})
+      @xpath.call(locator, xpath_options)
     end
 
     def match?(locator)
@@ -110,7 +142,7 @@ Capybara.add_selector(:link_or_button) do
 end
 
 Capybara.add_selector(:link) do
-  xpath { |locator, options| XPath::HTML.link(locator, options) }
+  xpath { |locator, xpath_options| XPath::HTML.link(locator, xpath_options) }
   failure_message { |node, selector| "no link with title, id or text '#{selector.locator}' found" }
 end
 
@@ -120,22 +152,22 @@ Capybara.add_selector(:button) do
 end
 
 Capybara.add_selector(:fillable_field) do
-  xpath { |locator, options| XPath::HTML.fillable_field(locator, options) }
+  xpath { |locator, xpath_options| XPath::HTML.fillable_field(locator, xpath_options) }
   failure_message { |node, selector| "no text field, text area or password field with id, name, or label '#{selector.locator}' found" }
 end
 
 Capybara.add_selector(:radio_button) do
-  xpath { |locator, options| XPath::HTML.radio_button(locator, options) }
+  xpath { |locator, xpath_options| XPath::HTML.radio_button(locator, xpath_options) }
   failure_message { |node, selector| "no radio button with id, name, or label '#{selector.locator}' found" }
 end
 
 Capybara.add_selector(:checkbox) do
-  xpath { |locator, options| XPath::HTML.checkbox(locator, options) }
+  xpath { |locator, xpath_options| XPath::HTML.checkbox(locator, xpath_options) }
   failure_message { |node, selector| "no checkbox with id, name, or label '#{selector.locator}' found" }
 end
 
 Capybara.add_selector(:select) do
-  xpath { |locator, options| XPath::HTML.select(locator, options) }
+  xpath { |locator, xpath_options| XPath::HTML.select(locator, xpath_options) }
   failure_message { |node, selector| "no select box with id, name, or label '#{selector.locator}' found" }
 end
 
@@ -149,7 +181,7 @@ Capybara.add_selector(:option) do
 end
 
 Capybara.add_selector(:file_field) do
-  xpath { |locator, options| XPath::HTML.file_field(locator, options) }
+  xpath { |locator, xpath_options| XPath::HTML.file_field(locator, xpath_options) }
   failure_message { |node, selector| "no file field with id, name, or label '#{selector.locator}' found" }
 end
 
@@ -158,5 +190,5 @@ Capybara.add_selector(:content) do
 end
 
 Capybara.add_selector(:table) do
-  xpath { |locator, options| XPath::HTML.table(locator, options) }
+  xpath { |locator, xpath_options| XPath::HTML.table(locator, xpath_options) }
 end