Implemented anti-adblock message notification

lib/ui.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 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 378 matching lines @@
       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"));
       if ("abp-menuitem" in this.overlay)
         this.overlay["abp-menuitem"].appendChild(fixId(menuSource.cloneNode(true), "abp-menuitem"));
     }
+  },
+
+  _initAntiAdblockNotification: function()
+  {
+    let notification = {
+      id: "antiadblock",
+      type: "question",
+      title: Utils.getString("notification_antiadblock_title"),
+      message: Utils.getString("notification_antiadblock_message"),
+      urlFilters: []
+    };
+
+    function notificationListener(approved)
+    {
+      let subscription = Subscription.fromURL(Prefs.subscriptions_antiadblockurl);
+      if (subscription.url in FilterStorage.knownSubscriptions)
+        subscription.disabled = !approved;
+    }
+
+    function addAntiAdblockNotification(subscription)
+    {
+      let urlFilters = [];
+      for each (let filter in subscription.filters)
+        if (filter instanceof ActiveFilter)
+          for (let domain in filter.domains)
+            if (domain && urlFilters.indexOf(domain) == -1)
+              urlFilters.push(domain);
+      notification.urlFilters = urlFilters;
+      Notification.addNotification(notification);
+      Notification.addQuestionListener(notification.id, notificationListener);
+    }
+
+    function removeAntiAdblockNotification()
+    {
+      Notification.removeNotification(notification);
+      Notification.removeQuestionListener(notification.id, notificationListener);
+    }
+
+    let subscription = Subscription.fromURL(Prefs.subscriptions_antiadblockurl);
+    if (subscription.lastDownload && subscription.disabled)
+      addAntiAdblockNotification(subscription);
+
+    FilterNotifier.addListener(function(action, value, newItem, oldItem)
+    {
+      if (!/^subscription\.(updated|removed|disabled)$/.test(action) || value.url != Prefs.subscriptions_antiadblockurl)
+        return;
+
+      if (action == "subscription.updated")
+        addAntiAdblockNotification(value);
+      else if (action == "subscription.removed" || (action == "subscription.disabled" && !value.disabled))
+        removeAntiAdblockNotification();
+    });
+
+    var documentCreationObserver = {
+      observe: function(subject, topic, data) 
+      {
+        if (!(subject instanceof Ci.nsIDOMWindow))
+          return;
+        
+        this._showNextNotification(subject.location.href);
+      }.bind(this)
+    };
+    Services.obs.addObserver(documentCreationObserver, "content-document-global-created", false);
+    onShutdown.add(function()
+    {
+      Services.obs.removeObserver(documentCreationObserver, "content-document-global-created", false);
+    });
   },
 
   /**
    * 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);
@@ skipping 114 matching lines @@
       this.updateIconState(window, window.document.getElementById("abp-status"));
       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);
 
     // Add "anti-adblock messages" notification

[Felix Dahlke, 2014/02/11 17:57:17]

I think all the anti adblock message specific code should go to its own function
(initAntiAdblockMessageBlocking?) - it's a lot of code.

[Thomas Greiner, 2014/02/12 11:58:49]

Done.

-    function getURLFiltersFromSubscription(subscription)
-    {
-      let urlFilters = [];
-      for each (let filter in subscription.filters)
-        if (filter instanceof ActiveFilter)
-          for (let domain in filter.domains)
-            if (domain && urlFilters.indexOf(domain) == -1)
-              urlFilters.push(domain);
-      return urlFilters;
-    }
-
-    let antiadblockNotification = {
-      id: "antiadblock",
-      severity: "question",
-      title: Utils.getString("notification_antiadblock_title"),
-      message: Utils.getString("notification_antiadblock_message"),
-      urlFilters: [],
-      onApproved: function()

[Felix Dahlke, 2014/02/11 17:57:17]

I really don't think we should be able to put code directly into the
notification - it's a huge security issue.

How about this: We put a map of notification listeners in notification.js, and
add addListener/removeListener functions, maybe like this:

Notification.addListener("antiadblock", function(approved)
{
  ...
});

[Thomas Greiner, 2014/02/12 11:58:49]

You cannot include functions in JSON so they can only be added locally. But I
prefer your approach so I changed it.

[Felix Dahlke, 2014/02/12 15:09:27]

Oh that's right of course, wouldn't work anyway.

-      {
-        let subscription = Subscription.fromURL(Prefs.subscriptions_antiadblockurl);
-        if (subscription.url in FilterStorage.knownSubscriptions)
-          subscription.disabled = false;
-      },
-      onDeclined: function()
-      {
-        let subscription = Subscription.fromURL(Prefs.subscriptions_antiadblockurl);
-        if (subscription.url in FilterStorage.knownSubscriptions)
-          subscription.disabled = true;
-      }
-    };
-    let antiadblockSubscription = Subscription.fromURL(Prefs.subscriptions_antiadblockurl);
-    if (antiadblockSubscription.lastDownload && antiadblockSubscription.disabled)
-    {
-      antiadblockNotification.urlFilters = getURLFiltersFromSubscription(antiadblockSubscription);
-      Notification.addNotification(antiadblockNotification);
-    }
-    FilterNotifier.addListener(function(action, value, newItem, oldItem)
-    {
-      if (!/^subscription\.(updated|removed|disabled)$/.test(action) || value.url != Prefs.subscriptions_antiadblockurl)
-        return;
-
-      if (action == "subscription.updated")
-      {
-        antiadblockNotification.urlFilters = getURLFiltersFromSubscription(value);

[Felix Dahlke, 2014/02/11 17:57:17]

This is repeated above. How about having a
createAntiAdblockBlockingNotification(subscription) function instead?

[Thomas Greiner, 2014/02/12 11:58:49]

Done.

-        Notification.addNotification(antiadblockNotification);
-      }
-      else if (action == "subscription.removed" || (action == "subscription.disabled" && !value.disabled))
-      {
-        Notification.removeNotification(antiadblockNotification);
-      }
-    });
-
-    var httpRequestObserver = {

[Felix Dahlke, 2014/02/11 17:57:17]

Shouldn't it be called documentCreatingObserver or something? This reacts to a
page being loaded effectively, not individual HTTP requests, doesn't it?

[Thomas Greiner, 2014/02/12 11:58:49]

Done.

-      observe: function(subject, topic, data) 
-      {
-        if (!(subject instanceof Ci.nsIDOMWindow))
-          return;
-        
-        this._showNextNotification(subject.location.href);
-      }.bind(this)
-    };
-    Services.obs.addObserver(httpRequestObserver, "content-document-global-created", false);
-    onShutdown.add(function()
-    {
-      Services.obs.removeObserver(httpRequestObserver, "content-document-global-created", false);
-    });
+    this._initAntiAdblockNotification();
 
     // First-run actions?
     if (!this.firstRunDone)
     {
       this.firstRunDone = true;
 
       let {addonVersion} = require("info");
       let prevVersion = Prefs.currentVersion;
       if (prevVersion != addonVersion)
       {
@@ skipping 226 matching lines @@
       {
         subscription.title = "Allow non-intrusive advertising";
         FilterStorage.addSubscription(subscription);
         if (subscription instanceof DownloadableSubscription && !subscription.lastDownload)
           Synchronizer.execute(subscription);
       }
       else
         addAcceptable = false;
     }
 
-    // Add "anti-adblock messages" subscription for new users and users updating from old ABP versions.
-    if (Services.vc.compare(prevVersion, "2.5") < 0)

[Felix Dahlke, 2014/02/11 17:57:17]

I think we should do this for everyone, no matter what version.

[Thomas Greiner, 2014/02/12 11:58:49]

Done.

-    {
-      let subscription = Subscription.fromURL(Prefs.subscriptions_antiadblockurl);
-      if (subscription)
-      {
-        subscription.disabled = true;
-        FilterStorage.addSubscription(subscription);
-        if (subscription instanceof DownloadableSubscription && !subscription.lastDownload)
-          Synchronizer.execute(subscription);
-      }
+    // Add "anti-adblock messages" subscription
+    let subscription = Subscription.fromURL(Prefs.subscriptions_antiadblockurl);
+    if (subscription)
+    {
+      subscription.disabled = true;
+      FilterStorage.addSubscription(subscription);
+      if (subscription instanceof DownloadableSubscription && !subscription.lastDownload)
+        Synchronizer.execute(subscription);
     }
 
     if (!addSubscription && !addAcceptable)
       return;
 
     function notifyUser()
     {
       let {addTab} = require("appSupport");
       if (addTab)
       {
@@ skipping 1036 matching lines @@
     let messageElement = window.document.getElementById("abp-notification-message");
     messageElement.innerHTML = "";
     let docLinks = [];
     for each (let link in notification.links)
       docLinks.push(Utils.getDocLink(link));
     insertMessage(messageElement, texts.message, docLinks);
 
     messageElement.addEventListener("click", function(event)
     {
       let link = event.target;
-      while (!link || link === messageElement || link.localName !== "a")

[Felix Dahlke, 2014/02/11 17:57:17]

Shouldn't this be an "if"?

Anyway, why aren't we bubbling up anymore?

[Thomas Greiner, 2014/02/12 11:58:49]

Done.

The bubbling lead to the link opening no matter where you clicked on the panel.
I fixed it now in a different way to allow bubbling again.

+      while (link && link !== messageElement && link.localName !== "a")
+        link = link.parentNode;
+      if (!link || link.localName !== "a")
         return;
       event.preventDefault();
       event.stopPropagation();
       this.loadInBrowser(link.href, window);
     }.bind(this));
 
-    if (notification.severity === "question")
-    {
-      function buttonHandler(event)
+    if (notification.type === "question")
+    {
+      function buttonHandler(approved, event)
       {
         event.preventDefault();
         event.stopPropagation();
         panel.hidePopup();
-
-        let action = notification[event.target.getAttribute("action")];
-        if (typeof action === "function")
-          action();
+        Notification.triggerQuestionListeners(notification.id, approved)
         Notification.markAsShown(notification.id);
       }
-      window.document.getElementById("abp-notification-yes").oncommand = buttonHandler;
-      window.document.getElementById("abp-notification-no").oncommand = buttonHandler;
-    }
-
-    panel.setAttribute("class", "abp-" + notification.severity);
-    panel.setAttribute("noautohide", notification.severity === "question");
+      window.document.getElementById("abp-notification-yes").onclick = buttonHandler.bind(null, true);
+      window.document.getElementById("abp-notification-no").onclick = buttonHandler.bind(null, false);
+    }
+
+    panel.setAttribute("class", "abp-" + notification.type);
+    panel.setAttribute("noautohide", notification.type === "question");
     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.init();
 
 /**
  * List of event handers to be registered for each window. For each event
@@ skipping 16 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);
 });