[rb] move new window functionality into TargetLocator to match other bindings

Author: titusfortner

rb/lib/selenium/webdriver/common/manager.rb

@@ -114,15 +114,13 @@ def logs
       end
 
       #
-      # Create a new top-level browsing context
-      # https://w3c.github.io/webdriver/#new-window
       # @param type [Symbol] Supports two values: :tab and :window.
-      #  Use :tab if you'd like the new window to share an OS-level window
-      #  with the current browsing context.
-      #  Use :window otherwise
       # @return [String] The value of the window handle
       #
       def new_window(type = :tab)
+        WebDriver.logger.deprecate('Manager#new_window', 'TargetLocator#new_window', id: :new_window) do
+          'e.g., `driver.switch_to.new_window(:tab)`'
+        end
         case type
         when :tab, :window
           result = @bridge.new_window(type)

rb/lib/selenium/webdriver/common/target_locator.rb

@@ -44,6 +44,34 @@ def parent_frame
         @bridge.switch_to_parent_frame
       end
 
+      #
+      # Switch to a new top-level browsing context
+      #
+      # @param type either :tab or :window
+      #
+
+      def new_window(type = :window)
+        unless %i[window tab].include?(type)
+          raise ArgumentError, "Valid types are :tab and :window, received: #{type.inspect}"
+        end
+
+        handle = @bridge.new_window(type)['handle']
+
+        if block_given?
+          execute_and_close = proc do
+            yield(self)
+            begin
+              @bridge.close
+            rescue Error::NoSuchWindowError
+              # window already closed
+            end
+          end
+          window(handle, &execute_and_close)
+        else
+          window(handle)
+        end
+      end
+
       #
       # switch to the given window handle
       #

rb/spec/integration/selenium/webdriver/manager_spec.rb

@@ -267,21 +267,8 @@ module WebDriver
       describe 'new_window' do
         after { ensure_single_window }
 
-        types = %i[tab window]
-        types.each do |type|
-          it "should be able to open a new #{type}" do
-            before_window_handles = driver.window_handles.length
-            driver.manage.new_window(type)
-            expect(driver.window_handles.length).to eq(before_window_handles + 1)
-          end
-        end
-
         it "returns an exception if an invalid type is provided" do
-          invalid_types = [:invalid, 'invalid', 'tab', 'window']
-          invalid_types.each do |type|
-            expect { driver.manage.new_window(type) }.to \
-              raise_error(ArgumentError, "invalid argument for type. Got: '#{type.inspect}'. Try :tab or :window")
-          end
+            expect { driver.manage.new_window }.to have_deprecated(:new_window)
         end
       end
     end # Options

rb/spec/integration/selenium/webdriver/target_locator_spec.rb

@@ -68,6 +68,47 @@ module WebDriver
           quit_driver
         end
 
+        describe '#new_window' do
+          it 'should switch to a new window' do
+            original_window = driver.window_handle
+            driver.switch_to.new_window(:window)
+
+            expect(driver.window_handles.size).to eq 2
+            expect(driver.window_handle).not_to eq original_window
+          end
+
+          it 'should switch to a new tab' do
+            original_window = driver.window_handle
+            driver.switch_to.new_window(:tab)
+
+            expect(driver.window_handles.size).to eq 2
+            expect(driver.window_handle).not_to eq original_window
+          end
+
+          it 'should raise exception when the new window type is not recognized' do
+            expect {
+              driver.switch_to.new_window(:unknown)
+            }.to raise_error(ArgumentError)
+          end
+
+          it 'should switch to the new window then close it when given a block' do
+            original_window = driver.window_handle
+
+            driver.switch_to.new_window do
+              expect(driver.window_handles.size).to eq 2
+            end
+
+            expect(driver.window_handles.size).to eq 1
+            expect(driver.window_handle).to eq original_window
+          end
+
+          it 'should not error if switching to a new window with a block that closes window' do
+            expect {
+              driver.switch_to.new_window { driver.close }
+            }.not_to raise_exception
+          end
+        end
+
         it 'should switch to a window and back when given a block' do
           driver.navigate.to url_for('xhtmlTest.html')