Bug 1266134 - Cleanup focus workarounds r=jryans
MozReview-Commit-ID: 4Em2QXwanta
--- a/devtools/client/framework/toolbox-host-manager.js
+++ b/devtools/client/framework/toolbox-host-manager.js
@@ -12,19 +12,18 @@ loader.lazyRequireGetter(this, "Hosts",
* Implement a wrapper on the chrome side to setup a Toolbox within Firefox UI.
*
* This component handles iframe creation within Firefox, in which we are loading
* the toolbox document. Then both the chrome and the toolbox document communicate
* via "message" events.
*
* Messages sent by the toolbox to the chrome:
* - switch-host:
- * Order to display the toolbox in another host (side, bottom or window)
- * - switch-to-previous-host:
- * Order to display the toolbox in the previously used host
+ * Order to display the toolbox in another host (side, bottom, window, or the
+ * previously used one)
* - toggle-minimize-mode:
* When using the bottom host, the toolbox can be miximized to only display
* the tool titles
* - maximize-host:
* When using the bottom host in minimized mode, revert back to regular mode
* in order to see tool titles and the tools
* - raise-host:
* Focus the tools
@@ -110,19 +109,16 @@ ToolboxHostManager.prototype = {
// origin via event.source as it is null. So use a custom id.
if (event.data.frameId != this.frameId) {
return;
}
switch (event.data.name) {
case "switch-host":
this.switchHost(event.data.hostType);
break;
- case "switch-to-previous-host":
- this.switchToPreviousHost();
- break;
case "maximize-host":
this.host.maximize();
break;
case "raise-host":
this.host.raise();
break;
case "toggle-minimize-mode":
this.host.toggleMinimizeMode(event.data.toolbarHeight);
@@ -177,48 +173,38 @@ ToolboxHostManager.prototype = {
},
onHostMaximized() {
this.postMessage({
name: "host-maximized"
});
},
- /**
- * Switch to the last used host for the toolbox UI.
- * This is determined by the devtools.toolbox.previousHost pref.
- */
- switchToPreviousHost() {
- let hostType = Services.prefs.getCharPref(PREVIOUS_HOST);
+ switchHost: Task.async(function* (hostType) {
+ if (hostType == "previous") {
+ // Switch to the last used host for the toolbox UI.
+ // This is determined by the devtools.toolbox.previousHost pref.
+ hostType = Services.prefs.getCharPref(PREVIOUS_HOST);
- // Handle the case where the previous host happens to match the current
- // host. If so, switch to bottom if it's not already used, and side if not.
- if (hostType === this.hostType) {
- if (hostType === Toolbox.HostType.BOTTOM) {
- hostType = Toolbox.HostType.SIDE;
- } else {
- hostType = Toolbox.HostType.BOTTOM;
+ // Handle the case where the previous host happens to match the current
+ // host. If so, switch to bottom if it's not already used, and side if not.
+ if (hostType === this.hostType) {
+ if (hostType === Toolbox.HostType.BOTTOM) {
+ hostType = Toolbox.HostType.SIDE;
+ } else {
+ hostType = Toolbox.HostType.BOTTOM;
+ }
}
}
-
- return this.switchHost(hostType);
- },
-
- switchHost: Task.async(function* (hostType) {
let iframe = this.host.frame;
let newHost = this.createHost(hostType);
let newIframe = yield newHost.create();
// change toolbox document's parent to the new host
newIframe.swapFrameLoaders(iframe);
- // See bug 1022726, most probably because of swapFrameLoaders we need to
- // first focus the window here, and then once again further from
- // toolbox.js to make sure focus actually happens.
- iframe.contentWindow.focus();
-
this.destroyHost();
if (this.hostType != Toolbox.HostType.CUSTOM) {
Services.prefs.setCharPref(PREVIOUS_HOST, this.hostType);
}
this.host = newHost;
this.hostType = hostType;
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -1847,40 +1847,36 @@ Toolbox.prototype = {
button.setAttribute("checked", "true");
}
},
/**
* Switch to the last used host for the toolbox UI.
*/
switchToPreviousHost: function () {
- this.postMessage({
- name: "switch-to-previous-host"
- });
- return this.once("host-changed");
+ return this.switchHost("previous");
},
/**
* Switch to a new host for the toolbox UI. E.g. bottom, sidebar, window,
* and focus the window when done.
*
* @param {string} hostType
* The host type of the new host object
*/
switchHost: function (hostType) {
if (hostType == this.hostType || !this._target.isLocalTab) {
return null;
}
this.emit("host-will-change", hostType);
- // If we call swapFrameLoaders() when a tool if focused it leaves the
- // browser in a state where it thinks that the tool is focused but in
- // reality the content area is focused. Blurring the tool before calling
- // swapFrameLoaders() works around this issue.
+ // ToolboxHostManager is going to call swapFrameLoaders which mess up with
+ // focus. We have to blur before calling it in order to be able to restore
+ // the focus after, in _onSwitchedHost.
this.focusTool(this.currentToolId, false);
// Host code on the chrome side will send back a message once the host
// switched
this.postMessage({
name: "switch-host",
hostType
});
@@ -1889,17 +1885,18 @@ Toolbox.prototype = {
},
_onSwitchedHost: function ({ hostType }) {
this._hostType = hostType;
this._buildDockButtons();
this._addKeysToWindow();
- // Focus the tool to make sure keyboard shortcuts work straight away.
+ // We blurred the tools at start of switchHost, but also when clicking on
+ // host switching button. We now have to restore the focus.
this.focusTool(this.currentToolId, true);
this.emit("host-changed");
this._telemetry.log(HOST_HISTOGRAM, this._getTelemetryHostId());
},
/**
* Return if the tool is available as a tab (i.e. if it's checked