Fix an issue where drag positions could get out of sync after scrolling

Summary:
Ref T5240. Currently, we calculate drag positions assuming the "ghost" element is not present (it isn't, usually), then adjust them while dragging to account for the ghost.

However, this fails after scrolling: we dirty the cache, but the ghost //is// present. We continue adjusting for it, but essentially double-adjust. This leads to scroll positions being about 80-ish px off from where they should be.

Test Plan:
  - Begin dragging a task in a long task list.
  - While dragging, use mousewheel to scroll to the bottom of the list.
  - Drag task downward through the list.
    - Before fix: ghost is off by, like, an inch or so.
    - After fix: ghost position is accurate to cursor position.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5240

Differential Revision: https://secure.phabricator.com/D15157

resources/celerity/map.php

@@ -8,7 +8,7 @@
 return array(
   'names' => array(
     'core.pkg.css' => '0f87bfe0',
-    'core.pkg.js' => 'a79eed25',
+    'core.pkg.js' => 'bf947f93',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '2de124c9',
     'differential.pkg.js' => '5c2ba922',
@@ -446,7 +446,7 @@ return array(
     'rsrc/js/application/uiexample/notification-example.js' => '8ce821c5',
     'rsrc/js/core/Busy.js' => '59a7976a',
     'rsrc/js/core/DragAndDropFileUpload.js' => 'ad10aeac',
-    'rsrc/js/core/DraggableList.js' => 'a16ec1c6',
+    'rsrc/js/core/DraggableList.js' => '255d85da',
     'rsrc/js/core/FileUpload.js' => '477359c8',
     'rsrc/js/core/Hovercard.js' => 'c6f720ff',
     'rsrc/js/core/KeyboardShortcut.js' => '1ae869f2',
@@ -741,7 +741,7 @@ return array(
     'phabricator-countdown-css' => 'e7544472',
     'phabricator-dashboard-css' => 'eb458607',
     'phabricator-drag-and-drop-file-upload' => 'ad10aeac',
-    'phabricator-draggable-list' => 'a16ec1c6',
+    'phabricator-draggable-list' => '255d85da',
     'phabricator-fatal-config-template-css' => '8e6c6fcd',
     'phabricator-feed-css' => 'ecd4ec57',
     'phabricator-file-upload' => '477359c8',
@@ -1021,6 +1021,14 @@ return array(
       'phabricator-drag-and-drop-file-upload',
       'phabricator-draggable-list',
     ),
+    '255d85da' => array(
+      'javelin-install',
+      'javelin-dom',
+      'javelin-stratcom',
+      'javelin-util',
+      'javelin-vector',
+      'javelin-magical-init',
+    ),
     '2926fff2' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1587,14 +1595,6 @@ return array(
       'javelin-dom',
       'javelin-reactor-dom',
     ),
-    'a16ec1c6' => array(
-      'javelin-install',
-      'javelin-dom',
-      'javelin-stratcom',
-      'javelin-util',
-      'javelin-vector',
-      'javelin-magical-init',
-    ),
     'a2828756' => array(
       'javelin-dom',
       'javelin-util',

webroot/rsrc/js/core/DraggableList.js

@@ -290,6 +290,11 @@ JX.install('DraggableList', {
       }
 
       this._target = false;
+
+      // Clear the target position cache, since adding or removing ghosts
+      // changes element positions.
+      this._dirtyTargetCache();
+
       return this;
     },
 
@@ -298,9 +303,6 @@ JX.install('DraggableList', {
       var targets = this._getTargets();
       var dragging = this._dragging;
 
-      var adjust_h = JX.Vector.getDim(ghost).y;
-      var adjust_y = JX.$V(ghost).y;
-
       // Find the node we're dragging the object underneath. This is the first
       // node in the list that's above the cursor. If that node is the node
       // we're dragging or its predecessor, don't select a target, because the
@@ -314,34 +316,23 @@ JX.install('DraggableList', {
       var cur_target = null;
       var trigger;
       for (var ii = 0; ii < targets.length; ii++) {
-
-        // If the drop target indicator is above the target, we need to adjust
-        // the target's trigger height down accordingly. This makes dragging
-        // items down the list smoother, because the target doesn't jump to the
-        // next item while the cursor is over it.
-
         trigger = targets[ii].y;
-        if (adjust_y <= trigger) {
-          trigger += adjust_h;
-        }
 
         // If the cursor is above this target, we aren't dropping underneath it.
-
         if (trigger >= p.y) {
           continue;
         }
 
         // Don't choose the dragged row or its predecessor as targets.
-
         cur_target = targets[ii].item;
         if (!dragging) {
           // If the item on the cursor isn't from this list, it can't be
           // dropped onto itself or its predecessor in this list.
         } else {
-          if (cur_target == dragging) {
+          if (cur_target === dragging) {
             cur_target = false;
           }
-          if (targets[ii - 1] && targets[ii - 1].item == dragging) {
+          if (targets[ii - 1] && (targets[ii - 1].item === dragging)) {
             cur_target = false;
           }
         }
@@ -480,7 +471,6 @@ JX.install('DraggableList', {
       for (var ii = 0; ii < group.length; ii++) {
         JX.DOM.alterClass(group[ii].getRootNode(), 'drag-target-list', false);
         group[ii]._clearTarget();
-        group[ii]._dirtyTargetCache();
         group[ii]._lastAdjust = null;
       }