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
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.