From 23e8b27ae9f244cf7ba81d7e8300fb64cc2d55e9 Mon Sep 17 00:00:00 2001 From: Joe Walker Date: Wed, 25 Jun 2014 11:21:07 +0100 Subject: [PATCH] Bug 1028234 - Allow command buttons to use target; r=bgrins --- browser/devtools/eyedropper/commands.js | 4 +- .../responsivedesign/responsivedesign.jsm | 6 ++- browser/devtools/shared/DeveloperToolbar.jsm | 38 +++++++++++++++---- browser/devtools/tilt/tilt.js | 6 ++- .../devtools/webconsole/console-commands.js | 4 +- .../devtools/gcli/commands/paintflashing.js | 10 +++-- 6 files changed, 49 insertions(+), 19 deletions(-) diff --git a/browser/devtools/eyedropper/commands.js b/browser/devtools/eyedropper/commands.js index cafe5fd1182..95ccc478646 100644 --- a/browser/devtools/eyedropper/commands.js +++ b/browser/devtools/eyedropper/commands.js @@ -41,10 +41,10 @@ exports.items = [{ let dropper = EyedropperManager.createInstance(chromeWindow); dropper.open(); - eventEmitter.emit("changed", target.tab); + eventEmitter.emit("changed", { target: target }); dropper.once("destroy", () => { - eventEmitter.emit("changed", target.tab); + eventEmitter.emit("changed", { target: target }); }); } }]; diff --git a/browser/devtools/responsivedesign/responsivedesign.jsm b/browser/devtools/responsivedesign/responsivedesign.jsm index 9a17aa4bd5c..f3b09bdf576 100644 --- a/browser/devtools/responsivedesign/responsivedesign.jsm +++ b/browser/devtools/responsivedesign/responsivedesign.jsm @@ -213,7 +213,8 @@ function ResponsiveUI(aWindow, aTab) this.onPageLoad(); } - ResponsiveUIManager.emit("on", this.tab, this); + // E10S: We should be using target here. See bug 1028234 + ResponsiveUIManager.emit("on", { tab: this.tab }); } ResponsiveUI.prototype = { @@ -305,7 +306,8 @@ ResponsiveUI.prototype = { if (this.touchEventHandler) this.touchEventHandler.stop(); this._telemetry.toolClosed("responsive"); - ResponsiveUIManager.emit("off", this.tab, this); + // E10S: We should be using target here. See bug 1028234 + ResponsiveUIManager.emit("off", { tab: this.tab }); }, /** diff --git a/browser/devtools/shared/DeveloperToolbar.jsm b/browser/devtools/shared/DeveloperToolbar.jsm index 5d2119672d8..41f6752557a 100644 --- a/browser/devtools/shared/DeveloperToolbar.jsm +++ b/browser/devtools/shared/DeveloperToolbar.jsm @@ -125,18 +125,42 @@ let CommandUtils = { // Allow the command button to be toggleable if (command.state) { button.setAttribute("autocheck", false); - let onChange = (event, eventTab) => { - if (eventTab == target.tab) { - if (command.state.isChecked(target)) { - button.setAttribute("checked", true); + + /** + * The onChange event should be called with an event object that + * contains a target property which specifies which target the event + * applies to. For legacy reasons the event object can also contain + * a tab property. + */ + let onChange = (eventName, ev) => { + if (ev.target == target || ev.tab == target.tab) { + + let updateChecked = (checked) => { + if (checked) { + button.setAttribute("checked", true); + } + else if (button.hasAttribute("checked")) { + button.removeAttribute("checked"); + } + }; + + // isChecked would normally be synchronous. An annoying quirk + // of the 'csscoverage toggle' command forces us to accept a + // promise here, but doing Promise.resolve(reply).then(...) here + // makes this async for everyone, which breaks some tests so we + // treat non-promise replies separately to keep then synchronous. + let reply = command.state.isChecked(target); + if (typeof reply.then == "function") { + reply.then(updateChecked, console.error); } - else if (button.hasAttribute("checked")) { - button.removeAttribute("checked"); + else { + updateChecked(reply); } } }; + command.state.onChange(target, onChange); - onChange(null, target.tab); + onChange("", { target: target }); document.defaultView.addEventListener("unload", () => { command.state.offChange(target, onChange); }, false); diff --git a/browser/devtools/tilt/tilt.js b/browser/devtools/tilt/tilt.js index 2e243e024da..cfb5d408d0f 100644 --- a/browser/devtools/tilt/tilt.js +++ b/browser/devtools/tilt/tilt.js @@ -144,7 +144,8 @@ Tilt.prototype = { } this.lastInstanceId = id; - this.emit("change", this.chromeWindow.gBrowser.selectedTab); + // E10S: We should be using target here. See bug 1028234 + this.emit("change", { tab: this.chromeWindow.gBrowser.selectedTab }); Services.obs.notifyObservers(contentWindow, TILT_NOTIFICATIONS.INITIALIZING, null); }, @@ -201,7 +202,8 @@ Tilt.prototype = { this._isDestroying = false; this.chromeWindow.gBrowser.selectedBrowser.focus(); - this.emit("change", this.chromeWindow.gBrowser.selectedTab); + // E10S: We should be using target here. See bug 1028234 + this.emit("change", { tab: this.chromeWindow.gBrowser.selectedTab }); Services.obs.notifyObservers(contentWindow, TILT_NOTIFICATIONS.DESTROYED, null); }, diff --git a/browser/devtools/webconsole/console-commands.js b/browser/devtools/webconsole/console-commands.js index 93632d2eef0..31084964c3a 100644 --- a/browser/devtools/webconsole/console-commands.js +++ b/browser/devtools/webconsole/console-commands.js @@ -16,7 +16,7 @@ gDevTools.on("toolbox-ready", (e, toolbox) => { } let fireChangeForTab = () => { - eventEmitter.emit("changed", toolbox.target.tab); + eventEmitter.emit("changed", { target: toolbox.target }); }; toolbox.on("split-console", fireChangeForTab); @@ -38,7 +38,7 @@ exports.items = [ state: { isChecked: function(target) { let toolbox = gDevTools.getToolbox(target); - return toolbox && toolbox.splitConsole; + return !!(toolbox && toolbox.splitConsole); }, onChange: function(target, changeHandler) { eventEmitter.on("changed", changeHandler); diff --git a/toolkit/devtools/gcli/commands/paintflashing.js b/toolkit/devtools/gcli/commands/paintflashing.js index d1722fdb33b..fadb8dba810 100644 --- a/toolkit/devtools/gcli/commands/paintflashing.js +++ b/toolkit/devtools/gcli/commands/paintflashing.js @@ -17,11 +17,13 @@ const gcli = require("gcli/index"); function onPaintFlashingChanged(context) { let tab = context.environment.chromeWindow.gBrowser.selectedTab; - eventEmitter.emit("changed", tab); - function fireChange() { - eventEmitter.emit("changed", tab); - } let target = TargetFactory.forTab(tab); + + eventEmitter.emit("changed", { target: target }); + function fireChange() { + eventEmitter.emit("changed", { target: target }); + } + target.off("navigate", fireChange); target.once("navigate", fireChange);