Bug 1349483 - fix colorpicker eyedropper when devtools are in a window host;r=pbro draft
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 09 Jan 2018 19:30:11 +0100
changeset 717866 17eae1fea9b6552d67907d09a04d8c3eefb01aef
parent 717864 d6bddd7d65d6c384b5a5c66d7b646110ca90c8c6
child 745368 7fcc5a6b978b1b4a3fcc63e8ad77ec347a20a378
push id94801
push userjdescottes@mozilla.com
push dateTue, 09 Jan 2018 18:40:02 +0000
reviewerspbro
bugs1349483
milestone59.0a1
Bug 1349483 - fix colorpicker eyedropper when devtools are in a window host;r=pbro MozReview-Commit-ID: CwfZaIsGoBW
devtools/client/inspector/rules/test/browser_rules_eyedropper.js
devtools/client/shared/widgets/tooltip/SwatchColorPickerTooltip.js
--- a/devtools/client/inspector/rules/test/browser_rules_eyedropper.js
+++ b/devtools/client/inspector/rules/test/browser_rules_eyedropper.js
@@ -29,20 +29,41 @@ const TEST_URI = `
   <body><div id="div1"></div><div id="div2"></div></body>
 `;
 
 // #f09
 const ORIGINAL_COLOR = "rgb(255, 0, 153)";
 // #ff5
 const EXPECTED_COLOR = "rgb(255, 255, 85)";
 
+registerCleanupFunction(() => {
+  // Restore the default Toolbox host position after the test.
+  Services.prefs.clearUserPref("devtools.toolbox.host");
+});
+
 add_task(function* () {
   info("Add the test tab, open the rule-view and select the test node");
-  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
-  let {testActor, inspector, view} = yield openRuleView();
+
+  let url = "data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI);
+  yield addTab(url);
+
+  let {testActor, inspector, view, toolbox} = yield openRuleView();
+
+  yield runTest(testActor, inspector, view);
+
+  info("Reload the page to restore the initial state");
+  yield navigateTo(inspector, url);
+
+  info("Change toolbox host to WINDOW");
+  yield toolbox.switchHost("window");
+
+  yield runTest(testActor, inspector, view);
+});
+
+function* runTest(testActor, inspector, view) {
   yield selectNode("#div2", inspector);
 
   info("Get the background-color property from the rule-view");
   let property = getRuleViewProperty(view, "#div2", "background-color");
   let swatch = property.valueSpan.querySelector(".ruleview-colorswatch");
   ok(swatch, "Color swatch is displayed for the bg-color property");
 
   info("Open the eyedropper from the colorpicker tooltip");
@@ -61,17 +82,17 @@ add_task(function* () {
   yield testSelect(view, swatch, inspector, testActor);
 
   let onHidden = tooltip.once("hidden");
   tooltip.hide();
   yield onHidden;
   ok(!tooltip.isVisible(), "color picker tooltip is closed");
 
   yield waitForTick();
-});
+}
 
 function* testESC(swatch, inspector, testActor) {
   info("Press escape");
   let onCanceled = new Promise(resolve => {
     inspector.inspector.once("color-pick-canceled", resolve);
   });
   yield testActor.synthesizeKey({key: "VK_ESCAPE", options: {}});
   yield onCanceled;
--- a/devtools/client/shared/widgets/tooltip/SwatchColorPickerTooltip.js
+++ b/devtools/client/shared/widgets/tooltip/SwatchColorPickerTooltip.js
@@ -172,19 +172,22 @@ class SwatchColorPickerTooltip extends S
 
   _openEyeDropper() {
     let {inspector, toolbox, telemetry} = this.inspector;
     telemetry.toolOpened("pickereyedropper");
 
     // cancelling picker(if it is already selected) on opening eye-dropper
     toolbox.highlighterUtils.cancelPicker();
 
+    // pickColorFromPage will focus the content document. If the devtools are in a
+    // separate window, the colorpicker tooltip will be closed before pickColorFromPage
+    // resolves. Flip the flag early to avoid issues with onTooltipHidden().
+    this.eyedropperOpen = true;
+
     inspector.pickColorFromPage(toolbox, {copyOnSelect: false}).then(() => {
-      this.eyedropperOpen = true;
-
       // close the colorpicker tooltip so that only the eyedropper is open.
       this.hide();
 
       this.tooltip.emit("eyedropper-opened");
     }, console.error);
 
     inspector.once("color-picked", color => {
       toolbox.win.focus();