Fix toolbar icon customization in Australis

lib/ui.js

Index: lib/ui.js
===================================================================
--- a/lib/ui.js
+++ b/lib/ui.js
@@ -26,16 +26,27 @@ let {FilterStorage} = require("filterSto
 let {FilterNotifier} = require("filterNotifier");
 let {RequestNotifier} = require("requestNotifier");
 let {Filter} = require("filterClasses");
 let {Subscription, SpecialSubscription, DownloadableSubscription} = require("subscriptionClasses");
 let {Synchronizer} = require("synchronizer");
 let {KeySelector} = require("keySelector");
 let {Notification} = require("notification");
 
+let CustomizableUI;
+try
+{
+  ({CustomizableUI}) = Cu.import("resource:///modules/CustomizableUI.jsm", null);
+}
+catch (e)
+{
+  // No built-in CustomizableUI API, use our own implementation.
+  ({CustomizableUI}) = require("customizableUI");
+}
+
 /**
  * Filter corresponding with "disable on site" menu item (set in fillIconMent()).
  * @type Filter
  */
 let siteWhitelist = null;
 /**
  * Filter corresponding with "disable on site" menu item (set in fillIconMenu()).
  * @type Filter
@@ -92,21 +103,19 @@ let optionsObserver =
 
     addCommandHandler("adblockplus-filters", UI.openFiltersDialog.bind(UI));
 
     let {Sync} = require("sync");
     let syncEngine = Sync.getEngine();
     hideElement("adblockplus-sync", !syncEngine);
 
     let {defaultToolbarPosition, statusbarPosition} = require("appSupport");
-    let hasToolbar = defaultToolbarPosition && !defaultToolbarPosition.isAddonBar;
-    let hasAddonBar = defaultToolbarPosition && defaultToolbarPosition.isAddonBar;
+    let hasToolbar = defaultToolbarPosition;
     let hasStatusBar = statusbarPosition;
 
-    hideElement("adblockplus-showinaddonbar", !hasAddonBar);
     hideElement("adblockplus-showintoolbar", !hasToolbar);
     hideElement("adblockplus-showinstatusbar", !hasStatusBar);
 
     let checkbox = doc.querySelector("setting[type=bool]");
     if (checkbox)
       initCheckboxes();
 
     function initCheckboxes()
@@ -133,33 +142,22 @@ let optionsObserver =
       });
 
       setChecked("adblockplus-sync", syncEngine && syncEngine.enabled);
       addCommandHandler("adblockplus-sync", function()
       {
         this.value = UI.toggleSync();
       });
 
-      let window = null;
-      for (window in UI.applicationWindows)
-        break;
-
-      if (window)
+      setChecked("adblockplus-showintoolbar", UI.isToolbarIconVisible());
+      addCommandHandler("adblockplus-showintoolbar", function()
       {
-        setChecked("adblockplus-showinaddonbar", UI.isToolbarIconVisible(window));
-        setChecked("adblockplus-showintoolbar", UI.isToolbarIconVisible(window));
-
-        let handler = function()
-        {
-          UI.toggleToolbarIcon();
-          this.value = UI.isToolbarIconVisible(window);
-        };
-        addCommandHandler("adblockplus-showinaddonbar", handler);
-        addCommandHandler("adblockplus-showintoolbar", handler);
-      }
+        UI.toggleToolbarIcon();
+        this.value = UI.isToolbarIconVisible();
+      });
 
       let list = doc.getElementById("adblockplus-subscription-list");
       if (list)
       {
         // Load subscriptions data
         let request = new XMLHttpRequest();
         request.mozBackgroundRequest = true;
         request.open("GET", "chrome://adblockplus/content/ui/subscriptions.xml");
@@ -314,19 +312,17 @@ let UI = exports.UI =
           this.initDone();
       }.bind(this);
       FilterNotifier.addListener(listener);
     }
     else
       filtersLoaded = true;
 
     // Initialize UI after the session is restored
-    let window = null;
-    for (window in this.applicationWindows)
-      break;
+    let window = this.currentWindow;
     if (!window && "nsISessionStore" in Ci)
     {
       // No application windows yet, the application must be starting up. Wait
       // for session to be restored before initializing our UI.
       new SessionRestoreObserver(function()
       {
         sessionRestored = true;
         if (overlayLoaded && filtersLoaded && sessionRestored)
@@ -406,16 +402,46 @@ let UI = exports.UI =
    * Gets called once the initialization is finished and Adblock Plus elements
    * can be added to the UI.
    */
   initDone: function()
   {
     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",
+        type: "custom",
+        positionAttribute: "abp-iconposition",        // For emulation only
+        defaultArea: defaultToolbarPosition.parent,
+        defaultBefore: defaultToolbarPosition.before, // For emulation only
+        defaultAfter: defaultToolbarPosition.after,   // For emulation only
+        removable: true,
+        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.

+          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")
       {
         for (let window in this.applicationWindows)
@@ -464,37 +490,16 @@ let UI = exports.UI =
       Utils.runAsync(this.applyToWindow.bind(this, window, true));
       return;
     }
 
     // Add general items to the document
     for (let i = 0; i < this.overlay.all.length; i++)
       window.document.documentElement.appendChild(this.overlay.all[i].cloneNode(true));
 
-    // Add toolbar icon
-    if ("abp-toolbarbutton" in this.overlay)
-    {
-      let toolbox = this.getToolbox(window);
-      if (toolbox)
-      {
-        // Insert toolbar button asynchronously, otherwise it will show up
-        // before our stylesheet loads
-        Utils.runAsync(function()
-        {
-          toolbox.addEventListener("aftercustomization", this.onToolbarCustomization, false);
-
-          let {defaultToolbarPosition} = require("appSupport");
-          let icon = this.overlay["abp-toolbarbutton"].cloneNode(true);
-          if ("addClass" in defaultToolbarPosition)
-            icon.classList.add(defaultToolbarPosition.addClass);
-          this.restoreToolbarIcon(toolbox, icon);
-        }.bind(this));
-      }
-    }
-
     // Add status bar icon
     this.updateStatusbarIcon(window);
 
     // Add tools menu item
     if ("abp-menuitem" in this.overlay)
     {
       let {toolsMenu} = require("appSupport");
       let [parent, before] = this.resolveInsertionPoint(window, toolsMenu);
@@ -574,29 +579,16 @@ let UI = exports.UI =
       else
       {
         let clone = window.document.getElementById(id);
         if (clone)
           clone.parentNode.removeChild(clone);
       }
     }
 
-    if ("abp-toolbarbutton" in this.overlay)
-    {
-      let toolbox = this.getToolbox(window);
-      if (toolbox)
-      {
-        toolbox.removeEventListener("aftercustomization", this.onToolbarCustomization, false);
-
-        let paletteItem = this.getPaletteItem(toolbox);
-        if (paletteItem)
-          paletteItem.parentNode.removeChild(paletteItem);
-      }
-    }
-
     window.removeEventListener("popupshowing", this.onPopupShowing, false);
     window.removeEventListener("keypress", this.onKeyPress, false);
     removeBrowserLocationListeners(window);
     removeBrowserClickListeners(window);
   },
 
   /**
    * The overlay information to be used when adding elements to the UI.
@@ -622,26 +614,36 @@ let UI = exports.UI =
     {
       let window = enumerator.getNext().QueryInterface(Ci.nsIDOMWindow);
       if (isKnownWindow(window))
         yield window;
     }
   },
 
   /**
+   * Returns the top-most application window or null if none exists.
+   * @type Window
+   */
+  get currentWindow()
+  {
+    for (let window of this.applicationWindows)
+      return window;
+    return null;
+  },
+
+  /**
    * Opens a URL in the browser window. If browser window isn't passed as parameter,
    * this function attempts to find a browser window. If an event is passed in
    * it should be passed in to the browser if possible (will e.g. open a tab in
    * background depending on modifiers keys).
    */
   loadInBrowser: function(/**String*/ url, /**Window*/ currentWindow, /**Event*/ event)
   {
     if (!currentWindow)
-      for (currentWindow in this.applicationWindows)
-        break;
+      currentWindow = this.currentWindow;
 
     let {addTab} = require("appSupport");
     if (currentWindow && addTab)
       addTab(currentWindow, url, event);
     else
     {
       let protocolService = Cc["@mozilla.org/uriloader/external-protocol-service;1"].getService(Ci.nsIExternalProtocolService);
       protocolService.loadURI(Services.io.newURI(url, null, null), null);
@@ -946,19 +948,19 @@ let UI = exports.UI =
   /**
    * Looks up an element with given ID in the window. If a list of IDs is given
    * will try all of them until an element exists.
    */
   findElement: function(/**Window*/ window, /**String|String[]*/ id) /**Element*/
   {
     if (id instanceof Array)
     {
-      for (let i = 0; i < id.length; i++)
+      for (let candidate of id)
       {
-        let result = window.document.getElementById(id[i]);
+        let result = window.document.getElementById(candidate);
         if (result)
           return result;
       }
       return null;
     }
     else
       return window.document.getElementById(id);
   },
@@ -989,188 +991,26 @@ let UI = exports.UI =
       if (before && before.parentNode != parent)
         before = null;
     }
 
     return [parent, before];
   },
 
   /**
-   * Finds the toolbox element responsible for the toolbar where Adblock Plus
-   * icon should be placed.
-   */
-  getToolbox: function(/**Window*/ window) /**Element*/
-  {
-    let {defaultToolbarPosition} = require("appSupport");
-    if (!defaultToolbarPosition || !defaultToolbarPosition.parent)
-      return null;
-
-    let toolbar = this.findElement(window, defaultToolbarPosition.parent);
-    if (!toolbar)
-      return null;
-
-    let toolbox = toolbar.toolbox;
-    if (toolbox && ("palette" in toolbox) && toolbox.palette)
-      return toolbox;
-    else
-      return null;
-  },
-
-  /**
-   * Restores the Adblock Plus icon at its original position.
-   */
-  restoreToolbarIcon: function(/**Element*/ toolbox, /**Element*/ icon)
-  {
-    // Insert into the palette first
-    toolbox.palette.insertBefore(icon, toolbox.palette.firstChild);
-
-    // Now find where we should put the icon
-    let position = toolbox.getAttribute("abp-iconposition");
-    if (!/^\S*,\S*,\S*$/.test(position))
-      position = null;
-
-    if (position == null)
-    {
-      // No explicitly saved position but maybe we can find it in a currentset
-      // attribute somewhere.
-      let toolbars = toolbox.externalToolbars.slice();
-      for (let child = toolbox.firstElementChild; child; child = child.nextElementSibling)
-        if (child.localName == "toolbar")
-          toolbars.push(child);
-      for (let i = 0; i < toolbars.length; i++)
-      {
-        let toolbar = toolbars[i];
-        let currentSet = toolbar.getAttribute("currentset");
-        if (currentSet)
-        {
-          let items = currentSet.split(",");
-          let index = items.indexOf("abp-toolbarbutton");
-          if (index >= 0)
-          {
-            position = "visible," + toolbar.id + "," + (index + 1 < items.length ? items[index + 1] : "");
-            toolbox.setAttribute("abp-iconposition", position);
-            toolbox.ownerDocument.persist(toolbox.id, "abp-iconposition");
-            break;
-          }
-        }
-      }
-    }
-
-    this.showToolbarIcon(toolbox, position);
-  },
-
-  /**
-   * Finds the toolbar button in the toolbar palette.
-   */
-  getPaletteItem: function(/**Element*/ toolbox) /**Element*/
-  {
-    for (var child = toolbox.palette.firstElementChild; child; child = child.nextElementSibling)
-      if (child.id == "abp-toolbarbutton")
-        return child;
-
-    return null;
-  },
-
-  /**
-   * Called after toolbar customization, sets up our icon and remembers its
-   * position.
-   */
-  onToolbarCustomization: function(/**Event*/ event)
-  {
-    let toolbox = event.currentTarget;
-    let icon = toolbox.ownerDocument.getElementById("abp-toolbarbutton");
-
-    let position = toolbox.getAttribute("abp-iconposition") || "hidden,,";
-    if (icon && icon.parentNode.localName == "toolbar")
-    {
-      this.updateIconState(icon.ownerDocument.defaultView, icon);
-      icon.addEventListener("click", this.onIconClick, false);
-      icon.addEventListener("command", this.onIconClick, false);
-      position = "visible," + icon.parentNode.id + "," + (icon.nextSibling ? icon.nextSibling.id : "");
-    }
-    else
-      position = position.replace(/^visible,/, "hidden,")
-
-    toolbox.setAttribute("abp-iconposition", position);
-    toolbox.ownerDocument.persist(toolbox.id, "abp-iconposition");
-  },
-
-  /**
-   * Shows or hides toolbar icon based on a remembered position.
-   */
-  showToolbarIcon: function(/**Element*/ toolbox, /**String*/ position)
-  {
-    let visible, parent, before;
-    if (position)
-    {
-      [visible, parent, before] = position.split(",", 3);
-      parent = toolbox.ownerDocument.getElementById(parent);
-      if (before == "")
-        before = null;
-      else
-        before = toolbox.ownerDocument.getElementById(before);
-      if (before && before.parentNode != parent)
-        before = null;
-    }
-    else
-    {
-      let {defaultToolbarPosition} = require("appSupport");
-      visible = "visible";
-      [parent, before] = this.resolveInsertionPoint(toolbox.ownerDocument.defaultView, defaultToolbarPosition);
-
-      if (parent && parent.collapsed)
-      {
-        // First time we insert the toolbar icon, make sure it is actually visible
-        parent.setAttribute("collapsed", "false");
-        toolbox.ownerDocument.persist(parent.id, "collapsed");
-      }
-    }
-
-    if (parent && parent.localName != "toolbar")
-      parent = null;
-
-    if (visible != "visible")
-    {
-      // Hide icon if it is currently visible
-      let icon = toolbox.ownerDocument.getElementById("abp-toolbarbutton");
-      if (icon && icon.parentNode.localName == "toolbar")
-        toolbox.palette.appendChild(icon);
-    }
-    else if (parent)
-    {
-      // Add the icon to the toolbar
-      let items = parent.currentSet.split(",");
-      let index = (before ? items.indexOf(before.id) : -1);
-      if (index < 0)
-        before = null;
-      parent.insertItem("abp-toolbarbutton", before, null, false);
-    }
-
-    this.onToolbarCustomization({currentTarget: toolbox});
-  },
-
-  /**
    * Toggles visibility state of the toolbar icon.
    */
   toggleToolbarIcon: function()
   {
-    for (let window in this.applicationWindows)
+    if (this.isToolbarIconVisible())
+      CustomizableUI.removeWidgetFromArea("abp-toolbarbutton");
+    else
     {
-      let toolbox = this.getToolbox(window);
-      if (!toolbox)
-        continue;
-
-      let position = toolbox.getAttribute("abp-iconposition");
-      if (position)
-      {
-        let parts = position.split(",");
-        parts[0] = (parts[0] == "visible" ? "hidden" : "visible");
-        position = parts.join(",");
-      }
-      this.showToolbarIcon(toolbox, position);
+      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()
   {
@@ -1365,17 +1205,17 @@ let UI = exports.UI =
    * 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")
@@ -1623,26 +1463,23 @@ let UI = exports.UI =
     setDisabled("abp-command-sendReport", !location || !Policy.isBlockableScheme(location) || location.scheme == "mailto");
 
     setChecked(prefix + "disabled", !Prefs.enabled);
     setChecked(prefix + "frameobjects", Prefs.frameobjects);
     setChecked(prefix + "slowcollapse", !Prefs.fastcollapse);
     setChecked(prefix + "savestats", Prefs.savestats);
 
     let {defaultToolbarPosition, statusbarPosition} = require("appSupport");
-    let hasToolbar = defaultToolbarPosition && !defaultToolbarPosition.isAddonBar;
-    let hasAddonBar = defaultToolbarPosition && defaultToolbarPosition.isAddonBar;
+    let hasToolbar = defaultToolbarPosition;
     let hasStatusBar = statusbarPosition;
-    hideElement(prefix + "showinaddonbar", !hasAddonBar || prefix == "abp-toolbar-");
     hideElement(prefix + "showintoolbar", !hasToolbar || prefix == "abp-toolbar-");
     hideElement(prefix + "showinstatusbar", !hasStatusBar);
-    hideElement(prefix + "iconSettingsSeparator", (prefix == "abp-toolbar-" || (!hasAddonBar && !hasToolbar)) && !hasStatusBar);
+    hideElement(prefix + "iconSettingsSeparator", (prefix == "abp-toolbar-" || !hasToolbar) && !hasStatusBar);
 
-    setChecked(prefix + "showinaddonbar", this.isToolbarIconVisible(window));
-    setChecked(prefix + "showintoolbar", this.isToolbarIconVisible(window));
+    setChecked(prefix + "showintoolbar", this.isToolbarIconVisible());
     setChecked(prefix + "showinstatusbar", Prefs.showinstatusbar);
 
     let {Sync} = require("sync");
     let syncEngine = Sync.getEngine();
     hideElement(prefix + "sync", !syncEngine);
     setChecked(prefix + "sync", syncEngine && syncEngine.enabled);
 
     let defAction = (!window.document.popupNode || window.document.popupNode.id == "abp-toolbarbutton" ?
@@ -1830,20 +1667,20 @@ let UI = exports.UI =
           command.doCommand();
       }
     }
   },
 
   /**
    * Checks whether the toolbar icon is currently displayed.
    */
-  isToolbarIconVisible: function(/**Window*/ window)
+  isToolbarIconVisible: function() /**Boolean*/
   {
-    let button = window.document.getElementById("abp-toolbarbutton");
-    return (button && button.parentNode && button.parentNode.localName == "toolbar" && !button.parentNode.collapsed);
+    let placement = CustomizableUI.getPlacementOfWidget("abp-toolbarbutton");
+    return !!placement;
   },
 
   /**
    * Stores the selected hotkeys, initialized when the user presses a key.
    */
   hotkeys: null,
 
   /**
@@ -1923,22 +1760,19 @@ let UI = exports.UI =
     for each (let id in ["abp-status-contributebutton", "abp-toolbar-contributebutton", "abp-menuitem-contributebutton"])
     {
       let button = window.document.getElementById(id);
       if (button)
         button.hidden = true;
     }
   },
 
-  _showNextNotification: function(notification)
+  _showNextNotification: function()
   {
-    let window = null;
-    for (window in this.applicationWindows)
-      break;
-
+    let window = this.currentWindow;
     if (!window)
       return;
 
     let button = window.document.getElementById("abp-toolbarbutton")
       || window.document.getElementById("abp-status");
     if (!button)
       return;
 
@@ -1996,17 +1830,16 @@ let UI = exports.UI =
 
     let panel = window.document.getElementById("abp-notification");
     panel.openPopup(button, "bottomcenter topcenter", 0, 0, false, false, null);
   }
 };
 UI.onPopupShowing = UI.onPopupShowing.bind(UI);
 UI.onKeyPress = UI.onKeyPress.bind(UI);
 UI.onIconClick = UI.onIconClick.bind(UI);
-UI.onToolbarCustomization = UI.onToolbarCustomization.bind(UI);
 UI.init();
 
 /**
  * List of event handers to be registered for each window. For each event
  * handler the element ID, event and the actual event handler are listed.
  * @type Array
  */
 let eventHandlers = [