Bug 894345 - [New Tab Page] Fix drag/drop behavior when rearranging sites; r=jaws
authorTim Taubert <ttaubert@mozilla.com>
Wed, 24 Jul 2013 09:58:50 +0200
changeset 139775 0a2cc86d61b7
parent 139774 b66185168416
child 139776 105fa6fdce30
push id25003
push userryanvm@gmail.com
push date2013-07-24 22:20 +0000
treeherdermozilla-central@d7f60ad11f48 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs894345
milestone25.0a1
Bug 894345 - [New Tab Page] Fix drag/drop behavior when rearranging sites; r=jaws
browser/base/content/newtab/dropTargetShim.js
browser/base/content/newtab/sites.js
browser/base/content/newtab/transformations.js
--- a/browser/base/content/newtab/dropTargetShim.js
+++ b/browser/base/content/newtab/dropTargetShim.js
@@ -18,59 +18,141 @@ let gDropTargetShim = {
   /**
    * The last drop target that was hovered.
    */
   _lastDropTarget: null,
 
   /**
    * Initializes the drop target shim.
    */
-  init: function DropTargetShim_init() {
-    let node = gGrid.node;
+  init: function () {
+    gGrid.node.addEventListener("dragstart", this, true);
+  },
+
+  /**
+   * Add all event listeners needed during a drag operation.
+   */
+  _addEventListeners: function () {
+    gGrid.node.addEventListener("dragend", this);
 
-    // Add drag event handlers.
-    node.addEventListener("dragstart", this, true);
-    node.addEventListener("dragend", this, true);
+    let docElement = document.documentElement;
+    docElement.addEventListener("dragover", this);
+    docElement.addEventListener("dragenter", this);
+    docElement.addEventListener("drop", this);
+  },
+
+  /**
+   * Remove all event listeners that were needed during a drag operation.
+   */
+  _removeEventListeners: function () {
+    gGrid.node.removeEventListener("dragend", this);
+
+    let docElement = document.documentElement;
+    docElement.removeEventListener("dragover", this);
+    docElement.removeEventListener("dragenter", this);
+    docElement.removeEventListener("drop", this);
   },
 
   /**
    * Handles all shim events.
    */
-  handleEvent: function DropTargetShim_handleEvent(aEvent) {
+  handleEvent: function (aEvent) {
     switch (aEvent.type) {
       case "dragstart":
-        this._start(aEvent);
+        this._dragstart(aEvent);
+        break;
+      case "dragenter":
+        aEvent.preventDefault();
         break;
       case "dragover":
         this._dragover(aEvent);
         break;
+      case "drop":
+        this._drop(aEvent);
+        break;
       case "dragend":
-        this._end(aEvent);
+        this._dragend(aEvent);
         break;
     }
   },
 
   /**
    * Handles the 'dragstart' event.
    * @param aEvent The 'dragstart' event.
    */
-  _start: function DropTargetShim_start(aEvent) {
+  _dragstart: function (aEvent) {
     if (aEvent.target.classList.contains("newtab-link")) {
       gGrid.lock();
+      this._addEventListeners();
+    }
+  },
 
-      // XXX bug 505521 - Listen for dragover on the document.
-      document.documentElement.addEventListener("dragover", this, false);
+  /**
+   * Handles the 'dragover' event.
+   * @param aEvent The 'dragover' event.
+   */
+  _dragover: function (aEvent) {
+    // XXX bug 505521 - Use the dragover event to retrieve the
+    //                  current mouse coordinates while dragging.
+    let sourceNode = aEvent.dataTransfer.mozSourceNode.parentNode;
+    gDrag.drag(sourceNode._newtabSite, aEvent);
+
+    // Find the current drop target, if there's one.
+    this._updateDropTarget(aEvent);
+
+    // If we have a valid drop target,
+    // let the drag-and-drop service know.
+    if (this._lastDropTarget) {
+      aEvent.preventDefault();
     }
   },
 
   /**
-   * Handles the 'drag' event and determines the current drop target.
-   * @param aEvent The 'drag' event.
+   * Handles the 'drop' event.
+   * @param aEvent The 'drop' event.
+   */
+  _drop: function (aEvent) {
+    // We're accepting all drops.
+    aEvent.preventDefault();
+
+    // Make sure to determine the current drop target
+    // in case the dragover event hasn't been fired.
+    this._updateDropTarget(aEvent);
+
+    // A site was successfully dropped.
+    this._dispatchEvent(aEvent, "drop", this._lastDropTarget);
+  },
+
+  /**
+   * Handles the 'dragend' event.
+   * @param aEvent The 'dragend' event.
    */
-  _drag: function DropTargetShim_drag(aEvent) {
+  _dragend: function (aEvent) {
+    if (this._lastDropTarget) {
+      if (aEvent.dataTransfer.mozUserCancelled) {
+        // The drag operation was cancelled.
+        this._dispatchEvent(aEvent, "dragexit", this._lastDropTarget);
+        this._dispatchEvent(aEvent, "dragleave", this._lastDropTarget);
+      }
+
+      // Clean up.
+      this._lastDropTarget = null;
+      this._cellPositions = null;
+    }
+
+    gGrid.unlock();
+    this._removeEventListeners();
+  },
+
+  /**
+   * Tries to find the current drop target and will fire
+   * appropriate dragenter, dragexit, and dragleave events.
+   * @param aEvent The current drag event.
+   */
+  _updateDropTarget: function (aEvent) {
     // Let's see if we find a drop target.
     let target = this._findDropTarget(aEvent);
 
     if (target != this._lastDropTarget) {
       if (this._lastDropTarget)
         // We left the last drop target.
         this._dispatchEvent(aEvent, "dragexit", this._lastDropTarget);
 
@@ -82,63 +164,21 @@ let gDropTargetShim = {
         // We left the last drop target.
         this._dispatchEvent(aEvent, "dragleave", this._lastDropTarget);
 
       this._lastDropTarget = target;
     }
   },
 
   /**
-   * Handles the 'dragover' event as long as bug 505521 isn't fixed to get
-   * current mouse cursor coordinates while dragging.
-   * @param aEvent The 'dragover' event.
-   */
-  _dragover: function DropTargetShim_dragover(aEvent) {
-    let sourceNode = aEvent.dataTransfer.mozSourceNode.parentNode;
-    gDrag.drag(sourceNode._newtabSite, aEvent);
-
-    this._drag(aEvent);
-  },
-
-  /**
-   * Handles the 'dragend' event.
-   * @param aEvent The 'dragend' event.
-   */
-  _end: function DropTargetShim_end(aEvent) {
-    // Make sure to determine the current drop target in case the dragenter
-    // event hasn't been fired.
-    this._drag(aEvent);
-
-    if (this._lastDropTarget) {
-      if (aEvent.dataTransfer.mozUserCancelled) {
-        // The drag operation was cancelled.
-        this._dispatchEvent(aEvent, "dragexit", this._lastDropTarget);
-        this._dispatchEvent(aEvent, "dragleave", this._lastDropTarget);
-      } else {
-        // A site was successfully dropped.
-        this._dispatchEvent(aEvent, "drop", this._lastDropTarget);
-      }
-
-      // Clean up.
-      this._lastDropTarget = null;
-      this._cellPositions = null;
-    }
-
-    gGrid.unlock();
-
-    // XXX bug 505521 - Remove the document's dragover listener.
-    document.documentElement.removeEventListener("dragover", this, false);
-  },
-
-  /**
    * Determines the current drop target by matching the dragged site's position
    * against all cells in the grid.
    * @return The currently hovered drop target or null.
    */
-  _findDropTarget: function DropTargetShim_findDropTarget() {
+  _findDropTarget: function () {
     // These are the minimum intersection values - we want to use the cell if
     // the site is >= 50% hovering its position.
     let minWidth = gDrag.cellWidth / 2;
     let minHeight = gDrag.cellHeight / 2;
 
     let cellPositions = this._getCellPositions();
     let rect = gTransformation.getNodePosition(gDrag.draggedSite.node);
 
@@ -169,20 +209,19 @@ let gDropTargetShim = {
   },
 
   /**
    * Dispatches a custom DragEvent on the given target node.
    * @param aEvent The source event.
    * @param aType The event type.
    * @param aTarget The target node that receives the event.
    */
-  _dispatchEvent:
-    function DropTargetShim_dispatchEvent(aEvent, aType, aTarget) {
-
+  _dispatchEvent: function (aEvent, aType, aTarget) {
     let node = aTarget.node;
     let event = document.createEvent("DragEvents");
 
-    event.initDragEvent(aType, true, true, window, 0, 0, 0, 0, 0, false, false,
+    // The event should not bubble to prevent recursion.
+    event.initDragEvent(aType, false, true, window, 0, 0, 0, 0, 0, false, false,
                         false, false, 0, node, aEvent.dataTransfer);
 
     node.dispatchEvent(event);
   }
 };
--- a/browser/base/content/newtab/sites.js
+++ b/browser/base/content/newtab/sites.js
@@ -190,17 +190,14 @@ Site.prototype = {
         break;
       case "mouseover":
         this._node.removeEventListener("mouseover", this, false);
         this._speculativeConnect();
         break;
       case "dragstart":
         gDrag.start(this, aEvent);
         break;
-      case "drag":
-        gDrag.drag(this, aEvent);
-        break;
       case "dragend":
         gDrag.end(this, aEvent);
         break;
     }
   }
 };
--- a/browser/base/content/newtab/transformations.js
+++ b/browser/base/content/newtab/transformations.js
@@ -151,17 +151,17 @@ let gTransformation = {
     targetPosition.top += this._cellBorderWidths.top;
 
     // Nothing to do here if the positions already match.
     if (currentPosition.left == targetPosition.left &&
         currentPosition.top == targetPosition.top) {
       finish();
     } else {
       this.setSitePosition(aSite, targetPosition);
-      this._whenTransitionEnded(aSite.node, finish);
+      this._whenTransitionEnded(aSite.node, ["left", "top"], finish);
     }
   },
 
   /**
    * Rearranges a given array of sites and moves them to their new positions or
    * fades in/out new/removed sites.
    * @param aSites An array of sites to rearrange.
    * @param aOptions Set of options (see below).
@@ -197,25 +197,29 @@ let gTransformation = {
     let wait = Promise.promised(function () callback && callback());
     wait.apply(null, batch);
   },
 
   /**
    * Listens for the 'transitionend' event on a given node and calls the given
    * callback.
    * @param aNode The node that is transitioned.
+   * @param aProperties The properties we'll wait to be transitioned.
    * @param aCallback The callback to call when finished.
    */
   _whenTransitionEnded:
-    function Transformation_whenTransitionEnded(aNode, aCallback) {
+    function Transformation_whenTransitionEnded(aNode, aProperties, aCallback) {
 
-    aNode.addEventListener("transitionend", function onEnd() {
-      aNode.removeEventListener("transitionend", onEnd, false);
-      aCallback();
-    }, false);
+    let props = new Set(aProperties);
+    aNode.addEventListener("transitionend", function onEnd(e) {
+      if (props.has(e.propertyName)) {
+        aNode.removeEventListener("transitionend", onEnd);
+        aCallback();
+      }
+    });
   },
 
   /**
    * Gets a given node's opacity value.
    * @param aNode The node to get the opacity value from.
    * @return The node's opacity value.
    */
   _getNodeOpacity: function Transformation_getNodeOpacity(aNode) {
@@ -231,18 +235,19 @@ let gTransformation = {
    */
   _setNodeOpacity:
     function Transformation_setNodeOpacity(aNode, aOpacity, aCallback) {
 
     if (this._getNodeOpacity(aNode) == aOpacity) {
       if (aCallback)
         aCallback();
     } else {
-      if (aCallback)
-        this._whenTransitionEnded(aNode, aCallback);
+      if (aCallback) {
+        this._whenTransitionEnded(aNode, ["opacity"], aCallback);
+      }
 
       aNode.style.opacity = aOpacity;
     }
   },
 
   /**
    * Moves a site to the cell with the given index.
    * @param aSite The site to move.