Bug 734664 - Devtools toolbox should display the actual target url when detached. r=paul
authorDavid Creswick <dcrewi@gyrae.net>
Tue, 26 Feb 2013 12:40:19 +0000
changeset 122831 6092bfb91be4428874f3fca39132b6c77d9c68a2
parent 122830 0282296facc580d7b9dd98e2b2572433557c6ad0
child 122832 ed93e2b6cfd06c8a2e611037bf4addcd92f8e5fb
push id1384
push usermratcliffe@mozilla.com
push dateTue, 26 Feb 2013 12:40:56 +0000
treeherderfx-team@a5c189768303 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaul
bugs734664
milestone22.0a1
Bug 734664 - Devtools toolbox should display the actual target url when detached. r=paul
browser/devtools/framework/Toolbox.jsm
browser/devtools/framework/ToolboxHosts.jsm
browser/devtools/framework/test/Makefile.in
browser/devtools/framework/test/browser_toolbox_window_title_changes.js
browser/devtools/framework/toolbox-window.xul
browser/devtools/profiler/test/head.js
browser/locales/en-US/chrome/browser/devtools/toolbox.dtd
browser/locales/en-US/chrome/browser/devtools/toolbox.properties
--- a/browser/devtools/framework/Toolbox.jsm
+++ b/browser/devtools/framework/Toolbox.jsm
@@ -14,21 +14,25 @@ Cu.import("resource:///modules/devtools/
 
 XPCOMUtils.defineLazyModuleGetter(this, "Hosts",
                                   "resource:///modules/devtools/ToolboxHosts.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "CommandUtils",
                                   "resource:///modules/devtools/DeveloperToolbar.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "toolboxStrings", function() {
   let bundle = Services.strings.createBundle("chrome://browser/locale/devtools/toolbox.properties");
-  let l10n = function(name) {
+  let l10n = function(aName, ...aArgs) {
     try {
-      return bundle.GetStringFromName(name);
+      if (aArgs.length == 0) {
+        return bundle.GetStringFromName(aName);
+      } else {
+        return bundle.formatStringFromName(aName, aArgs, aArgs.length);
+      }
     } catch (ex) {
-      Services.console.logStringMessage("Error reading '" + name + "'");
+      Services.console.logStringMessage("Error reading '" + aName + "'");
     }
   };
   return l10n;
 });
 
 XPCOMUtils.defineLazyGetter(this, "Requisition", function() {
   Cu.import("resource://gre/modules/devtools/Require.jsm");
   Cu.import("resource:///modules/devtools/gcli.jsm");
@@ -135,16 +139,21 @@ this.Toolbox = function Toolbox(target, 
     selectedTool = "webconsole";
   }
   this._defaultToolId = selectedTool;
 
   this._host = this._createHost(hostType);
 
   EventEmitter.decorate(this);
 
+  this._refreshHostTitle = this._refreshHostTitle.bind(this);
+  this._target.on("navigate", this._refreshHostTitle);
+  this.on("host-changed", this._refreshHostTitle);
+  this.on("select", this._refreshHostTitle);
+
   gDevTools.on("tool-registered", this._toolRegistered);
   gDevTools.on("tool-unregistered", this._toolUnregistered);
 }
 
 /**
  * The toolbox can be 'hosted' either embedded in a browser window
  * or in a separate window.
  */
@@ -504,16 +513,34 @@ Toolbox.prototype = {
   /**
    * Raise the toolbox host.
    */
   raise: function TBOX_raise() {
     this._host.raise();
   },
 
   /**
+   * Refresh the host's title.
+   */
+  _refreshHostTitle: function TBOX_refreshHostTitle() {
+    let toolName;
+    let toolId = this.currentToolId;
+    if (toolId) {
+      let toolDef = gDevTools.getToolDefinitionMap().get(toolId);
+      toolName = toolDef.label;
+    } else {
+      // no tool is selected
+      toolName = toolboxStrings("toolbox.defaultTitle");
+    }
+    let title = toolboxStrings("toolbox.titleTemplate",
+                               toolName, this.target.url);
+    this._host.setTitle(title);
+  },
+
+  /**
    * Create a host object based on the given host type.
    *
    * Warning: some hosts require that the toolbox target provides a reference to
    * the attached tab. Not all Targets have a tab property - make sure you correctly
    * mix and match hosts and targets.
    *
    * @param {string} hostType
    *        The host type of the new host object
@@ -643,16 +670,25 @@ Toolbox.prototype = {
    * Remove all UI elements, detach from target and clear up
    */
   destroy: function TBOX_destroy() {
     // If several things call destroy then we give them all the same
     // destruction promise so we're sure to destroy only once
     if (this._destroyer) {
       return this._destroyer;
     }
+    // Assign the "_destroyer" property before calling the other
+    // destroyer methods to guarantee that the Toolbox's destroy
+    // method is only executed once.
+    let deferred = Promise.defer();
+    this._destroyer = deferred.promise;
+
+    this._target.off("navigate", this._refreshHostTitle);
+    this.off("select", this._refreshHostTitle);
+    this.off("host-changed", this._refreshHostTitle);
 
     let outstanding = [];
 
     // Remote targets need to be notified that the toolbox is being torn down.
     if (this._target && this._target.isRemote) {
       outstanding.push(this._target.destroy());
     }
     this._target = null;
@@ -661,20 +697,16 @@ Toolbox.prototype = {
       outstanding.push(panel.destroy());
     }
 
     outstanding.push(this._host.destroy());
 
     gDevTools.off("tool-registered", this._toolRegistered);
     gDevTools.off("tool-unregistered", this._toolUnregistered);
 
-    this._destroyer = Promise.all(outstanding);
-    this._destroyer.then(function() {
+    Promise.all(outstanding).then(function() {
       this.emit("destroyed");
+      deferred.resolve();
     }.bind(this));
 
-    return this._destroyer.then(function() {
-      // Ensure that the promise resolves to nothing, rather than an array of
-      // several nothings, which is what we get from Promise.all
-      return undefined;
-    });
+    return this._destroyer;
   }
 };
--- a/browser/devtools/framework/ToolboxHosts.jsm
+++ b/browser/devtools/framework/ToolboxHosts.jsm
@@ -81,16 +81,23 @@ BottomHost.prototype = {
   /**
    * Raise the host.
    */
   raise: function BH_raise() {
     focusTab(this.hostTab);
   },
 
   /**
+   * Set the toolbox title.
+   */
+  setTitle: function BH_setTitle(title) {
+    // Nothing to do for this host type.
+  },
+
+  /**
    * Destroy the bottom dock.
    */
   destroy: function BH_destroy() {
     if (!this._destroyed) {
       this._destroyed = true;
 
       Services.prefs.setIntPref(this.heightPref, this.frame.height);
       this._nbox.removeChild(this._splitter);
@@ -154,16 +161,23 @@ SidebarHost.prototype = {
   /**
    * Raise the host.
    */
   raise: function SH_raise() {
     focusTab(this.hostTab);
   },
 
   /**
+   * Set the toolbox title.
+   */
+  setTitle: function SH_setTitle(title) {
+    // Nothing to do for this host type.
+  },
+
+  /**
    * Destroy the sidebar.
    */
   destroy: function SH_destroy() {
     if (!this._destroyed) {
       this._destroyed = true;
 
       Services.prefs.setIntPref(this.widthPref, this.frame.width);
       this._sidebar.removeChild(this._splitter);
@@ -231,16 +245,23 @@ WindowHost.prototype = {
   /**
    * Raise the host.
    */
   raise: function RH_raise() {
     this._window.focus();
   },
 
   /**
+   * Set the toolbox title.
+   */
+  setTitle: function WH_setTitle(title) {
+    this._window.document.title = title;
+  },
+
+  /**
    * Destroy the window.
    */
   destroy: function WH_destroy() {
     if (!this._destroyed) {
       this._destroyed = true;
 
       this._window.removeEventListener("unload", this._boundUnload);
       this._window.close();
--- a/browser/devtools/framework/test/Makefile.in
+++ b/browser/devtools/framework/test/Makefile.in
@@ -17,11 +17,12 @@ MOCHITEST_BROWSER_FILES = \
 		browser_toolbox_dynamic_registration.js \
 		browser_toolbox_hosts.js \
 		browser_toolbox_ready.js \
 		browser_toolbox_select_event.js \
 		browser_target_events.js \
 		browser_toolbox_tool_ready.js \
 		browser_toolbox_sidebar.js \
 		browser_toolbox_window_shortcuts.js \
+		browser_toolbox_window_title_changes.js \
 		$(NULL)
 
 include $(topsrcdir)/config/rules.mk
new file mode 100644
--- /dev/null
+++ b/browser/devtools/framework/test/browser_toolbox_window_title_changes.js
@@ -0,0 +1,83 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+let temp = {};
+Cu.import("resource:///modules/devtools/Toolbox.jsm", temp);
+let Toolbox = temp.Toolbox;
+temp = {};
+Cu.import("resource:///modules/devtools/Target.jsm", temp);
+let TargetFactory = temp.TargetFactory;
+temp = {};
+Cu.import("resource:///modules/devtools/gDevTools.jsm", temp);
+let gDevTools = temp.gDevTools;
+temp = {};
+Cu.import("resource://gre/modules/Services.jsm", temp);
+let Services = temp.Services;
+temp = null;
+
+function test() {
+  waitForExplicitFinish();
+
+  const URL_1 = "data:text/plain;charset=UTF-8,abcde";
+  const URL_2 = "data:text/plain;charset=UTF-8,12345";
+
+  const TOOL_ID_1 = "webconsole";
+  const TOOL_ID_2 = "jsdebugger";
+
+  const LABEL_1 = "Web Console";
+  const LABEL_2 = "Debugger";
+
+  let toolbox;
+
+  addTab(URL_1, function () {
+    let target = TargetFactory.forTab(gBrowser.selectedTab);
+    gDevTools.showToolbox(target, null, Toolbox.HostType.BOTTOM)
+      .then(function (aToolbox) { toolbox = aToolbox; })
+      .then(function () toolbox.selectTool(TOOL_ID_1))
+
+    // undock toolbox and check title
+      .then(function () toolbox.switchHost(Toolbox.HostType.WINDOW))
+      .then(checkTitle.bind(null, LABEL_1, URL_1, "toolbox undocked"))
+
+    // switch to different tool and check title
+      .then(function () toolbox.selectTool(TOOL_ID_2))
+      .then(checkTitle.bind(null, LABEL_2, URL_1, "tool changed"))
+
+    // navigate to different url and check title
+      .then(function () {
+        let deferred = Promise.defer();
+        target.once("navigate", function () deferred.resolve());
+        gBrowser.loadURI(URL_2);
+        return deferred.promise;
+      })
+      .then(checkTitle.bind(null, LABEL_2, URL_2, "url changed"))
+
+    // destroy toolbox, create new one hosted in a window (with a
+    // different tool id), and check title
+      .then(function () toolbox.destroy())
+      .then(function () gDevTools.showToolbox(target, null,
+                                              Toolbox.HostType.WINDOW))
+      .then(function (aToolbox) { toolbox = aToolbox; })
+      .then(function () toolbox.selectTool(TOOL_ID_1))
+      .then(checkTitle.bind(null, LABEL_1, URL_2,
+                            "toolbox destroyed and recreated"))
+
+    // clean up
+      .then(function () toolbox.destroy())
+      .then(function () {
+        toolbox = null;
+        gBrowser.removeCurrentTab();
+        Services.prefs.clearUserPref("devtools.toolbox.host");
+        Services.prefs.clearUserPref("devtools.toolbox.selectedTool");
+        Services.prefs.clearUserPref("devtools.toolbox.sideEnabled");
+        finish();
+      });
+  });
+}
+
+function checkTitle(toolLabel, url, context) {
+  let win = Services.wm.getMostRecentWindow("devtools:toolbox");
+  let definitions = gDevTools.getToolDefinitionMap();
+  let expectedTitle = toolLabel + " - " + url;
+  is(win.document.title, expectedTitle, context);
+}
--- a/browser/devtools/framework/toolbox-window.xul
+++ b/browser/devtools/framework/toolbox-window.xul
@@ -6,17 +6,16 @@
 <!ENTITY % toolboxDTD SYSTEM "chrome://browser/locale/devtools/toolbox.dtd" >
  %toolboxDTD;
 ]>
 
 <?xml-stylesheet href="chrome://browser/skin/" type="text/css"?>
 
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         id="devtools-toolbox-window"
-        title="&window.title;"
         macanimationtype="document"
         fullscreenbutton="true"
         windowtype="devtools:toolbox"
         width="900" height="320"
         persist="screenX screenY width height sizemode">
 
   <commandset id="toolbox-commandset">
     <command id="toolbox-cmd-close" oncommand="window.close();"/>
--- a/browser/devtools/profiler/test/head.js
+++ b/browser/devtools/profiler/test/head.js
@@ -49,20 +49,18 @@ function loadTab(url, callback) {
 
 function openProfiler(tab, callback) {
   let target = TargetFactory.forTab(tab);
   gDevTools.showToolbox(target, "jsprofiler").then(callback);
 }
 
 function closeProfiler(tab, callback) {
   let target = TargetFactory.forTab(tab);
-  let panel = gDevTools.getToolbox(target).getPanel("jsprofiler");
-  panel.once("destroyed", callback);
-
-  gDevTools.closeToolbox(target);
+  let toolbox = gDevTools.getToolbox(target);
+  toolbox.destroy().then(callback);
 }
 
 function setUp(url, callback=function(){}) {
   Services.prefs.setBoolPref(PROFILER_ENABLED, true);
 
   loadTab(url, function onTabLoad(tab, browser) {
     openProfiler(tab, function onProfilerOpen() {
       let target = TargetFactory.forTab(tab);
--- a/browser/locales/en-US/chrome/browser/devtools/toolbox.dtd
+++ b/browser/locales/en-US/chrome/browser/devtools/toolbox.dtd
@@ -1,9 +1,7 @@
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
-<!ENTITY window.title  "Developer Tools">
-
 <!ENTITY closeCmd.key  "W">
 
-<!ENTITY toolboxCloseButton.tooltip  "Close Developer Tools">
\ No newline at end of file
+<!ENTITY toolboxCloseButton.tooltip  "Close Developer Tools">
--- a/browser/locales/en-US/chrome/browser/devtools/toolbox.properties
+++ b/browser/locales/en-US/chrome/browser/devtools/toolbox.properties
@@ -15,8 +15,18 @@ toolboxToggleButton.errors=#1 error;#1 e
 toolboxToggleButton.warnings=#1 warning;#1 warnings
 
 # LOCALIZATION NOTE (toolboxToggleButton.tooltip): This string is shown
 # as tooltip in the developer toolbar to open/close the developer tools.
 # It's using toolboxToggleButton.errors as first and
 # toolboxToggleButton.warnings as second argument to show the number of errors
 # and warnings.
 toolboxToggleButton.tooltip=%1$S, %2$S\nClick to toggle the developer tools.
+
+# LOCALIZATION NOTE (toolbox.titleTemplate): This is the template
+# used to format the title of the toolbox.
+# The name of the selected tool: %1$S.
+# The url of the page being tooled: %2$S.
+toolbox.titleTemplate=%1$S - %2$S
+
+# LOCALIZATION NOTE (toolbox.defaultTitle): This is used as the tool
+# name when no tool is selected.
+toolbox.defaultTitle=Developer Tools