Element Hiding Helper inspector tool fix

lib/inspectorObserver.js

 /*
  * This Source Code is subject to the terms of the Mozilla Public License
  * version 2.0 (the "License"). You can obtain a copy of the License at
  * http://mozilla.org/MPL/2.0/.
  */
 
-Cu.import("resource:///modules/devtools/gDevTools.jsm");

[Wladimir Palant, 2014/04/11 11:06:08]

While you are at it, please don't use a blind import here (we need to change
this everywhere else as well, it's just not a priority):

let {gDevTools} = Cu.import("resource:///modules/devtools/gDevTools.jsm", null);

[saroyanm, 2014/04/11 16:09:10]

Got it.

+let {gDevTools} = Cu.import("resource:///modules/devtools/gDevTools.jsm", null);
 
 let InspectorObserver =
 {
   init: function()
   {
     gDevTools.on("inspector-ready", this.inspectorReady);
     onShutdown.add(function()
     {
       gDevTools.off("inspector-ready", this.inspectorReady);
     }.bind(this));
   },
 
   get inspectorButton()

[saroyanm, 2014/04/12 20:59:38]

Not sure if it's make sense to change getter into inspectorButtonTooltip ?

[Wladimir Palant, 2014/04/12 21:05:21]

Yes, I think the name should be changed.

   {
     // Randomize URI to work around bug 719376
     let stringBundle = Services.strings.createBundle("chrome://elemhidehelper/locale/global.properties?" + Math.random());
-    let result = [stringBundle.GetStringFromName("inspector.button.label"), stringBundle.GetStringFromName("inspector.button.accesskey"), stringBundle.GetStringFromName("inspector.button.tooltiptext")];
+    let result = stringBundle.GetStringFromName("inspector.button.tooltiptext");
 
     delete this.inspectorButton;
     this.__defineGetter__("inspectorButton", function() result);
     return this.inspectorButton;
   },
   
-  inspectorReady: function(eventName, listener)

[Wladimir Palant, 2014/04/11 11:06:08]

The second parameter here isn't listener but panel (ToolPanel instance).

[saroyanm, 2014/04/11 16:09:10]

Good catch, actually I've tested and seams like the parameters are:
1. eventName (string)
2. toolBox (Toolbox)
3. panel (ToolPanel)
example:
http://mxr.mozilla.org/mozilla-central/source/browser/devtools/tilt/tilt-visu...

+  inspectorReady: function(eventName, toolbox, panel)
   {
-    let toolbox = gDevTools.getToolbox(listener.target);

[Wladimir Palant, 2014/04/11 11:06:08]

I checked why the documentation mentions getToolboxByTarget() method instead -
it seems that the API changed with
https://bugzilla.mozilla.org/show_bug.cgi?id=813031. That means that the API we
are using here was introduced in Firefox 20 - please update metadata.gecko file
to make sure we don't support any earlier versions.

[saroyanm, 2014/04/11 16:09:10]

Good point: I've tested with FF19 and there is an error for current line:
Cu.import("resource:///modules/devtools/gDevTools.jsm");
Tested with FF20 and worked great, as you told :)

-    if (!toolbox)
+    let panelWindow = panel.panelWin;
+    let inspectBtn = panelWindow.document.getElementById("inspector-inspect-toolbutton");
+    if (!inspectBtn)
       return;
     
-    let panel = toolbox.getToolPanels().get("inspector");
-    if (!panel)
-      return;

[Wladimir Palant, 2014/04/11 11:06:08]

So you are going from the panel to the toolbox to get the panel :)

[saroyanm, 2014/04/11 16:09:10]

Lol :)

-    
-    let panelWindow = panel.panelWin;
-    let parent = panelWindow.document.getElementById("inspector-toolbar");
-    if (!parent)
-      return;
-    
-    let [label, accesskey, tooltiptext] = InspectorObserver.inspectorButton;
+    let tooltiptext = InspectorObserver.inspectorButton;
     button = panelWindow.document.createElement("toolbarbutton");
     button.setAttribute("id", "inspector-abp-elemhide-toolbutton");
-    button.setAttribute("label", label);
+    button.style.listStyleImage = "url('chrome://adblockplus/skin/abp-status-16.png')";
+    button.style.MozImageRegion = "rect(0px, 16px, 16px, 0px)";
+    button.style.paddingTop = "4px";
     button.setAttribute("class", "devtools-toolbarbutton");
     button.setAttribute("accesskey", accesskey);
     button.setAttribute("tooltiptext", tooltiptext);
     button.setAttribute("tabindex", "0");
     button.addEventListener("command", function()
     {
       panelWindow.openDialog("chrome://elemhidehelper/content/composer.xul", "_blank", 
                              "chrome,centerscreen,resizable,dialog=no", panel.selection.node);
     }, false);
-    parent.appendChild(button);

[Wladimir Palant, 2014/04/11 11:06:08]

Adding the button at the end probably isn't the best solution, it should be
placed before the inspector-breadcrumbs element.

Also, the button occupies too much space right now. You should use
button.style.listStyleImage to use the Adblock Plus icon for it and remove the
label. The tooltip should be good enough to explain what it is about.

[saroyanm, 2014/04/11 16:09:10]

Good point changed. Thanks for pointing.

+    inspectBtn.parentNode.insertBefore(button, inspectBtn.nextSibling);
   }
 };
 
 InspectorObserver.init();