Bug 1144779 - Do not use mouse coordinates from onMouseUp to set selection end when dragging in Graph;r=vporof
authorBrian Grinstead <bgrinstead@mozilla.com>
Thu, 07 May 2015 13:19:46 -0700
changeset 274311 056ceb0ad5eabb4c40d7b8c7d4adebde7982f316
parent 274310 0e5e9c5ac6f360e9606509d88ed863e1332e6e0c
child 274312 e8c071496de1185a4b283f5615ead356ac8b8bb2
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvporof
bugs1144779
milestone40.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 1144779 - Do not use mouse coordinates from onMouseUp to set selection end when dragging in Graph;r=vporof Instead use the cursor.x property that was stored during the mousemove event. This helps preserve the selection when a mouseup happens outside of the window.
browser/devtools/shared/test/browser_graphs-07a.js
browser/devtools/shared/widgets/Graphs.jsm
--- a/browser/devtools/shared/test/browser_graphs-07a.js
+++ b/browser/devtools/shared/test/browser_graphs-07a.js
@@ -196,21 +196,23 @@ function normalDragStop(graph, x, y = 1)
   graph._onMouseMove({ testX: x, testY: y });
   graph._onMouseUp({ testX: x, testY: y });
 }
 
 function buggyDragStop(graph, x, y = 1) {
   x /= window.devicePixelRatio;
   y /= window.devicePixelRatio;
 
-  // Only fire a mousemove instead of a mouseup.
-  // This happens when the mouseup happens outside of the toolbox,
-  // see Bug 1066504.
   graph._onMouseMove({ testX: x, testY: y });
-  graph._onMouseMove({ testX: x, testY: y, buttons: 0 });
+
+  // Only fire a mousemove with no buttons instead of a mouseup.
+  // This happens when the mouseup happens outside of the window.
+  // Send different coordinates to make sure the selection is preserved,
+  // see Bugs 1066504 and 1144779.
+  graph._onMouseMove({ testX: x+1, testY: y+1, buttons: 0 });
 }
 
 function scroll(graph, wheel, x, y = 1) {
   x /= window.devicePixelRatio;
   y /= window.devicePixelRatio;
   graph._onMouseMove({ testX: x, testY: y });
   graph._onMouseWheel({ testX: x, testY: y, detail: wheel });
 }
--- a/browser/devtools/shared/widgets/Graphs.jsm
+++ b/browser/devtools/shared/widgets/Graphs.jsm
@@ -981,22 +981,22 @@ AbstractCanvasGraph.prototype = {
     // Need to stop propagation here, since this function can be bound
     // to both this._window and this._topWindow.  It's only attached to
     // this._topWindow during a drag event.  Null check here since tests
     // don't pass this method into the event object.
     if (e.stopPropagation && this._isMouseActive) {
       e.stopPropagation();
     }
 
-    // If a mouseup happened outside the toolbox and the current operation
-    // is causing the selection changed, then end it.
+    // If a mouseup happened outside the window and the current operation
+    // is causing the selection to change, then end it.
     if (e.buttons == 0 && (this.hasSelectionInProgress() ||
                            resizer.margin != null ||
                            dragger.origin != null)) {
-      return this._onMouseUp(e);
+      return this._onMouseUp();
     }
 
     let {mouseX,mouseY} = this._getRelativeEventCoordinates(e);
     this._cursor.x = mouseX;
     this._cursor.y = mouseY;
 
     if (resizer.margin != null) {
       this._selection[resizer.margin] = mouseX;
@@ -1088,20 +1088,18 @@ AbstractCanvasGraph.prototype = {
 
     this._shouldRedraw = true;
     this.emit("mousedown");
   },
 
   /**
    * Listener for the "mouseup" event on the graph's container.
    */
-  _onMouseUp: function(e) {
+  _onMouseUp: function() {
     this._isMouseActive = false;
-    let {mouseX} = this._getRelativeEventCoordinates(e);
-
     switch (this._canvas.getAttribute("input")) {
       case "hovering-background":
       case "hovering-region":
         if (!this.selectionEnabled) {
           break;
         }
         if (this.getSelectionWidth() < 1) {
           let region = this.getHoveredRegion();
@@ -1110,17 +1108,17 @@ AbstractCanvasGraph.prototype = {
             this._selection.end = region.end;
             this.emit("selecting");
           } else {
             this._selection.start = null;
             this._selection.end = null;
             this.emit("deselecting");
           }
         } else {
-          this._selection.end = mouseX;
+          this._selection.end = this._cursor.x;
           this.emit("selecting");
         }
         break;
 
       case "hovering-selection-start-boundary":
       case "hovering-selection-end-boundary":
         this._selectionResizer.margin = null;
         break;