Bug 1295008 - Hide the previous eyedropper when we request to show a new one; r=miker
The eyedropper can be shown in 2 distinct ways:
- the 'eyedropper' gcli command can be called (from the dev toolboar
or from the browser devtools menu),
- or the inspector's 'pickColorFromPage' method can be called (from
the inspector toolbar or from the color-picker in the ruleview).
Before this change, it was possible to show several eyedropper because
these 2 codepaths didn't know about each other.
Now, when executing the gcli command, the inspector's
'cancelPickColorFromPage' method is called to hide it first.
And, when the 'pickColorFromPage' method is called, the 'eyedropper --hide'
gcli command is called too.
This way, there's only one eyedropper shown.
There was also a problem where the gcli command would create a new
eyedropper everytime it was called. This was fixed too, by maintaining a
WeakMap of all eyedroppers opened so far.
MozReview-Commit-ID: F6fBP5R7ZTJ
--- a/devtools/client/inspector/inspector-commands.js
+++ b/devtools/client/inspector/inspector-commands.js
@@ -6,16 +6,18 @@
const l10n = require("gcli/l10n");
const {gDevTools} = require("devtools/client/framework/devtools");
/* eslint-disable mozilla/reject-some-requires */
const {EyeDropper, HighlighterEnvironment} = require("devtools/server/actors/highlighters");
/* eslint-enable mozilla/reject-some-requires */
const Telemetry = require("devtools/client/shared/telemetry");
+const windowEyeDroppers = new WeakMap();
+
exports.items = [{
item: "command",
runAt: "client",
name: "inspect",
description: l10n.lookup("inspectDesc"),
manual: l10n.lookup("inspectManual"),
params: [
{
@@ -43,33 +45,70 @@ exports.items = [{
// This hidden parameter is only set to true when the eyedropper browser menu item is
// used. It is useful to log a different telemetry event whether the tool was used
// from the menu, or from the gcli command line.
group: "hiddengroup",
params: [{
name: "frommenu",
type: "boolean",
hidden: true
+ }, {
+ name: "hide",
+ type: "boolean",
+ hidden: true
}]
}],
- exec: function (args, context) {
+ exec: function* (args, context) {
+ if (args.hide) {
+ context.updateExec("eyedropper_server_hide").catch(e => console.error(e));
+ return;
+ }
+
+ // If the inspector is already picking a color from the page, cancel it.
+ let target = context.environment.target;
+ let toolbox = gDevTools.getToolbox(target);
+ if (toolbox) {
+ let inspector = toolbox.getPanel("inspector");
+ if (inspector) {
+ yield inspector.hideEyeDropper();
+ }
+ }
+
let telemetry = new Telemetry();
telemetry.toolOpened(args.frommenu ? "menueyedropper" : "eyedropper");
context.updateExec("eyedropper_server").catch(e => console.error(e));
}
}, {
item: "command",
runAt: "server",
name: "eyedropper_server",
hidden: true,
exec: function (args, {environment}) {
- let env = new HighlighterEnvironment();
- env.initFromWindow(environment.window);
- let eyeDropper = new EyeDropper(env);
+ let eyeDropper = windowEyeDroppers.get(environment.window);
+
+ if (!eyeDropper) {
+ let env = new HighlighterEnvironment();
+ env.initFromWindow(environment.window);
+
+ eyeDropper = new EyeDropper(env);
+ eyeDropper.once("hidden", () => {
+ eyeDropper.destroy();
+ env.destroy();
+ windowEyeDroppers.delete(environment.window);
+ });
+
+ windowEyeDroppers.set(environment.window, eyeDropper);
+ }
eyeDropper.show(environment.document.documentElement, {copyOnSelect: true});
-
- eyeDropper.once("hidden", () => {
- eyeDropper.destroy();
- env.destroy();
- });
+ }
+}, {
+ item: "command",
+ runAt: "server",
+ name: "eyedropper_server_hide",
+ hidden: true,
+ exec: function (args, {environment}) {
+ let eyeDropper = windowEyeDroppers.get(environment.window);
+ if (eyeDropper) {
+ eyeDropper.hide();
+ }
}
}];
--- a/devtools/client/inspector/inspector-panel.js
+++ b/devtools/client/inspector/inspector-panel.js
@@ -1299,17 +1299,17 @@ InspectorPanel.prototype = {
/**
* Show the eyedropper on the page.
* @return {Promise} resolves when the eyedropper is visible.
*/
showEyeDropper: function () {
this.telemetry.toolOpened("toolbareyedropper");
this.eyeDropperButton.setAttribute("checked", "true");
this.startEyeDropperListeners();
- return this.inspector.pickColorFromPage({copyOnSelect: true})
+ return this.inspector.pickColorFromPage(this.toolbox, {copyOnSelect: true})
.catch(e => console.error(e));
},
/**
* Hide the eyedropper.
* @return {Promise} resolves when the eyedropper is hidden.
*/
hideEyeDropper: function () {
--- a/devtools/client/shared/widgets/Tooltip.js
+++ b/devtools/client/shared/widgets/Tooltip.js
@@ -771,34 +771,34 @@ Heritage.extend(SwatchBasedEditorTooltip
this.commit();
}
}
},
_openEyeDropper: function () {
let {inspector, toolbox, telemetry} = this.inspector;
telemetry.toolOpened("pickereyedropper");
- inspector.pickColorFromPage({copyOnSelect: false}).catch(e => console.error(e));
+ 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");
+ }, e => console.error(e));
inspector.once("color-picked", color => {
toolbox.win.focus();
this._selectColor(color);
this._onEyeDropperDone();
});
inspector.once("color-pick-canceled", () => {
this._onEyeDropperDone();
});
-
- this.eyedropperOpen = true;
-
- // close the colorpicker tooltip so that only the eyedropper is open.
- this.hide();
-
- this.tooltip.emit("eyedropper-opened");
},
_onEyeDropperDone: function () {
this.eyedropperOpen = false;
this.activeSwatch = null;
},
_colorToRgba: function (color) {
--- a/devtools/server/actors/highlighters/eye-dropper.js
+++ b/devtools/server/actors/highlighters/eye-dropper.js
@@ -173,16 +173,18 @@ EyeDropper.prototype = {
pageListenerTarget.removeEventListener("mousemove", this);
pageListenerTarget.removeEventListener("click", this);
pageListenerTarget.removeEventListener("keydown", this);
pageListenerTarget.removeEventListener("DOMMouseScroll", this);
pageListenerTarget.removeEventListener("FullZoomChange", this);
this.getElement("root").setAttribute("hidden", "true");
this.getElement("root").removeAttribute("drawn");
+
+ this.emit("hidden");
},
prepareImageCapture() {
// Get the image data from the content window.
let imageData = getWindowAsImageData(this.win);
// We need to transform imageData to something drawWindow will consume. An ImageBitmap
// works well. We could have used an Image, but doing so results in errors if the page
--- a/devtools/shared/fronts/inspector.js
+++ b/devtools/shared/fronts/inspector.js
@@ -21,16 +21,18 @@ const {
} = require("devtools/shared/specs/inspector");
const promise = require("promise");
const defer = require("devtools/shared/defer");
const { Task } = require("devtools/shared/task");
const { Class } = require("sdk/core/heritage");
const events = require("sdk/event/core");
const object = require("sdk/util/object");
const nodeConstants = require("devtools/shared/dom-node-constants.js");
+loader.lazyRequireGetter(this, "CommandUtils",
+ "devtools/client/shared/developer-toolbar", true);
const HIDDEN_CLASS = "__fx-devtools-hide-shortcut__";
/**
* Convenience API for building a list of attribute modifications
* for the `modifyAttributes` request.
*/
const AttributeModificationList = Class({
@@ -972,12 +974,28 @@ var InspectorFront = FrontClass(inspecto
return pageStyle;
}
return this.getWalker().then(() => {
return pageStyle;
});
});
}, {
impl: "_getPageStyle"
+ }),
+
+ pickColorFromPage: custom(Task.async(function* (toolbox, options) {
+ if (toolbox) {
+ // If the eyedropper was already started using the gcli command, hide it so we don't
+ // end up with 2 instances of the eyedropper on the page.
+ let {target} = toolbox;
+ let requisition = yield CommandUtils.createRequisition(target, {
+ environment: CommandUtils.createEnvironment({target})
+ });
+ yield requisition.updateExec("eyedropper --hide");
+ }
+
+ yield this._pickColorFromPage(options);
+ }), {
+ impl: "_pickColorFromPage"
})
});
exports.InspectorFront = InspectorFront;