issue 1435 - Port popup.html from Chrome/Safari/Opera to Firefox

chrome/content/ui/popup.js

 /*

[Thomas Greiner, 2014/10/13 13:18:24]

Keep in mind that at some point we need to merge this version with the one in
adblockpluschrome. I'll leave it up to you whether you prefer rebasing the
changes now or as part of the backport.

[saroyanm, 2014/10/16 11:26:15]

I guess would be nice to do it during backport, or after all #181 related
tickets are covered, because popup.js and popup.html will again be changed by
#181 related subtickets.

[Thomas Greiner, 2014/10/16 13:39:44]

That's fine with me.

  * 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 var backgroundPage = ext.backgroundPage.getWindow();
-window["openOptions"] = backgroundPage["openOptions"];
+var openOptions = backgroundPage.openOptions;
 var page = null;

[Thomas Greiner, 2014/10/13 13:18:24]

I assume that in one of the later reviews this variable needs to be global but
in this one it doesn't.

[saroyanm, 2014/10/16 11:26:15]

I tried to keep it like in Chrome/safari/opera version, to make less confusion
by keeping in mind backport and yes that variable should be global after #1436
ticket.
Let me know if it's preferable to change the implementation for current review
and make it global in #1436 ticket review.

[Thomas Greiner, 2014/10/16 13:39:44]

No, that's not necessary then.

 
 function init()
 {
   ext.pages.query({active: true, lastFocusedWindow: true}, function(pages)
   {
     page = pages[0];
-
-    document.body.classList.toggle("local", !/^https?:\/\//.test(page.url));

[Thomas Greiner, 2014/10/13 13:18:24]

This will throw an error if the pages array is empty.

[saroyanm, 2014/10/16 11:26:15]

Done.

-    
     if (page)
     {
+      document.body.classList.toggle("local", !/^https?:\/\//.test(page.url));
+
       // Ask content script whether clickhide is active. If so, show cancel button.
       page.sendMessage({type: "get-clickhide-state"}, function(response)
       {
         if (response && response.active)
           document.body.classList.add("clickhide-active");
       });
     }
   });
 
   // Initialize features
@@ skipping 33 matching lines @@
   }, false);
   menuItem.removeAttribute("hidden");
 }
 
 function initBlockable()
 {
   if (!("openBlockable" in ext))
     return;
 
   var menuItem = document.getElementById("blockable");
-  if (ext.isBlockableOpen())
-    menuItem = document.getElementById("blockable-close");
-
+  menuItem.classList.toggle("open", ext.isBlockableOpen());
   menuItem.addEventListener("click", function()
   {
     ext.openBlockable();
   }, false);
   menuItem.removeAttribute("hidden");
 }
 
 function activateClickHide()
 {
   document.body.classList.add("clickhide-active");
   ext.activateClickHide(true);
 
   // Close the popup after a few seconds, so user doesn't have to
   activateClickHide.timeout = window.setTimeout(ext.closePopup, 5000);
 }
 
 function cancelClickHide()
 {
   if (activateClickHide.timeout)
   {
     window.clearTimeout(activateClickHide.timeout);
     activateClickHide.timeout = null;
   }
   document.body.classList.remove("clickhide-active");
   ext.activateClickHide(false);
 }