Issue 293 - AdBlock button is slow to appear on toolbar after launching Firefox

lib/ui.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 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 424 matching lines @@
 
         this.showNextNotification(subject.location.href);
       }.bind(UI)
     };
     Services.obs.addObserver(documentCreationObserver, "content-document-global-created", false);
     onShutdown.add(function()
     {
       Services.obs.removeObserver(documentCreationObserver, "content-document-global-created", false);
     });
 
-    if (!this.firstRunDone)
-    {
-      // Execute first-run actions if a window is open already, otherwise it
-      // will happen in applyToWindow() when a window is opened.
-      let window = this.currentWindow;
-      if (window)
-        this.firstRunActions(window);
-    }
+    // Execute first-run actions if a window is open already, otherwise it
+    // will happen in applyToWindow() when a window is opened.
+    this.firstRunActions(this.currentWindow);
   },
 
   addToolbarButton: function()
   {
     let {WindowObserver} = require("windowObserver");
     new WindowObserver(this);
 
     let {defaultToolbarPosition} = require("appSupport");
     if ("abp-toolbarbutton" in this.overlay && defaultToolbarPosition)
     {
@@ skipping 13 matching lines @@
         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)
         {
           let node = document.importNode(this.overlay["abp-toolbarbutton"], true);
           node.addEventListener("click", this.onIconClick, false);
           node.addEventListener("command", this.onIconClick, false);
-          if ("addClass" in defaultToolbarPosition)
-            node.classList.add(defaultToolbarPosition.addClass);
           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),
       });
       onShutdown.add(CustomizableUI.destroyWidget.bind(CustomizableUI, "abp-toolbarbutton"));
     }
   },
 
   firstRunActions: function(window)

[Thomas Greiner, 2014/05/23 16:10:31]

Is there a case in which firstRunActions are / could be run without
this.firstRunDone being true? If not, why not include it in this function
directly instead of right before calling firstRunActions?

[Wladimir Palant, 2014/05/26 11:16:52]

This was pretty much premature optimization on my end, moved the check inside
the function.

   {
-    if (FilterStorage._loading)
+    if (this.firstRunDone || !window || FilterStorage._loading)
       return;
 
     this.firstRunDone = true;
 
     let {addonVersion} = require("info");
     let prevVersion = Prefs.currentVersion;
     if (prevVersion != addonVersion)
     {
       Prefs.currentVersion = addonVersion;
       this.addSubscription(window, prevVersion);
@@ skipping 64 matching lines @@
       this.updateIconState(window, window.document.getElementById("abp-toolbarbutton"));
     }.bind(this));
     addBrowserClickListener(window, this.onBrowserClick.bind(this, window));
 
     window.document.getElementById("abp-notification-close").addEventListener("command", function(event)
     {
       window.document.getElementById("abp-notification").hidePopup();
     }, false);
 
     // First-run actions?
-    if (!this.firstRunDone)
-      this.firstRunActions(window);
+    this.firstRunActions(window);
 
     // Some people actually switch off browser.frames.enabled and are surprised
     // that things stop working...
     window.QueryInterface(Ci.nsIInterfaceRequestor)
           .getInterface(Ci.nsIWebNavigation)
           .QueryInterface(Ci.nsIDocShell)
           .allowSubframes = true;
   },
 
   /**
@@ skipping 494 matching lines @@
       return;
 
     let state = (Prefs.enabled ? "active" : "disabled");
     if (state == "active")
     {
       let location = this.getCurrentLocation(window);
       if (location && Policy.isWhitelisted(location.spec))
         state = "whitelisted";
     }
 
+    let popupId = "abp-status-popup";
     if (icon.localName == "statusbarpanel")
     {
       if (Prefs.defaultstatusbaraction == 0)
-        icon.setAttribute("popup", icon.getAttribute("context"));
+      {
+        icon.setAttribute("popup", popupId);
+        icon.removeAttribute("context");
+      }
       else
+      {
         icon.removeAttribute("popup");
+        icon.setAttribute("context", popupId);
+      }
     }
     else
     {
       if (Prefs.defaulttoolbaraction == 0)
+      {
         icon.setAttribute("type", "menu");
+        icon.removeAttribute("context");
+      }
       else
+      {
         icon.setAttribute("type", "menu-button");
+        icon.setAttribute("context", popupId);
+      }
     }
 
     icon.setAttribute("abpstate", state);
   },
 
   /**
    * Shows or hides status bar icons in all windows, according to pref.
    */
   updateStatusbarIcon: function(/**Window*/ window)
   {
@@ skipping 817 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);
 });