Issue 2193 - Added notification opt-out (Firefox)

lib/ui.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 112 matching lines @@
       {
         UI.toggleSaveStats(doc.defaultView);
         this.value = Prefs.savestats;
       });
 
       hideElement("adblockplus-shownotifications", !Prefs.notifications_showui);
       setChecked("adblockplus-shownotifications", Prefs.notifications_ignoredcategories.indexOf("*") == -1);
       addCommandHandler("adblockplus-shownotifications", function()
       {
         Notification.toggleIgnoreCategory("*");
         this.value = (Prefs.notifications_ignoredcategories.indexOf("*") == -1);

[Wladimir Palant, 2015/06/26 13:13:09]

Nit: toggleIgnoreCategory() should really return the new value, not terribly
important however.

[Thomas Greiner, 2015/06/26 15:32:48]

Totally makes sense. I created #2727 for that now so that we can take care of it
later on.

       });
 
       let hasAcceptableAds = FilterStorage.subscriptions.some((subscription) => subscription instanceof DownloadableSubscription &&
         subscription.url == Prefs.subscriptions_exceptionsurl);
       setChecked("adblockplus-acceptableAds", hasAcceptableAds);
       addCommandHandler("adblockplus-acceptableAds", function()
       {
         this.value = UI.toggleAcceptableAds();
       });
 
@@ skipping 453 matching lines @@
 
     let notificationPanel = window.document.getElementById("abp-notification");
     notificationPanel.addEventListener("command", function(event)
     {
       switch (event.target.id)
       {
         case "abp-notification-close":
           notificationPanel.classList.add("abp-closing");
           break;
         case "abp-notification-optout":
           Notification.toggleIgnoreCategory("*", true);

[Wladimir Palant, 2015/06/26 13:13:09]

I'm not a huge fan of fallthrough in switch statements, it's very easy to miss.
At the very least we should adopt the suggestion from
https://wiki.mozilla.org/JavaScript:SpiderMonkey:Coding_Style and have /* FALL
THROUGH */ instead of the missing break statement here.

[Thomas Greiner, 2015/06/26 15:32:48]

Done. I guess it would make sense to add this to
https://adblockplus.org/en/coding-style#javascript then for future reference.

Also note Sebastian's comment on the Platform review:
https://codereview.adblockplus.org/29321084/diff/29321085/notification.js#new...
Therefore  I'd personally also add that "break" should always explicitly be set
in switch-statements.

+          /* FALL THROUGH */
         case "abp-notification-hide":
           notificationPanel.hidePopup();
           break;
       }
-    }, true);

[Wladimir Palant, 2015/06/26 13:13:09]

Why true? We don't usually add capturing event listeners.

[Thomas Greiner, 2015/06/26 15:32:48]

Done.

+    }, false);
 
     // First-run actions?
     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 1355 matching lines @@
   ["abp-command-contribute-hide", "command", UI.hideContributeButton.bind(UI)],
   ["abp-command-toggleshownotifications", "command", Notification.toggleIgnoreCategory.bind(Notification, "*", null)]
 ];
 
 onShutdown.add(function()
 {
   for (let window of UI.applicationWindows)
     if (UI.isBottombarOpen(window))
       UI.toggleBottombar(window);
 });