Bug 719607 - The HTML Tree Panel should NOT use the registerTools mechanism. r=rcampbell

browser/app/profile/firefox.js

@@ -1028,6 +1028,7 @@ pref("devtools.errorconsole.enabled", false);
 // Enable the Inspector
 pref("devtools.inspector.enabled", true);
 pref("devtools.inspector.htmlHeight", 112);
+pref("devtools.inspector.htmlPanelOpen", false);
 
 // Enable the Debugger
 pref("devtools.debugger.enabled", false);

browser/base/content/browser-sets.inc

@@ -25,6 +25,7 @@
 #   Shawn Wilsher <me@shawnwilsher.com>
 #   Ehsan Akhgari <ehsan.akhgari@gmail.com>
 #   Rob Campbell <rcampbell@mozilla.com>
+#   Paul Rouget <paul@mozilla.com>
 #
 # Alternatively, the contents of this file may be used under the terms of
 # either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -153,6 +154,8 @@
              oncommand="InspectorUI.toggleSidebar();"/>
     <command id="Inspector:Tilt"
              oncommand="Tilt.initialize();"/>
+    <command id="Inspector:HTMLPanel"
+             oncommand="InspectorUI.toggleHTMLPanel();"/>
   </commandset>
 
   <broadcasterset id="mainBroadcasterSet">
@@ -167,7 +170,7 @@
                  type="checkbox" group="sidebar"
                  sidebarurl="chrome://browser/content/history/history-panel.xul"
                  oncommand="toggleSidebar('viewHistorySidebar');"/>
-                 
+
     <broadcaster id="viewWebPanelsSidebar" autoCheck="false"
                  type="checkbox" group="sidebar" sidebarurl="chrome://browser/content/web-panels.xul"
                  oncommand="toggleSidebar('viewWebPanelsSidebar');"/>

browser/base/content/browser.xul

@@ -38,6 +38,7 @@
 #   David Dahl <ddahl@mozilla.com>
 #   Frank Yan <fyan@mozilla.com>
 #   Victor Porof <vporof@mozilla.com>
+#   Paul Rouget <paul@mozilla.com>
 #
 # Alternatively, the contents of this file may be used under the terms of
 # either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -1004,6 +1005,12 @@
                      label="&inspectButton.label;"
                      accesskey="&inspectButton.accesskey;"
                      command="Inspector:Inspect"/>
+      <toolbarbutton id="inspector-treepanel-toolbutton"
+                     class="devtools-toolbarbutton"
+                     label="&htmlPanel.label;"
+                     accesskey="&htmlPanel.accesskey;"
+                     tooltiptext="&htmlPanel.tooltiptext;"
+                     command="Inspector:HTMLPanel"/>
       <arrowscrollbox id="inspector-breadcrumbs"
                       flex="1" orient="horizontal"
                       clicktoscroll="true"/>

browser/devtools/highlighter/TreePanel.jsm

@@ -64,7 +64,7 @@ function TreePanel(aContext, aIUI) {
 TreePanel.prototype = {
   showTextNodesWithWhitespace: false,
   id: "treepanel", // DO NOT LOCALIZE
-  openInDock: true,
+  _open: false,
 
   /**
    * The tree panel container element.
@@ -75,11 +75,7 @@ TreePanel.prototype = {
    */
   get container()
   {
-    if (this.openInDock) {
-      return this.document.getElementById("inspector-tree-box");
-    }
-
-    return this.document.getElementById("inspector-tree-panel");
+    return this.document.getElementById("inspector-tree-box");
   },
 
   /**
@@ -93,6 +89,8 @@ TreePanel.prototype = {
     this.IUI = aIUI;
     this.window = aContext;
     this.document = this.window.document;
+    this.button =
+     this.IUI.chromeDoc.getElementById("inspector-treepanel-toolbutton");
 
     domplateUtils.setDOM(this.window);
 
@@ -100,28 +98,7 @@ TreePanel.prototype = {
 
     let isOpen = this.isOpen.bind(this);
 
-    this.registrationObject = {
-      id: this.id,
-      label: this.IUI.strings.GetStringFromName("htmlPanel.label"),
-      tooltiptext: this.IUI.strings.GetStringFromName("htmlPanel.tooltiptext"),
-      accesskey: this.IUI.strings.GetStringFromName("htmlPanel.accesskey"),
-      context: this,
-      get isOpen() isOpen(),
-      show: this.open,
-      hide: this.close,
-      onSelect: this.select,
-      panel: this.openInDock ? null : this.container,
-      unregister: this.destroy,
-    };
     this.editingEvents = {};
-
-    if (!this.openInDock) {
-      this._boundClose = this.close.bind(this);
-      this.container.addEventListener("popuphiding", this._boundClose, false);
-    }
-
-    // Register the HTML panel with the highlighter
-    this.IUI.registerTool(this.registrationObject);
   },
 
   /**
@@ -154,13 +131,14 @@ TreePanel.prototype = {
    */
   open: function TP_open()
   {
-    if (this.initializingTreePanel && !this.treeLoaded) {
+    if (this._open) {
       return;
     }
 
+    this._open = true;
+
+    this.button.setAttribute("checked", true);
     this.initializingTreePanel = true;
-    if (!this.openInDock)
-      this.container.hidden = false;
 
     this.treeIFrame = this.document.getElementById("inspector-tree-iframe");
     if (!this.treeIFrame) {
@@ -170,56 +148,6 @@ TreePanel.prototype = {
       this.treeIFrame.setAttribute("type", "content");
     }
 
-    if (this.openInDock) { // Create vbox
-      this.openDocked();
-      return;
-    }
-
-    let resizerBox = this.document.getElementById("tree-panel-resizer-box");
-    this.treeIFrame = this.container.insertBefore(this.treeIFrame, resizerBox);
-
-    let boundLoadedInitializeTreePanel = function loadedInitializeTreePanel()
-    {
-      this.treeIFrame.removeEventListener("load",
-        boundLoadedInitializeTreePanel, true);
-      this.initializeIFrame();
-    }.bind(this);
-
-    let boundTreePanelShown = function treePanelShown()
-    {
-      this.container.removeEventListener("popupshown",
-        boundTreePanelShown, false);
-
-      this.treeIFrame.addEventListener("load",
-        boundLoadedInitializeTreePanel, true);
-
-      let src = this.treeIFrame.getAttribute("src");
-      if (src != INSPECTOR_URI) {
-        this.treeIFrame.setAttribute("src", INSPECTOR_URI);
-      } else {
-        this.treeIFrame.contentWindow.location.reload();
-      }
-    }.bind(this);
-
-    this.container.addEventListener("popupshown", boundTreePanelShown, false);
-
-    const panelWidthRatio = 7 / 8;
-    const panelHeightRatio = 1 / 5;
-
-    let width = parseInt(this.IUI.win.outerWidth * panelWidthRatio);
-    let height = parseInt(this.IUI.win.outerHeight * panelHeightRatio);
-    let y = Math.min(this.document.defaultView.screen.availHeight - height,
-      this.IUI.win.innerHeight);
-
-    this.container.openPopup(this.browser, "overlap", 0, 0,
-      false, false);
-
-    this.container.moveTo(80, y);
-    this.container.sizeTo(width, height);
-  },
-
-  openDocked: function TP_openDocked()
-  {
     let treeBox = null;
     treeBox = this.document.createElement("vbox");
     treeBox.id = "inspector-tree-box";
@@ -242,15 +170,16 @@ TreePanel.prototype = {
 
     treeBox.appendChild(this.treeIFrame);
 
-    let boundLoadedInitializeTreePanel = function loadedInitializeTreePanel()
+    this._boundLoadedInitializeTreePanel = function loadedInitializeTreePanel()
     {
       this.treeIFrame.removeEventListener("load",
-        boundLoadedInitializeTreePanel, true);
+        this._boundLoadedInitializeTreePanel, true);
+      delete this._boundLoadedInitializeTreePanel;
       this.initializeIFrame();
     }.bind(this);
 
     this.treeIFrame.addEventListener("load",
-      boundLoadedInitializeTreePanel, true);
+      this._boundLoadedInitializeTreePanel, true);
 
     let src = this.treeIFrame.getAttribute("src");
     if (src != INSPECTOR_URI) {
@@ -265,16 +194,22 @@ TreePanel.prototype = {
    */
   close: function TP_close()
   {
-    if (this.openInDock) {
-      let treeBox = this.container;
-      Services.prefs.setIntPref("devtools.inspector.htmlHeight", treeBox.height);
-      let treeBoxParent = treeBox.parentNode;
-      treeBoxParent.removeChild(this.splitter);
-      treeBoxParent.removeChild(treeBox);
-    } else {
-      this.container.hidePopup();
+    this._open = false;
+
+    // Stop caring about the tree iframe load if it's in progress.
+    if (this._boundLoadedInitializeTreePanel) {
+      this.treeIFrame.removeEventListener("load",
+        this._boundLoadedInitializeTreePanel, true);
+      delete this._boundLoadedInitializeTreePanel;
     }
 
+    this.button.removeAttribute("checked");
+    let treeBox = this.container;
+    Services.prefs.setIntPref("devtools.inspector.htmlHeight", treeBox.height);
+    let treeBoxParent = treeBox.parentNode;
+    treeBoxParent.removeChild(this.splitter);
+    treeBoxParent.removeChild(treeBox);
+
     if (this.treePanelDiv) {
       this.treePanelDiv.ownerPanel = null;
       let parent = this.treePanelDiv.parentNode;
@@ -292,10 +227,15 @@ TreePanel.prototype = {
    */
   isOpen: function TP_isOpen()
   {
-    if (this.openInDock)
-      return this.treeLoaded && this.container;
+    return this._open;
+  },
 
-    return this.treeLoaded && this.container.state == "open";
+  /**
+   * Toggle the TreePanel.
+   */
+  toggle: function TP_toggle()
+  {
+    this.isOpen() ? this.close() : this.open();
   },
 
   /**
@@ -704,11 +644,6 @@ TreePanel.prototype = {
       this.ioBox.destroy();
       delete this.ioBox;
     }
-
-    if (!this.openInDock) {
-      this.container.removeEventListener("popuphiding", this._boundClose, false);
-      delete this._boundClose;
-    }
   }
 };
 

browser/devtools/highlighter/inspector.jsm

@@ -108,7 +108,6 @@ InspectorUI.prototype = {
   tools: null,
   toolEvents: null,
   inspecting: false,
-  treePanelEnabled: true,
   ruleViewEnabled: true,
   isDirty: false,
   store: null,
@@ -192,6 +191,22 @@ InspectorUI.prototype = {
     }
   },
 
+  /**
+   * Toggle the TreePanel.
+   */
+  toggleHTMLPanel: function TP_toggle()
+  {
+    if (this.treePanel.isOpen()) {
+      this.treePanel.close();
+      Services.prefs.setBoolPref("devtools.inspector.htmlPanelOpen", false);
+      this.store.setValue(this.winID, "htmlPanelOpen", false);
+    } else {
+      this.treePanel.open();
+      Services.prefs.setBoolPref("devtools.inspector.htmlPanelOpen", true);
+      this.store.setValue(this.winID, "htmlPanelOpen", true);
+    }
+  },
+
   /**
    * Is the inspector UI open? Simply check if the toolbar is visible or not.
    *
@@ -260,9 +275,7 @@ InspectorUI.prototype = {
     this.initTools();
     this.chromeWin.Tilt.setup();
 
-    if (this.treePanelEnabled) {
-      this.treePanel = new TreePanel(this.chromeWin, this);
-    }
+    this.treePanel = new TreePanel(this.chromeWin, this);
 
     if (Services.prefs.getBoolPref("devtools.ruleview.enabled") &&
         !this.toolRegistered("ruleview")) {
@@ -349,6 +362,10 @@ InspectorUI.prototype = {
       this.store.setValue(this.winID, "selectedNode", null);
       this.store.setValue(this.winID, "inspecting", true);
       this.store.setValue(this.winID, "isDirty", this.isDirty);
+
+      this.store.setValue(this.winID, "htmlPanelOpen",
+        Services.prefs.getBoolPref("devtools.inspector.htmlPanelOpen"));
+
       this.win.addEventListener("pagehide", this, true);
     }
   },
@@ -400,6 +417,8 @@ InspectorUI.prototype = {
     if (this.treePanel && this.treePanel.editingContext)
       this.treePanel.closeEditor();
 
+    this.treePanel.destroy();
+
     if (this.closing || !this.win || !this.browser) {
       return;
     }
@@ -538,6 +557,7 @@ InspectorUI.prototype = {
 
     this.breadcrumbs.update();
     this.chromeWin.Tilt.update(aNode);
+    this.treePanel.select(aNode, aScroll);
 
     this.toolsSelect(aScroll);
   },
@@ -586,6 +606,10 @@ InspectorUI.prototype = {
     this.win.focus();
     this.highlighter.highlight();
 
+    if (this.store.getValue(this.winID, "htmlPanelOpen")) {
+      this.treePanel.open();
+    }
+
     Services.obs.notifyObservers({wrappedJSObject: this},
                                  INSPECTOR_NOTIFICATIONS.OPENED, null);
   },

browser/devtools/highlighter/test/browser_inspector_editor.js

@@ -34,7 +34,7 @@ function setupHTMLPanel()
 {
   Services.obs.removeObserver(setupHTMLPanel, InspectorUI.INSPECTOR_NOTIFICATIONS.OPENED);
   Services.obs.addObserver(runEditorTests, InspectorUI.INSPECTOR_NOTIFICATIONS.TREEPANELREADY, false);
-  InspectorUI.toolShow(InspectorUI.treePanel.registrationObject);
+  InspectorUI.toggleHTMLPanel();
 }
 
 function runEditorTests()

browser/devtools/highlighter/test/browser_inspector_ruleviewstore.js

@@ -128,8 +128,8 @@ function ruleViewOpened2()
   is(prop.name, "background-color", "First prop is the background color prop.");
   ok(!prop.enabled, "First prop should be disabled.");
 
-  gBrowser.removeCurrentTab();
   InspectorUI.closeInspectorUI();
+  gBrowser.removeCurrentTab();
   finish();
 }
 

browser/devtools/highlighter/test/browser_inspector_tab_switch.js

@@ -136,7 +136,7 @@ function inspectorFocusTab1()
   Services.obs.addObserver(inspectorOpenTreePanelTab1,
     InspectorUI.INSPECTOR_NOTIFICATIONS.TREEPANELREADY, false);
 
-  InspectorUI.treePanel.open();
+  InspectorUI.toggleHTMLPanel();
 }
 
 function inspectorOpenTreePanelTab1()

browser/devtools/highlighter/test/head.js

@@ -41,6 +41,14 @@ let tempScope = {};
 Cu.import("resource:///modules/devtools/LayoutHelpers.jsm", tempScope);
 let LayoutHelpers = tempScope.LayoutHelpers;
 
+// Clear preferences that may be set during the course of tests.
+function clearUserPrefs()
+{
+  Services.prefs.clearUserPref("devtools.inspector.htmlPanelOpen");
+}
+
+registerCleanupFunction(clearUserPrefs);
+
 function isHighlighting()
 {
   let veil = InspectorUI.highlighter.veilTransparentBox;

browser/locales/en-US/chrome/browser/browser.dtd

@@ -599,3 +599,15 @@ just addresses the organization to follow, e.g. "This site is run by " -->
 
 <!ENTITY addonBarCloseButton.tooltip  "Close Add-on Bar">
 <!ENTITY toggleAddonBarCmd.key        "/">
+
+<!-- LOCALIZATION NOTE (htmlPanel.label): This is a label for a button that
+activates the Web Developer->Inspect UI's HTML Tree Panel. -->
+<!ENTITY htmlPanel.label              "HTML">
+
+<!-- LOCALIZATION NOTE (htmlPanel.tooltiptext): The text that appears when a user
+hovers over the HTML panel's toolbar button. -->
+<!ENTITY htmlPanel.tooltiptext        "HTML panel">
+
+<!-- LOCALIZATION NOTE (htmlPanel.accesskey): The key bound to the HTML panel's
+toolbar button -->
+<!ENTITY htmlPanel.accesskey          "H">

browser/locales/en-US/chrome/browser/devtools/inspector.properties

@@ -20,20 +20,8 @@ breadcrumbs.siblings=Siblings
 # LOCALIZATION NOTE (htmlPanel): Used in the Inspector tool's openInspectorUI
 # method when registering the HTML panel.
 
-# LOCALIZATION NOTE (htmlPanel.label): This is a lable for a button that
-# activates the Web Developer->Inspect UI's HTML Tree Panel.
-htmlPanel.label=HTML
-
-# LOCALIZATION NOTE (htmlPanel.tooltiptext): The text that appears when a user
-# hovers over the HTML panel's toolbar button.
-htmlPanel.tooltiptext=HTML panel
-
-# LOCALIZATION NOTE (htmlPanel.accesskey): The key bound to the HTML panel's
-# toolbar button.
-htmlPanel.accesskey=H
-
 # LOCALIZATION NOTE (ruleView.*): Button label, accesskey and tooltip text
 # associated with the Highlighter's CSS Rule View in the Style Sidebar.
 ruleView.label=Rules
 ruleView.accesskey=R
-ruleView.tooltiptext=View and Edit CSS
+ruleView.tooltiptext=View and Edit CSS