Bug 1022726 - Focus the devtools window after switching host type. r=bgrins, a=sledru
authorPatrick Brosset <pbrosset@mozilla.com>
Fri, 22 May 2015 16:06:37 +0200
changeset 274762 b08fab71b4f2412baae78228b51a16e37fd1529c
parent 274761 032f91456953f64a276c6b48d87a669bb268b37c
child 274763 b500a8efdc29243aff69335aca91bc3724108210
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, sledru
bugs1022726
milestone40.0a2
Bug 1022726 - Focus the devtools window after switching host type. r=bgrins, a=sledru After switching to a different toolbox host, make sure the toolbox window receives the focus so that the toolbox keyboard shortcuts (like move to next or previous tool) still work.
browser/devtools/debugger/test/browser_dbg_on-pause-raise.js
browser/devtools/framework/test/browser.ini
browser/devtools/framework/test/browser_keybindings.js
browser/devtools/framework/test/browser_keybindings_01.js
browser/devtools/framework/test/browser_keybindings_02.js
browser/devtools/framework/test/browser_toolbox_toggle.js
browser/devtools/framework/toolbox.js
--- a/browser/devtools/debugger/test/browser_dbg_on-pause-raise.js
+++ b/browser/devtools/debugger/test/browser_dbg_on-pause-raise.js
@@ -57,19 +57,20 @@ function onFocus() {
 function testPause() {
   is(gDebugger.gThreadClient.paused, false,
     "Should be running after starting the test.");
 
   is(gFocusedWindow, window,
     "Main window is the top level window before pause.");
 
   if (gToolbox.hostType == devtools.Toolbox.HostType.WINDOW) {
-    gToolbox._host._window.onfocus = () => {
+    gToolbox._host._window.addEventListener("focus", function onFocus() {
+      gToolbox._host._window.removeEventListener("focus", onFocus, true);
       gFocusedWindow = gToolbox._host._window;
-    };
+    }, true);
   }
 
   gDebugger.gThreadClient.addOneTimeListener("paused", () => {
     if (gToolbox.hostType == devtools.Toolbox.HostType.WINDOW) {
       is(gFocusedWindow, gToolbox._host._window,
          "Toolbox window is the top level window on pause.");
     } else {
       is(gBrowser.selectedTab, gTab,
--- a/browser/devtools/framework/test/browser.ini
+++ b/browser/devtools/framework/test/browser.ini
@@ -12,17 +12,18 @@ support-files =
   doc_theme.css
   doc_viewsource.html
   browser_toolbox_options_enable_serviceworkers_testing.html
   serviceworker.js
 
 [browser_devtools_api.js]
 [browser_devtools_api_destroy.js]
 [browser_dynamic_tool_enabling.js]
-[browser_keybindings.js]
+[browser_keybindings_01.js]
+[browser_keybindings_02.js]
 [browser_new_activation_workflow.js]
 [browser_target_events.js]
 [browser_target_remote.js]
 [browser_target_support.js]
 [browser_two_tabs.js]
 [browser_toolbox_dynamic_registration.js]
 [browser_toolbox_getpanelwhenready.js]
 [browser_toolbox_highlight.js]
rename from browser/devtools/framework/test/browser_keybindings.js
rename to browser/devtools/framework/test/browser_keybindings_01.js
new file mode 100644
--- /dev/null
+++ b/browser/devtools/framework/test/browser_keybindings_02.js
@@ -0,0 +1,39 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that the toolbox keybindings still work after the host is changed.
+
+const URL = "data:text/html;charset=utf8,test page";
+
+add_task(function*() {
+  info("Create a test tab and open the toolbox");
+  let tab = yield addTab(URL);
+  let target = TargetFactory.forTab(tab);
+  let toolbox = yield gDevTools.showToolbox(target, "webconsole");
+
+  let {SIDE, BOTTOM} = devtools.Toolbox.HostType;
+  for (let type of [SIDE, BOTTOM, SIDE]) {
+    info("Switch to host type " + type);
+    yield toolbox.switchHost(type);
+
+    info("Try to use the toolbox shortcuts");
+    yield checkKeyBindings(toolbox);
+  }
+
+  Services.prefs.clearUserPref("devtools.toolbox.zoomValue", BOTTOM);
+  Services.prefs.setCharPref("devtools.toolbox.host", BOTTOM);
+  yield toolbox.destroy();
+  gBrowser.removeCurrentTab();
+});
+
+function* checkKeyBindings(toolbox) {
+  let currentZoom = toolbox.zoomValue;
+
+  let key = toolbox.doc.getElementById("toolbox-zoom-in-key").getAttribute("key");
+  EventUtils.synthesizeKey(key, {accelKey: true}, toolbox.doc.defaultView);
+
+  let newZoom = toolbox.zoomValue;
+  isnot(newZoom, currentZoom, "The zoom level was changed in the toolbox");
+}
--- a/browser/devtools/framework/test/browser_toolbox_toggle.js
+++ b/browser/devtools/framework/test/browser_toolbox_toggle.js
@@ -1,86 +1,100 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
-const URL = "data:text/html;charset=utf-8,Test toggling devtools using keyboard shortcuts";
+
+"use strict";
+
+// Test toggling the toolbox with ACCEL+SHIFT+I / ACCEL+ALT+I and F12 in docked
+// and detached (window) modes.
+
+const URL = "data:text/html;charset=utf-8,Toggling devtools using shortcuts";
 
 add_task(function*() {
   // Test with ACCEL+SHIFT+I / ACCEL+ALT+I (MacOSX) ; modifiers should match :
   // - toolbox-key-toggle in browser/devtools/framework/toolbox-window.xul
   // - key_devToolboxMenuItem in browser/base/content/browser.xul
-  info('Test toggle using CTRL+SHIFT+I/CMD+ALT+I');
-  yield testToggle('I', {
-    accelKey : true,
-    shiftKey : !navigator.userAgent.match(/Mac/),
-    altKey : navigator.userAgent.match(/Mac/),
+  info("Test toggle using CTRL+SHIFT+I/CMD+ALT+I");
+  yield testToggle("I", {
+    accelKey: true,
+    shiftKey: !navigator.userAgent.match(/Mac/),
+    altKey: navigator.userAgent.match(/Mac/)
   });
+
   // Test with F12 ; no modifiers
-  info('Test toggle using F12');
-  yield testToggle('VK_F12', {});
+  info("Test toggle using F12");
+  yield testToggle("VK_F12", {});
 });
 
 function* testToggle(key, modifiers) {
   let tab = yield addTab(URL + " ; key : '" + key + "'");
   yield gDevTools.showToolbox(TargetFactory.forTab(tab));
 
   yield testToggleDockedToolbox(tab, key, modifiers);
-
   yield testToggleDetachedToolbox(tab, key, modifiers);
 
   yield cleanup();
 }
 
 function* testToggleDockedToolbox (tab, key, modifiers) {
   let toolbox = getToolboxForTab(tab);
 
-  isnot(toolbox.hostType, devtools.Toolbox.HostType.WINDOW, "Toolbox is docked in the main window");
+  isnot(toolbox.hostType, devtools.Toolbox.HostType.WINDOW,
+    "Toolbox is docked in the main window");
 
-  info('verify docked toolbox is destroyed when using toggle key');
+  info("verify docked toolbox is destroyed when using toggle key");
   let onToolboxDestroyed = once(gDevTools, "toolbox-destroyed");
   EventUtils.synthesizeKey(key, modifiers);
   yield onToolboxDestroyed;
   ok(true, "Docked toolbox is destroyed when using a toggle key");
 
-  info('verify new toolbox is created when using toggle key');
+  info("verify new toolbox is created when using toggle key");
   let onToolboxReady = once(gDevTools, "toolbox-ready");
   EventUtils.synthesizeKey(key, modifiers);
   yield onToolboxReady;
   ok(true, "Toolbox is created by using when toggle key");
 }
 
 function* testToggleDetachedToolbox (tab, key, modifiers) {
   let toolbox = getToolboxForTab(tab);
 
-  info('change the toolbox hostType to WINDOW');
+  info("change the toolbox hostType to WINDOW");
+
   yield toolbox.switchHost(devtools.Toolbox.HostType.WINDOW);
-  is(toolbox.hostType, devtools.Toolbox.HostType.WINDOW, "Toolbox opened on separate window");
+  is(toolbox.hostType, devtools.Toolbox.HostType.WINDOW,
+    "Toolbox opened on separate window");
 
-  let toolboxWindow = toolbox._host._window;
-  info('Wait for focus on the toolbox window')
-  yield new Promise(resolve => waitForFocus(resolve, toolboxWindow));
+  info("Wait for focus on the toolbox window");
+  yield new Promise(res => waitForFocus(res, toolbox.frame.contentWindow));
 
-  info('Focus main window')
+  info("Focus main window to put the toolbox window in the background");
+
   let onMainWindowFocus = once(window, "focus");
   window.focus();
   yield onMainWindowFocus;
   ok(true, "Main window focused");
 
-  info('verify windowed toolbox is focused when using toggle key from the main window')
-  let onToolboxWindowFocus = once(toolboxWindow, "focus");
+  info("Verify windowed toolbox is focused instead of closed when using " +
+    "toggle key from the main window");
+  let toolboxWindow = toolbox._host._window;
+  let onToolboxWindowFocus = once(toolboxWindow, "focus", true);
   EventUtils.synthesizeKey(key, modifiers);
   yield onToolboxWindowFocus;
   ok(true, "Toolbox focused and not destroyed");
 
-  info('verify windowed toolbox is destroyed when using toggle key from its own window')
+  info("Verify windowed toolbox is destroyed when using toggle key from its " +
+    "own window");
+
   let onToolboxDestroyed = once(gDevTools, "toolbox-destroyed");
   EventUtils.synthesizeKey(key, modifiers, toolboxWindow);
   yield onToolboxDestroyed;
   ok(true, "Toolbox destroyed");
 }
 
 function getToolboxForTab(tab) {
   return gDevTools.getToolbox(TargetFactory.forTab(tab));
 }
 
-function* cleanup(toolbox) {
-  Services.prefs.setCharPref("devtools.toolbox.host", devtools.Toolbox.HostType.BOTTOM);
+function* cleanup() {
+  Services.prefs.setCharPref("devtools.toolbox.host",
+    devtools.Toolbox.HostType.BOTTOM);
   gBrowser.removeCurrentTab();
 }
--- a/browser/devtools/framework/toolbox.js
+++ b/browser/devtools/framework/toolbox.js
@@ -1469,45 +1469,54 @@ Toolbox.prototype = {
 
     // clean up the toolbox if its window is closed
     let newHost = new Hosts[hostType](this.target.tab, options);
     newHost.on("window-closed", this.destroy);
     return newHost;
   },
 
   /**
-   * Switch to a new host for the toolbox UI. E.g.
-   * bottom, sidebar, separate window.
+   * 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._host.type || !this._target.isLocalTab) {
       return null;
     }
 
     let newHost = this._createHost(hostType);
     return newHost.create().then(iframe => {
       // change toolbox document's parent to the new host
       iframe.QueryInterface(Ci.nsIFrameLoaderOwner);
       iframe.swapFrameLoaders(this.frame);
 
+      // See bug 1022726, most probably because of swapFrameLoaders we need to
+      // first focus the window here, and then once again further below to make
+      // sure focus actually happens.
+      this.frame.contentWindow.focus();
+
       this._host.off("window-closed", this.destroy);
       this.destroyHost();
 
       this._host = newHost;
 
       if (this.hostType != Toolbox.HostType.CUSTOM) {
         Services.prefs.setCharPref(this._prefs.LAST_HOST, this._host.type);
       }
 
       this._buildDockButtons();
       this._addKeysToWindow();
 
+      // Focus the contentWindow to make sure keyboard shortcuts work straight
+      // away.
+      this.frame.contentWindow.focus();
+
       this.emit("host-changed");
     });
   },
 
   /**
    * Handler for the tool-registered event.
    * @param  {string} event
    *         Name of the event ("tool-registered")