Bug 1160720 - Stop dragging the color selector around if no buttons are pressed down on mousemove. r=bgrins
authorSami Jaktholm <sjakthol@outlook.com>
Wed, 06 May 2015 16:58:20 -0700
changeset 274027 019c10ffbe1607f2618d216b86b028d41bceea74
parent 274026 377431f8f40807b49a7859edd2c94da332e3410e
child 274028 6c8b6e1d328fe23812697eeaae4c7df8076e1b65
child 274080 47643a12ed87b86fced4d3f8c4ebffffe873485e
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)
reviewersbgrins
bugs1160720
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 1160720 - Stop dragging the color selector around if no buttons are pressed down on mousemove. r=bgrins The color picker stops following the mouse when it receives a mouseup event. However, in some cases when the mouse button is released outside the color picker frame, the picker won't know about it and will continue to move the selection when mouse comes back over the picker frame. As the MouseEvent.buttons property specifies which buttons are pressed when the event occurs the picker can use said property to detect if it should stop following the mouse. When .buttons === 0 on a mousemove event no buttons are pressed down and it should stop following the mouse.
browser/devtools/shared/widgets/Spectrum.js
browser/devtools/styleinspector/test/browser.ini
browser/devtools/styleinspector/test/browser_ruleview_colorpicker-release-outside-frame.js
--- a/browser/devtools/shared/widgets/Spectrum.js
+++ b/browser/devtools/shared/widgets/Spectrum.js
@@ -158,16 +158,20 @@ Spectrum.draggable = function(element, o
 
   function prevent(e) {
     e.stopPropagation();
     e.preventDefault();
   }
 
   function move(e) {
     if (dragging) {
+      if (e.buttons === 0) {
+        // The button is no longer pressed but we did not get a mouseup event.
+        return stop();
+      }
       let pageX = e.pageX;
       let pageY = e.pageY;
 
       let dragX = Math.max(0, Math.min(pageX - offset.left, maxWidth));
       let dragY = Math.max(0, Math.min(pageY - offset.top, maxHeight));
 
       onmove.apply(element, [dragX, dragY]);
     }
--- a/browser/devtools/styleinspector/test/browser.ini
+++ b/browser/devtools/styleinspector/test/browser.ini
@@ -57,16 +57,17 @@ support-files =
 [browser_ruleview_add-rule_03.js]
 [browser_ruleview_colorpicker-and-image-tooltip_01.js]
 [browser_ruleview_colorpicker-and-image-tooltip_02.js]
 [browser_ruleview_colorpicker-appears-on-swatch-click.js]
 [browser_ruleview_colorpicker-commit-on-ENTER.js]
 [browser_ruleview_colorpicker-edit-gradient.js]
 [browser_ruleview_colorpicker-hides-on-tooltip.js]
 [browser_ruleview_colorpicker-multiple-changes.js]
+[browser_ruleview_colorpicker-release-outside-frame.js]
 [browser_ruleview_colorpicker-revert-on-ESC.js]
 [browser_ruleview_colorpicker-swatch-displayed.js]
 [browser_ruleview_completion-existing-property_01.js]
 [browser_ruleview_completion-existing-property_02.js]
 [browser_ruleview_completion-new-property_01.js]
 [browser_ruleview_completion-new-property_02.js]
 [browser_ruleview_content_01.js]
 [browser_ruleview_content_02.js]
new file mode 100644
--- /dev/null
+++ b/browser/devtools/styleinspector/test/browser_ruleview_colorpicker-release-outside-frame.js
@@ -0,0 +1,64 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that color pickers stops following the pointer if the pointer is
+// released outside the tooltip frame (bug 1160720).
+
+const PAGE_CONTENT = "data:text/html;charset=utf-8," +
+  '<body style="color: red">Test page for bug 1160720';
+
+add_task(function*() {
+  yield addTab(PAGE_CONTENT);
+  let {toolbox, inspector, view} = yield openRuleView();
+
+  let cSwatch = getRuleViewProperty(view, "element", "color").valueSpan
+    .querySelector(".ruleview-colorswatch");
+
+  let picker = yield openColorPickerForSwatch(cSwatch, view);
+  let spectrum = yield picker.spectrum;
+  let change = spectrum.once("changed");
+
+  info("Pressing mouse down over color picker.");
+  EventUtils.synthesizeMouseAtCenter(spectrum.dragger, {
+    type: "mousedown",
+  }, spectrum.dragger.ownerDocument.defaultView);
+
+  let value = yield change;
+  info(`Color changed to ${value} on mousedown.`);
+
+  // If the mousemove below fails to detect that the button is no longer pressed
+  // the spectrum will update and emit changed event synchronously after calling
+  // synthesizeMouse so this handler is executed before the test ends.
+  spectrum.once("changed", (event, newValue) => {
+    is(newValue, value, "Value changed on mousemove without a button pressed.");
+  });
+
+  // Releasing the button pressed by mousedown above on top of a different frame
+  // does not make sense in this test as EventUtils doesn't preserve the context
+  // i.e. the buttons that were pressed down between events.
+
+  info("Moving mouse over color picker without any buttons pressed.");
+  EventUtils.synthesizeMouse(spectrum.dragger, 10, 10, {
+    button: -1, // -1 = no buttons are pressed down
+    type: "mousemove",
+  }, spectrum.dragger.ownerDocument.defaultView);
+});
+
+function* openColorPickerForSwatch(swatch, view) {
+  let cPicker = view.tooltips.colorPicker;
+  ok(cPicker, "The rule-view has the expected colorPicker property");
+
+  let cPickerPanel = cPicker.tooltip.panel;
+  ok(cPickerPanel, "The XUL panel for the color picker exists");
+
+  let onShown = cPicker.tooltip.once("shown");
+  swatch.click();
+  yield onShown;
+
+  ok(true, "The color picker was shown on click of the color swatch");
+
+  return cPicker;
+}