Implemented anti-adblock message notification

lib/notification.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 13 matching lines @@
 let {TimeLine} = require("timeline");
 let {Prefs} = require("prefs");
 let {Downloader, Downloadable, MILLIS_IN_MINUTE, MILLIS_IN_HOUR, MILLIS_IN_DAY} = require("downloader");
 let {Utils} = require("utils");
 let {Matcher} = require("matcher");
 let {Filter} = require("filterClasses");
 
 let INITIAL_DELAY = 12 * MILLIS_IN_MINUTE;
 let CHECK_INTERVAL = 1 * MILLIS_IN_HOUR;
 let EXPIRATION_INTERVAL = 1 * MILLIS_IN_DAY;
-let SEVERITY = {
+let TYPE = {
   information: 0,
   question: 1,
   critical: 2
 };
 
-let listeners = [];
+let listeners = {};
 
 function getNumericalSeverity(notification)
 {
-  return (notification.severity in SEVERITY ? SEVERITY[notification.severity] : SEVERITY.information);

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

I actually thought it's fine to call this TYPE, just that we should be downwards
compatible. i.e.:

let type = "type" in notification ? notification.type : notification.severity;
return (type in TYPE ? TYPE[type] : TYPE.information);

Not really important though, feel free to leave it as it is. Severity still
makes some sort of sense here.

[Thomas Greiner, 2014/02/12 18:28:02]

Done.

+  return (notification.type in TYPE ? TYPE[notification.type] : TYPE.information);
 }
 
 function saveNotificationData()
 {
   // HACK: JSON values aren't saved unless they are assigned a different object.
   Prefs.notificationdata = JSON.parse(JSON.stringify(Prefs.notificationdata));
 }
 
 function localize(translations, locale)
 {
@@ skipping 65 matching lines @@
     Prefs.notificationdata.lastCheck = downloadable.lastCheck;
     Prefs.notificationdata.softExpiration = downloadable.softExpiration;
     Prefs.notificationdata.hardExpiration = downloadable.hardExpiration;
     saveNotificationData();
   },
 
   _onDownloadSuccess: function(downloadable, responseText, errorCallback, redirectCallback)
   {
     try
     {
-      Prefs.notificationdata.data = JSON.parse(responseText);
+      let data = JSON.parse(responseText);
+      for each (let notification in data.notifications)
+      {
+        if ("severity" in notification)
+        {
+          if (!("type" in notification))
+            notification.type = notification.severity;
+          delete notification.severity;
+        }
+      }
+      Prefs.notificationdata.data = data;
     }
     catch (e)
     {
       Cu.reportError(e);
       errorCallback("synchronize_invalid_data");
       return;
     }
 
     Prefs.notificationdata.lastError = 0;
     Prefs.notificationdata.downloadStatus = "synchronize_ok";
     [Prefs.notificationdata.softExpiration, Prefs.notificationdata.hardExpiration] = downloader.processExpirationInterval(EXPIRATION_INTERVAL);
     saveNotificationData();
   },
 
   _onDownloadError: function(downloadable, downloadURL, error, channelStatus, responseStatus, redirectCallback)
   {
     Prefs.notificationdata.lastError = Date.now();
     Prefs.notificationdata.downloadStatus = error;
     saveNotificationData();
   },
 
   /**
    * Determines which notification is to be shown next.
-   * @param {String} url URL to match notifications to

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

Would be nice to point out here that it's optional

[Thomas Greiner, 2014/02/12 18:28:02]

Done.

+   * @param {String} url URL to match notifications to (optional)
    * @return {Object} notification to be shown, or null if there is none
    */
   getNextToShow: function(url)
   {
     function checkTarget(target, parameter, name, version)
     {
       let minVersionKey = parameter + "MinVersion";
       let maxVersionKey = parameter + "MaxVersion";
       return !((parameter in target && target[parameter] != name) ||
                (minVersionKey in target && Services.vc.compare(version, target[minVersionKey]) < 0) ||
@@ skipping 11 matching lines @@
     }
 
     let notifications = localData.concat(remoteData);
     if (notifications.length === 0)
       return null;
 
     let {addonName, addonVersion, application, applicationVersion, platform, platformVersion} = require("info");
     let notificationToShow = null;
     for each (let notification in notifications)
     {
-      if ((typeof notification.severity === "undefined" || notification.severity !== "critical")
+      if ((typeof notification.type === "undefined" || notification.type !== "critical")
           && Prefs.notificationdata.shown.indexOf(notification.id) !== -1)
         continue;
 
       if (typeof url === "string" || notification.urlFilters instanceof Array)
       {
         if (typeof url === "string" && notification.urlFilters instanceof Array)
         {
           let matcher = new Matcher();
           for each (let urlFilter in notification.urlFilters)
             matcher.add(Filter.fromText(urlFilter));
@@ skipping 14 matching lines @@
               checkTarget(target, "platform", platform, platformVersion))
           {
             match = true;
             break;
           }
         }
         if (!match)
           continue;
       }
 
-      if (!notificationToShow || getNumericalSeverity(notification) > getNumericalSeverity(notificationToShow))

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

Can you restore the line break? To avoid unrelated changes.

[Thomas Greiner, 2014/02/12 18:28:02]

Done.

+      if (!notificationToShow
+          || getNumericalSeverity(notification) > getNumericalSeverity(notificationToShow))
         notificationToShow = notification;
     }
 
     if (notificationToShow && "id" in notificationToShow)
     {
-      if (notificationToShow.severity !== "question")
+      if (notificationToShow.type !== "question")
         this.markAsShown(notificationToShow.id);
     }
 
     return notificationToShow;
   },
 
   markAsShown: function(id)
   {
     if (Prefs.notificationdata.shown.indexOf(id) > -1)
       return;
@@ skipping 42 matching lines @@
    * @param {Object} notification notification to remove
    */
   removeNotification: function(notification)
   {
     let index = localData.indexOf(notification);
     if (index > -1)
       localData.splice(index, 1);
   },
 
   /**
-   * Adds a listener
-   */
-  addListener: function(/**function(id, approved)*/ listener)

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

I think it'd make more sense to register listeners per notification ID. Like
this:

addListener: function(/**string*/ id, /**function(approved)*/ listener)
{
  if (!(id in listeners))
    listeners[id] = [];
  if listeners[id].indexOf(listener) === -1)
    listeners[id].push(listener);
}

Also, I think we should call the whole thing "addQuestionListener" for clarity.

[Thomas Greiner, 2014/02/12 18:28:02]

Done. Although from a consistency standpoint I'm more in favor of my approach
(same as for FilterNotifier) but for now it's only used for this specific case
so I can live with your suggestion. We can still change it whenever we need to
in the future.

-  {
-    if (listeners.indexOf(listener) === -1)
-      listeners.push(listener);
-  },
-
-  /**
-   * Removes a listener that was previously added via addListener
-   */
-  removeListener: function(/**function(id, approved)*/ listener)
-  {
-    let index = listeners.indexOf(listener);
+   * Adds a listener for question-type notifications
+   */
+  addQuestionListener: function(/**string*/ id, /**function(approved)*/ listener)
+  {
+    if (!(id in listeners))
+      listeners[id] = [];
+    if (listeners[id].indexOf(listener) === -1)
+      listeners[id].push(listener);
+  },
+
+  /**
+   * Removes a listener that was previously added via addQuestionListener
+   */
+  removeQuestionListener: function(/**string*/ id, /**function(approved)*/ listener)
+  {
+    if (!(id in listeners))
+      return;
+    let index = listeners[id].indexOf(listener);
     if (index > -1)
-      listeners.splice(index, 1);
+      listeners[id].splice(index, 1);
+    if (listeners[id].length === 0)
+      delete listeners[id];
   },
 
   /**
    * Notifies listeners about interactions with a notification
    * @param {String} id notification ID
    * @param {Boolean} approved indicator whether notification has been approved or not
    */
-  triggerListeners: function(id, approved)
-  {
-    for each (let listener in listeners)
-      listener(id, approved);
+  triggerQuestionListeners: function(id, approved)
+  {
+    if (!(id in listeners))
+      return;
+    for each (let listener in listeners[id])
+      listener(approved);
   }
 };
 Notification.init();