Fix toolbar icon customization in Australis

lib/ui.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 395 matching lines @@
   {
     let {WindowObserver} = require("windowObserver");
     new WindowObserver(this);
 
     // Add toolbar icon
     let {defaultToolbarPosition} = require("appSupport");
     if ("abp-toolbarbutton" in this.overlay && defaultToolbarPosition)
     {
       CustomizableUI.createWidget({
         id: "abp-toolbarbutton",
-        positionAttribute: "abp-iconposition",
+        type: "custom",
+        positionAttribute: "abp-iconposition",        // For emulation only
         defaultArea: defaultToolbarPosition.parent,
-        defaultBefore: defaultToolbarPosition.before,
-        defaultAfter: defaultToolbarPosition.after,
+        defaultBefore: defaultToolbarPosition.before, // For emulation only
+        defaultAfter: defaultToolbarPosition.after,   // For emulation only
         removable: true,
-        onCreated: function(node)
+        onBuild: function(document)
         {

[tschuster, 2014/05/15 20:12:30]

We can probably make these arrow functions when I land my patch first :)

[Wladimir Palant, 2014/07/11 07:15:54]

Don't think so, arrow functions are for trivial functions - here I'd prefer
having proper ones.

-          for (let attribute of this.overlay["abp-toolbarbutton"].attributes)
-            node.setAttribute(attribute.name, attribute.value);
-          if (!node.firstChild)
-            for (let child of this.overlay["abp-toolbarbutton"].children)
-              node.appendChild(child.cloneNode(true));
-          if ("addClass" in defaultToolbarPosition)
-            icon.classList.add(defaultToolbarPosition.addClass);
-
+          let node = document.importNode(this.overlay["abp-toolbarbutton"], true);
+          this.updateIconState(document.defaultView, node);
+          return node;
+        }.bind(this),
+        onAdded: function(node)
+        {
+          // For emulation only, this callback isn't part of the official
+          // CustomizableUI API.
           this.updateIconState(node.ownerDocument.defaultView, node);
         }.bind(this),
         onClick: this.onIconClick,
         onCommand: this.onIconCommand

[Wladimir Palant, 2013/12/02 15:14:18]

These two lines don't actually do anything, they are ignored if type isn't
button. https://hg.adblockplus.org/adblockplus/rev/245083285e39 fixed this.

       });
       onShutdown.add(CustomizableUI.destroyWidget.bind(CustomizableUI, "abp-toolbarbutton"));
     }
 
     // Listen for pref and filters changes
     Prefs.addListener(function(name)
     {
       if (name == "enabled" || name == "defaulttoolbaraction" || name == "defaultstatusbaraction")
         this.updateState();
       else if (name == "showinstatusbar")
@@ skipping 551 matching lines @@
 
   /**
    * Toggles visibility state of the toolbar icon.
    */
   toggleToolbarIcon: function()
   {
     if (this.isToolbarIconVisible())
       CustomizableUI.removeWidgetFromArea("abp-toolbarbutton");
     else
     {
       let {defaultToolbarPosition} = require("appSupport");

[tschuster, 2014/05/15 20:12:30]

From the rest of the code it looks like this can be null, but I can't say if it
could happen here.

[Wladimir Palant, 2014/07/11 07:15:54]

No, it cannot - nothing would call toggleToolbarIcon() if the toolbar icon isn't
supported.

       CustomizableUI.addWidgetToArea("abp-toolbarbutton", defaultToolbarPosition.parent);
     }
   },
 
   /**
    * Updates Adblock Plus icon state for all windows.
    */
   updateState: function()
   {
     for (let window in this.applicationWindows)
@@ skipping 185 matching lines @@
 
   /**
    * Called when some pop-up in the application window shows up, initializes
    * pop-ups related to Adblock Plus.
    */
   onPopupShowing: function(/**Event*/ event)
   {
     if (event.defaultPrevented)
       return;
 
-    let popup = event.target;
+    let popup = event.originalTarget;
 
     let {contentContextMenu} = require("appSupport");
     if ((typeof contentContextMenu == "string" && popup.id == contentContextMenu) ||
         (contentContextMenu instanceof Array && contentContextMenu.indexOf(popup.id) >= 0))
     {
       this.fillContentContextMenu(popup);
     }
     else if (popup.id == "abp-tooltip")
       this.fillIconTooltip(event, popup.ownerDocument.defaultView);
     else
@@ skipping 634 matching lines @@
   ["abp-command-contribute", "command", UI.openContributePage.bind(UI)],
   ["abp-command-contribute-hide", "command", UI.hideContributeButton.bind(UI)]
 ];
 
 onShutdown.add(function()
 {
   for (let window in UI.applicationWindows)
     if (UI.isBottombarOpen(window))
       UI.toggleBottombar(window);
 });