Merge pull request #1722 from jnicklas/frame_api

new frame api for drivers

History.md

@@ -11,6 +11,7 @@ Release date: Unreleased
 * Raise error if Capybara.app_host/default_host are specified incorrectly [Thomas Walpole]
 * Capybara::Selector::FilterSet allows for sharing filter definitions between selectors [Thomas Walpole]
 * Remove need to pass nil locator in most node actions when locator is not needed [Thomas Walpole]
+* New frames API for drivers - Issue #1365 [Thomas Walpole]
 
 #Version 2.7.1
 Release date: 2016-05-01

lib/capybara/driver/base.rb

@@ -48,8 +48,12 @@ class Capybara::Driver::Base
     raise Capybara::NotSupportedByDriverError, 'Capybara::Driver::Base#status_code'
   end
 
-  def within_frame(frame_handle)
-    raise Capybara::NotSupportedByDriverError, 'Capybara::Driver::Base#within_frame'
+  ##
+  #
+  # @param frame [Capybara::Node::Element, :parent, :top]  The iframe element to switch to
+  #
+  def switch_to_frame(frame)
+    raise Capybara::NotSupportedByDriverError, 'Capybara::Driver::Base#switch_to_frame'
   end
 
   def current_window_handle

lib/capybara/selenium/driver.rb

@@ -131,29 +131,22 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
     end
   end
 
-  ##
-  #
-  # Webdriver supports frame name, id, index(zero-based) or {Capybara::Node::Element} to find iframe
-  #
-  # @overload within_frame(index)
-  #   @param [Integer] index                 index of a frame
-  # @overload within_frame(name_or_id)
-  #   @param [String] name_or_id             name or id of a frame
-  # @overload within_frame(element)
-  #   @param [Capybara::Node::Base] a_node   frame element
-  #
-  def within_frame(frame_handle)
-    frame_handle = frame_handle.native if frame_handle.is_a?(Capybara::Node::Base)
-    @frame_handles[browser.window_handle] ||= []
-    @frame_handles[browser.window_handle] << frame_handle
-    browser.switch_to.frame(frame_handle)
-    yield
-  ensure
+  def switch_to_frame(frame)
+    case frame
+    when :top
+      @frame_handles[browser.window_handle] = []
+      browser.switch_to.default_content
+    when :parent
       # would love to use browser.switch_to.parent_frame here
       # but it has an issue if the current frame is removed from within it
       @frame_handles[browser.window_handle].pop
       browser.switch_to.default_content
       @frame_handles[browser.window_handle].each { |fh| browser.switch_to.frame(fh) }
+    else
+      @frame_handles[browser.window_handle] ||= []
+      @frame_handles[browser.window_handle] << frame.native
+      browser.switch_to.frame(frame.native)
+    end
   end
 
   def current_window_handle

lib/capybara/session.rb

@@ -327,18 +327,48 @@ module Capybara
 
     ##
     #
-    # Execute the given block within the given iframe using given frame name or index.
-    # May be supported by not all drivers. Drivers that support it, may provide additional options.
+    # Execute the given block within the given iframe using given frame, frame name/id or index.
+    # May not be supported by all drivers.
     #
+    # @overload within_frame(element)
+    #   @param [Capybara::Node::Element]  frame element
+    # @overload within_frame(name)
+    #   @param [String] name           name/id of a frame
     # @overload within_frame(index)
     #   @param [Integer] index         index of a frame
-    # @overload within_frame(name)
-    #   @param [String] name           name of a frame
     #
-    def within_frame(frame_handle)
+    def within_frame(locator)
       scopes.push(nil)
-      driver.within_frame(frame_handle) do
+
+      #support older driver frame api for now
+      frame = case locator
+      when Capybara::Node::Element
+        locator
+      when String
+        find(:xpath, XPath.descendant(:iframe)[XPath.attr(:id).equals(locator) | XPath.attr(:name).equals(locator)].to_xpath(:exact))
+      when Integer
+        all(:css, 'iframe', minimum: locator+1)[locator]
+      else
+        raise ArgumentError
+      end
+
+      begin
+        driver.switch_to_frame(frame)
+        begin
           yield
+        ensure
+          driver.switch_to_frame(:parent)
+        end
+      rescue Capybara::NotSupportedByDriverError
+        # Support older driver frame API for now
+        if driver.respond_to?(:within_frame)
+          warn "Your driver (#{driver.class.name}) is using an older version of Capybara's frame API - please update your driver"
+          driver.within_frame(frame) do
+            yield
+          end
+        else
+          raise
+        end
       end
     ensure
       scopes.pop

lib/capybara/spec/session/within_frame_spec.rb

@@ -9,32 +9,38 @@ Capybara::SpecHelper.spec '#within_frame', :requires => [:frames] do
       expect(@session.find("//*[@id='divInFrameOne']").text).to eql 'This is the text of divInFrameOne'
     end
   end
+
   it "should find the div in FrameTwo" do
     @session.within_frame("frameTwo") do
       expect(@session.find("//*[@id='divInFrameTwo']").text).to eql 'This is the text of divInFrameTwo'
     end
   end
+
   it "should find the text div in the main window after finding text in frameOne" do
     @session.within_frame("frameOne") do
       expect(@session.find("//*[@id='divInFrameOne']").text).to eql 'This is the text of divInFrameOne'
     end
     expect(@session.find("//*[@id='divInMainWindow']").text).to eql 'This is the text for divInMainWindow'
   end
+
   it "should find the text div in the main window after finding text in frameTwo" do
     @session.within_frame("frameTwo") do
       expect(@session.find("//*[@id='divInFrameTwo']").text).to eql 'This is the text of divInFrameTwo'
     end
     expect(@session.find("//*[@id='divInMainWindow']").text).to eql 'This is the text for divInMainWindow'
   end
+
   it "should return the result of executing the block" do
     expect(@session.within_frame("frameOne") { "return value" }).to eql "return value"
   end
+
   it "should find the div given Element" do
     element = @session.find(:id, 'frameOne')
     @session.within_frame element do
       expect(@session.find("//*[@id='divInFrameOne']").text).to eql 'This is the text of divInFrameOne'
     end
   end
+
   it "should find multiple nested frames" do
     @session.within_frame 'parentFrame' do
       @session.within_frame 'childFrame' do
@@ -43,6 +49,7 @@ Capybara::SpecHelper.spec '#within_frame', :requires => [:frames] do
       end
     end
   end
+
   it "should reset scope when changing frames" do
     @session.within(:css, '#divInMainWindow') do
       @session.within_frame 'parentFrame' do