Loosen restrictions on where 'Session#within_window' can be called from

lib/capybara/session.rb

@@ -472,14 +472,14 @@ module Capybara
     #   @raise [Capybara::WindowError]     if no window matches given block
     # @overload switch_to_window(window)
     #   @param window [Capybara::Window]   window that should be switched to
-    #   @raise [Capybara::Driver::Base#no_such_window_error] if unexistent (e.g. closed) window was passed
+    #   @raise [Capybara::Driver::Base#no_such_window_error] if non-existent (e.g. closed) window was passed
     #
     # @return [Capybara::Window]         window that has been switched to
-    # @raise [Capybara::ScopeError]        if this method is invoked inside `within`,
-    #   `within_frame` or `within_window` methods
+    # @raise [Capybara::ScopeError]        if this method is invoked inside `within` or
+    #   `within_frame` methods
     # @raise [ArgumentError]               if both or neither arguments were provided
     #
-    def switch_to_window(window = nil, options= {})
+    def switch_to_window(window = nil, options= {}, &window_locator)
       options, window = window, nil if window.is_a? Hash
 
       block_given = block_given?
@@ -487,34 +487,12 @@ module Capybara
         raise ArgumentError, "`switch_to_window` can take either a block or a window, not both"
       elsif !window && !block_given
         raise ArgumentError, "`switch_to_window`: either window or block should be provided"
-      elsif scopes.size > 1
+      elsif !scopes.last.nil?
         raise Capybara::ScopeError, "`switch_to_window` is not supposed to be invoked from "\
-                                    "`within`'s, `within_frame`'s' or `within_window`'s' block."
+                                    "`within` or `within_frame` blocks."
       end
 
-      if window
-        driver.switch_to_window(window.handle)
-        window
-      else
-        wait_time = Capybara::Queries::BaseQuery.wait(options, config.default_max_wait_time)
-        document.synchronize(wait_time, errors: [Capybara::WindowError]) do
-          original_window_handle = driver.current_window_handle
-          begin
-            driver.window_handles.each do |handle|
-              driver.switch_to_window handle
-              if yield
-                return Window.new(self, handle)
-              end
-            end
-          rescue => e
-            driver.switch_to_window(original_window_handle)
-            raise e
-          else
-            driver.switch_to_window(original_window_handle)
-            raise Capybara::WindowError, "Could not find a window matching block/lambda"
-          end
-        end
-      end
+      _switch_to_window(window, options, &window_locator)
     end
 
     ##
@@ -538,30 +516,35 @@ module Capybara
     #   @deprecated                            Pass window or lambda instead
     #   @param [String]                        handle, name, url or title of the window
     #
-    # @raise [Capybara::ScopeError]        if this method is invoked inside `within`,
-    #   `within_frame` or `within_window` methods
+    # @raise [Capybara::ScopeError]        if this method is invoked inside `within_frame` method
     # @return                              value returned by the block
     #
     def within_window(window_or_handle)
       if window_or_handle.instance_of?(Capybara::Window)
         original = current_window
-        switch_to_window(window_or_handle) unless original == window_or_handle
         scopes << nil
         begin
-          yield
+          _switch_to_window(window_or_handle) unless original == window_or_handle
+          begin
+            yield
+          ensure
+            _switch_to_window(original) unless original == window_or_handle
+          end
         ensure
-          @scopes.pop
-          switch_to_window(original) unless original == window_or_handle
+          scopes.pop
         end
       elsif window_or_handle.is_a?(Proc)
         original = current_window
-        switch_to_window { window_or_handle.call }
         scopes << nil
         begin
-          yield
+          _switch_to_window { window_or_handle.call }
+          begin
+            yield
+          ensure
+            _switch_to_window(original)
+          end
         ensure
-          @scopes.pop
-          switch_to_window(original)
+          scopes.pop
         end
       else
         offending_line = caller.first
@@ -572,7 +555,7 @@ module Capybara
           scopes << nil
           driver.within_window(window_or_handle) { yield }
         ensure
-          @scopes.pop
+          scopes.pop
         end
       end
     end
@@ -919,5 +902,37 @@ module Capybara
         end
       end
     end
+
+    def _switch_to_window(window = nil, options= {})
+      options, window = window, nil if window.is_a? Hash
+
+      raise Capybara::ScopeError, "Window cannot be switched inside a `within_frame` block" if scopes.include?(:frame)
+      raise Capybara::ScopeError, "Window cannot be switch inside a `within` block" unless scopes.last.nil?
+
+      if window
+        driver.switch_to_window(window.handle)
+        window
+      else
+        wait_time = Capybara::Queries::BaseQuery.wait(options, config.default_max_wait_time)
+        document.synchronize(wait_time, errors: [Capybara::WindowError]) do
+          original_window_handle = driver.current_window_handle
+          begin
+            driver.window_handles.each do |handle|
+              driver.switch_to_window handle
+              if yield
+                return Window.new(self, handle)
+              end
+            end
+          rescue => e
+            driver.switch_to_window(original_window_handle)
+            raise e
+          else
+            driver.switch_to_window(original_window_handle)
+            raise Capybara::WindowError, "Could not find a window matching block/lambda"
+          end
+        end
+      end
+    end
+
   end
 end

lib/capybara/spec/session/window/switch_to_window_spec.rb

@@ -79,7 +79,7 @@ Capybara::SpecHelper.spec '#switch_to_window', requires: [:windows] do
         @session.within(:css, '#doesNotOpenWindows') do
           @session.switch_to_window { @session.title == 'With Windows' }
         end
-      end.to raise_error(Capybara::ScopeError, "`switch_to_window` is not supposed to be invoked from `within`'s, `within_frame`'s' or `within_window`'s' block.")
+      end.to raise_error(Capybara::ScopeError, /`switch_to_window` is not supposed to be invoked/)
     end
 
     it "should raise error when invoked inside `within_frame` as it's nonsense" do
@@ -87,16 +87,18 @@ Capybara::SpecHelper.spec '#switch_to_window', requires: [:windows] do
         @session.within_frame('frameOne') do
           @session.switch_to_window { @session.title == 'With Windows' }
         end
-      end.to raise_error(Capybara::ScopeError, "`switch_to_window` is not supposed to be invoked from `within`'s, `within_frame`'s' or `within_window`'s' block.")
+      end.to raise_error(Capybara::ScopeError, /`switch_to_window` is not supposed to be invoked from/)
     end
 
-    it "should raise error when invoked inside `within_window` as it's nonsense" do
-      window = (@session.windows - [@window]).first
-      expect do
-        @session.within_window window do
-          @session.switch_to_window { @session.title == 'With Windows' }
-        end
-      end.to raise_error(Capybara::ScopeError, "`switch_to_window` is not supposed to be invoked from `within`'s, `within_frame`'s' or `within_window`'s' block.")
+    it "should allow to be called inside within_window and within_window will still return to original" do
+      other_windows = (@session.windows - [@window])
+      expect(@session.current_window).to eq(@window)
+      @session.within_window other_windows[0] do
+        expect(@session.current_window).to eq(other_windows[0])
+        @session.switch_to_window other_windows[1]
+        expect(@session.current_window).to eq(other_windows[1])
+      end
+      expect(@session.current_window).to eq(@window)
     end
 
     it "should raise error if window matching block wasn't found" do

lib/capybara/spec/session/window/within_window_spec.rb

@@ -57,10 +57,10 @@ Capybara::SpecHelper.spec '#within_window', requires: [:windows] do
       expect(@session.send(:scopes)).to eq([nil])
     end
 
-    it "should leave correct scopes after execution in case of error" do
+    it "should leave correct scopes after execution in case of error", requires: [:windows, :frames] do
       window = (@session.windows - [@window]).first
       expect do
-        @session.within 'html' do
+        @session.within_frame 'frameOne' do
           @session.within_window(window) {}
         end
       end.to raise_error(Capybara::ScopeError)
@@ -102,6 +102,31 @@ Capybara::SpecHelper.spec '#within_window', requires: [:windows] do
       expect(@session.title).to eq('With Windows')
     end
 
+    it "should be able to nest within_window" do
+      @session.within_window(->{ @session.title == 'Title of popup two'}) do
+        expect(@session).to have_css('#divInPopupTwo')
+        @session.within_window(->{ @session.title == 'Title of the first popup'}) do
+          expect(@session).to have_css('#divInPopupOne')
+        end
+        expect(@session).to have_css('#divInPopupTwo')
+        expect(@session).not_to have_css('divInPopupOne')
+      end
+      expect(@session).not_to have_css('#divInPopupTwo')
+      expect(@session).not_to have_css('divInPopupOne')
+      expect(@session.title).to eq('With Windows')
+    end
+
+    it "should work inside a normal scope" do
+      expect(@session).to have_css('#openWindow')
+      @session.within(:css, '#scope') do
+        @session.within_window(->{ @session.title == 'Title of the first popup'}) do
+          expect(@session).to have_css('#divInPopupOne')
+        end
+        expect(@session).to have_content('My scoped content')
+        expect(@session).not_to have_css('#openWindow')
+      end
+    end
+
     it "should raise error if window wasn't found" do
       expect do
         @session.within_window(->{ @session.title == 'Invalid title'}) do

lib/capybara/spec/views/with_windows.erb

@@ -46,5 +46,9 @@
     <button id="doesNotOpenWindows">Does not open windows</button>
 
     <iframe src="/frame_one" id="frameOne"></iframe>
+
+    <div id="scope">
+      <span>My scoped content</span>
+    </div>
   </body>
 </html>