Minor fixups in drag modifiers and change option name

lib/capybara/selenium/extensions/html5_drag.rb

@@ -4,25 +4,32 @@ class Capybara::Selenium::Node
   module Html5Drag
     # Implement methods to emulate HTML5 drag and drop
 
-    def drag_to(element, html5: nil, delay: 0.05, modifier_keys: [])
+    def drag_to(element, html5: nil, delay: 0.05, drop_modifiers: [])
+      drop_modifiers = Array(drop_modifiers)
+
       driver.execute_script MOUSEDOWN_TRACKER
       scroll_if_needed { browser_action.click_and_hold(native).perform }
       html5 = !driver.evaluate_script(LEGACY_DRAG_CHECK, self) if html5.nil?
       if html5
-        perform_html5_drag(element, delay, modifier_keys)
+        perform_html5_drag(element, delay, drop_modifiers)
       else
-        perform_legacy_drag(element, modifier_keys)
+        perform_legacy_drag(element, drop_modifiers)
       end
     end
 
   private
 
-    def perform_legacy_drag(element, modifier_keys)
-      move_to(element, modifier_keys: modifier_keys)
+    def perform_legacy_drag(element, drop_modifiers)
+      element.scroll_if_needed do
+        # browser_action.move_to(element.native).release.perform
+        keys_down = modifiers_down(browser_action, drop_modifiers)
+        keys_up = modifiers_up(keys_down.move_to(element.native).release, drop_modifiers)
+        keys_up.perform
+      end
     end
 
-    def perform_html5_drag(element, delay, modifier_keys)
-      driver.evaluate_async_script HTML5_DRAG_DROP_SCRIPT, self, element, delay * 1000, modifier_keys
+    def perform_html5_drag(element, delay, drop_modifiers)
+      driver.evaluate_async_script HTML5_DRAG_DROP_SCRIPT, self, element, delay * 1000, normalize_keys(drop_modifiers)
       browser_action.release.perform
     end
 
@@ -185,14 +192,18 @@ class Capybara::Selenium::Node
       var source = arguments[0],
           target = arguments[1],
           step_delay = arguments[2],
-          modifier_keys = arguments[3],
+          drop_modifier_keys = arguments[3],
           callback = arguments[4];
 
       var dt = new DataTransfer();
       var opts = { cancelable: true, bubbles: true, dataTransfer: dt };
 
-      for (var i = 0; i < modifier_keys.length; i++) {
-        opts[modifier_keys[i] + 'Key'] = true;
+      for (var i = 0; i < drop_modifier_keys.length; i++) {
+        key = drop_modifier_keys[i];
+        if (key == "control"){
+          key = "ctrl"
+        }
+        opts[key + 'Key'] = true;
       }
 
       while (source && !source.draggable) {

lib/capybara/selenium/node.rb

@@ -134,17 +134,15 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
     scroll_if_needed { browser_action.move_to(native).perform }
   end
 
-  def drag_to(element, **kwargs)
+  def drag_to(element, drop_modifiers: [], **)
+    drop_modifiers = Array(drop_modifiers)
     # Due to W3C spec compliance - The Actions API no longer scrolls to elements when necessary
     # which means Seleniums `drag_and_drop` is now broken - do it manually
     scroll_if_needed { browser_action.click_and_hold(native).perform }
-    move_to(element, **kwargs)
-  end
-
-  def move_to(element, modifier_keys: [], **)
+    # element.scroll_if_needed { browser_action.move_to(element.native).release.perform }
     element.scroll_if_needed do
-      keys_down = modifiers_down(browser_action, modifier_keys)
-      keys_up = modifiers_up(keys_down.move_to(element.native).release, modifier_keys)
+      keys_down = modifiers_down(browser_action, drop_modifiers)
+      keys_up = modifiers_up(keys_down.move_to(element.native).release, drop_modifiers)
       keys_up.perform
     end
   end
@@ -382,12 +380,12 @@ private
   end
 
   def modifiers_down(actions, keys)
-    each_key(keys) { |key| actions = actions.key_down(key) }
+    each_key(keys) { |key| actions.key_down(key) }
     actions
   end
 
   def modifiers_up(actions, keys)
-    each_key(keys) { |key| actions = actions.key_up(key) }
+    each_key(keys) { |key| actions.key_up(key) }
     actions
   end
 
@@ -403,18 +401,21 @@ private
     browser.action
   end
 
-  def each_key(keys)
-    keys.each do |key|
-      key = case key
+  def normalize_keys(keys)
+    keys.map do |key|
+      case key
       when :ctrl then :control
       when :command, :cmd then :meta
       else
         key
       end
-      yield key
     end
   end
 
+  def each_key(keys)
+    normalize_keys(keys).each { |key| yield(key) }
+  end
+
   def find_context
     native
   end

lib/capybara/selenium/nodes/chrome_node.rb

@@ -75,12 +75,15 @@ class Capybara::Selenium::ChromeNode < Capybara::Selenium::Node
 
 private
 
-  def perform_legacy_drag(element, modifier_keys = [])
+  def perform_legacy_drag(element, drop_modifiers)
     return super if chromedriver_fixed_actions_key_state? || !w3c? || element.obscured?
 
+    raise ArgumentError, 'Modifier keys are not supported while dragging in this version of Chrome.' unless drop_modifiers.empty?
+
     # W3C Chrome/chromedriver < 77 doesn't maintain mouse button state across actions API performs
     # https://bugs.chromium.org/p/chromedriver/issues/detail?id=2981
-    move_to(element, modifier_keys: modifier_keys)
+    browser_action.release.perform
+    browser_action.click_and_hold(native).move_to(element.native).release.perform
   end
 
   def file_errors

lib/capybara/spec/session/node_spec.rb

@@ -480,7 +480,7 @@ Capybara::SpecHelper.spec 'node' do
         element = @session.find('//div[@id="drag"]')
         target = @session.find('//div[@id="drop"]')
 
-        element.drag_to(target, modifier_keys: [modifier_key])
+        element.drag_to(target, drop_modifiers: modifier_key)
         expect(@session).to have_xpath("//div[contains(., 'Dropped!-#{modifier_key}')]")
       end
     end
@@ -491,17 +491,26 @@ Capybara::SpecHelper.spec 'node' do
       element = @session.find('//div[@id="drag"]')
       target = @session.find('//div[@id="drop"]')
 
-      modifier_keys = %I[
-        alt
-        ctrl
-        meta
-        shift
-      ]
+      modifier_keys = %I[alt ctrl meta shift]
 
-      element.drag_to(target, modifier_keys: modifier_keys)
+      element.drag_to(target, drop_modifiers: modifier_keys)
       expect(@session).to have_xpath("//div[contains(., 'Dropped!-#{modifier_keys.join('-')}')]")
     end
 
+    it 'should support key aliases' do
+      { control: :ctrl,
+        command: :meta,
+        cmd: :meta }.each do |(key_alias, key)|
+        @session.visit('/with_js')
+
+        element = @session.find('//div[@id="drag"]')
+        target = @session.find('//div[@id="drop"]')
+
+        element.drag_to(target, drop_modifiers: [key_alias])
+        expect(target).to have_text("Dropped!-#{key}", exact: true)
+      end
+    end
+
     context 'HTML5', requires: %i[js html5_drag] do
       it 'should HTML5 drag and drop an object' do
         @session.visit('/with_js')
@@ -592,18 +601,13 @@ Capybara::SpecHelper.spec 'node' do
       end
 
       it 'should simulate a single held down modifier key' do
-        %I[
-          alt
-          ctrl
-          meta
-          shift
-        ].each do |modifier_key|
+        %I[alt ctrl meta shift].each do |modifier_key|
           @session.visit('/with_js')
 
           element = @session.find('//div[@id="drag_html5"]')
           target = @session.find('//div[@id="drop_html5"]')
 
-          element.drag_to(target, modifier_keys: [modifier_key])
+          element.drag_to(target, drop_modifiers: modifier_key)
 
           expect(@session).to have_xpath("//div[contains(., 'HTML5 Dropped string: text/plain drag_html5-#{modifier_key}')]")
         end
@@ -615,16 +619,25 @@ Capybara::SpecHelper.spec 'node' do
         element = @session.find('//div[@id="drag_html5"]')
         target = @session.find('//div[@id="drop_html5"]')
 
-        modifier_keys = %I[
-          alt
-          ctrl
-          meta
-          shift
-        ]
+        modifier_keys = %I[alt ctrl meta shift]
 
-        element.drag_to(target, modifier_keys: modifier_keys)
+        element.drag_to(target, drop_modifiers: modifier_keys)
         expect(@session).to have_xpath("//div[contains(., 'HTML5 Dropped string: text/plain drag_html5-#{modifier_keys.join('-')}')]")
       end
+
+      it 'should support key aliases' do
+        { control: :ctrl,
+          command: :meta,
+          cmd: :meta }.each do |(key_alias, key)|
+          @session.visit('/with_js')
+
+          element = @session.find('//div[@id="drag_html5"]')
+          target = @session.find('//div[@id="drop_html5"]')
+
+          element.drag_to(target, drop_modifiers: [key_alias])
+          expect(target).to have_text(%r{^HTML5 Dropped string: text/plain drag_html5-#{key}$}m, exact: true)
+        end
+      end
     end
   end