issue 1435 - Port popup.html from Chrome/Safari/Opera to 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 368 matching lines @@
     }
 
     if ("abp-status-popup" in this.overlay)
     {
       let menuSource = this.overlay["abp-status-popup"];
       delete this.overlay["abp-status-popup"];
 
       if (this.overlay.all.length)
         this.overlay.all[0].appendChild(menuSource);
       if ("abp-toolbarbutton" in this.overlay)
         this.overlay["abp-toolbarbutton"].appendChild(fixId(menuSource.cloneNode(true), "abp-toolbar"));

[Wladimir Palant, 2014/10/23 21:57:40]

Shouldn't this go away as well, along with the fixId() function?

[saroyanm, 2014/10/27 21:49:20]

In current review we are using abp-status-popup for showing panel during right
click when the default left click action is changed using customization addon
and abp-toolbar-popup, but in future we will change that behavior and use
openPopup method to show panel, both for left and right click, so with that
review this part shouldn't make that much confusion.

     }
   },
 
   /**
    * Gets called once the initialization is finished and Adblock Plus elements
    * can be added to the UI.
    */
   initDone: function()
   {
     // The icon might be added already, make sure its state is correct
@@ skipping 887 matching lines @@
       this.fillContentContextMenu(popup);
     }
     else if (popup.id == "abp-tooltip")
       this.fillIconTooltip(event, popup.ownerDocument.defaultView);
     else
     {
       let match = /^(abp-(toolbar|status|menuitem)-)popup$/.exec(popup.id);
       if (match)
       {
         let [, prefix, name] = match;
+        let window = popup.ownerDocument.defaultView;
         if (name == "toolbar" || name == "status")
         {
-          let window = popup.ownerDocument.defaultView;

[Wladimir Palant, 2014/10/23 21:57:40]

How about defining the window variable before the |if| block and using it for
the |else| case as well?

[saroyanm, 2014/10/27 21:49:20]

Oops.

           let browser = window.document.getElementById(prefix + "popup-browser");
           browser.setAttribute("src", "chrome://adblockplus/content/ui/popup.html");
         }
         else
-          this.fillIconMenu(event, popup.ownerDocument.defaultView, prefix);
+          this.fillIconMenu(event, window, prefix);
       }
     }
   },
 
    /**
    * Called when some pop-up in the application window is being hidden
    */
   onPopupHiding: function(/**Event*/ event)
   {
     if (event.defaultPrevented)
       return;
 
     let popup = event.originalTarget;
     if (popup.id == "abp-toolbar-popup")

[Wladimir Palant, 2014/10/23 21:57:40]

What about abp-status-popup?

     {
       let window = popup.ownerDocument.defaultView;
       let browser = window.document.getElementById("abp-toolbar-popup-browser");
       browser.setAttribute("src", "about:blank");
     }
   },
 
   /**
    * Handles click on toolbar and status bar icons.
    */
@@ skipping 649 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);
 });