Element Hiding Helper inspector tool fix

lib/inspectorObserver.js

Index: lib/inspectorObserver.js
===================================================================
--- a/lib/inspectorObserver.js
+++ b/lib/inspectorObserver.js
@@ -4,7 +4,7 @@
  * http://mozilla.org/MPL/2.0/.
  */
 
-Cu.import("resource:///modules/devtools/gDevTools.jsm");
+let {gDevTools} = Cu.import("resource:///modules/devtools/gDevTools.jsm", null);
 
 let InspectorObserver =
 {
@@ -21,32 +21,26 @@
   {
     // 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.accesskey"), stringBundle.GetStringFromName("inspector.button.tooltiptext")];

[Wladimir Palant, 2014/04/11 18:39:10]

An access key only makes sense in combination with a label.

[saroyanm, 2014/04/12 10:39:35]

Wladimir why we need to remove accesskey ? Isn't it useful for quickly access
the action ?
Not sure if I understand how they are related, can you please drop a comment on
this.

[Wladimir Palant, 2014/04/12 18:14:11]

You probably don't know what access keys are - see
https://developer.mozilla.org/en/docs/XUL_Accesskey_FAQ_and_Policies.

[saroyanm, 2014/04/12 20:23:20]

Thanks for the reference Wladimir,
actually I feel myself that I still don't understand why we need to use
inspector button in this case without accesskey attribute even if we don't have
label attribute,
Sorry if I understand your point wrong, but let me describe how I understand
Accesskeys:
Accesskey is used to simulate click on the element and I'm not sure why it's
only make sense in combination with label in our case even if we will have no
underline hint(A) in label for accesskey.
I feel myself that it's quite useful feature from usability point of view.
Maybe I'm missing something, sorry for that beforehand.
Anyway if that is the point to only have accesskeys with combination of label or
underline letter hint, maybe we can use some other combination or use
Ctrl+Shift+f3 in case of inspector opened, what you think ?

[Wladimir Palant, 2014/04/12 20:51:26]

An access key is not the same thing as a shortcut key. An access key is the
underlined letter of a button or some other control - e.g. if "a" is underlined
then you know that you can press Alt-A to trigger that button.

A shortcut key on the other hand can be any key combination that triggers any
action, not necessarily related to a particular control. Shortcut keys are
usually assigned to menu items, that's also how users are normally supposed to
discover them.

A shortcut key makes no sense in this context, there is no way for the user to
find out about it. Also, buttons normally don't have shortcut keys. As to access
keys, an access key requires a label - image-only buttons cannot have one.

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

I thought again regarding your notes Wladimir I guess you are right as usual,
just forget about upper notes :) 
It's still make sense to follow the rules for using accesskey in case there will
be other extension or feature implemented in FF that uses same accesskey in
inspector.
The button should be enough.

[saroyanm, 2014/04/12 21:03:22]

Got it. Thanks.

 
     delete this.inspectorButton;
     this.__defineGetter__("inspectorButton", function() result);
     return this.inspectorButton;
   },
   
-  inspectorReady: function(eventName, listener)
+  inspectorReady: function(eventName, toolbox, panel)
   {
-    let toolbox = gDevTools.getToolbox(listener.target);
-    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;
-    
-    let panelWindow = panel.panelWin;
-    let parent = panelWindow.document.getElementById("inspector-toolbar");
-    if (!parent)
-      return;
-    
-    let [label, accesskey, tooltiptext] = InspectorObserver.inspectorButton;
+    let [accesskey, 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')";

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

Not sure if I need to copy icon to elemhidehelper/chrome/skin/abp-status-16.png
?

[Wladimir Palant, 2014/04/11 18:39:10]

No, it's fine like that - Element Hiding Helper can rely on Adblock Plus.

+    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);
@@ -56,7 +50,7 @@
       panelWindow.openDialog("chrome://elemhidehelper/content/composer.xul", "_blank", 
                              "chrome,centerscreen,resizable,dialog=no", panel.selection.node);
     }, false);
-    parent.appendChild(button);
+    inspectBtn.parentNode.insertBefore(button, inspectBtn.nextSibling);
   }
 };