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
  * 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/>.
  */
 
 /**
  * @fileOverview Handles notifications.
  */
 
 Cu.import("resource://gre/modules/Services.jsm");
 
 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 TYPES = {
+  information: 0,
+  question: 1,
+  critical: 2
+};
 
 function getNumericalSeverity(notification)
 {
-  let levels = {information: 0, critical: 1};
-  return (notification.severity in levels ? levels[notification.severity] : levels.information);
+  return (notification.type in TYPES ? TYPES[notification.type] : TYPES.information);

[Felix Dahlke, 2014/02/11 10:35:27]

I think we should be backwards compatible here, we won't change the name of this
field on the server right away, and if we do, we can't show notifications to all
users in the migration period.

[Thomas Greiner, 2014/02/11 16:53:31]

Done.

 }
 
 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)
 {
   if (locale in translations)
     return translations[locale];
 
   let languagePart = locale.substring(0, locale.indexOf("-"));
   if (languagePart && languagePart in translations)
     return translations[languagePart];
 
   let defaultLocale = "en-US";
   return translations[defaultLocale];
 }
 
 /**
  * The object providing actual downloading functionality.
  * @type Downloader
  */
 let downloader = null;
+let localData = [];
 
 /**
  * Regularly fetches notifications and decides which to show.
  * @class
  */
 let Notification = exports.Notification =
 {
   /**
    * Called on module startup.
    */
@@ skipping 62 matching lines @@
 
   _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 {Array of Object} notifications active notifications
-   * @return {Object} notification to be shown, or null if there is none
+   * @param {String} url URL to match notifications to
+   * @return {Array of Object} active notifications
    */
-  getNextToShow: function()
+  _getActiveNotifications: 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) ||
                (maxVersionKey in target && Services.vc.compare(version, target[maxVersionKey]) > 0));
-
     }
 
-    if (typeof Prefs.notificationdata.data != "object" || !(Prefs.notificationdata.data.notifications instanceof Array))
-      return null;
+    let remoteData = [];
+    if (typeof Prefs.notificationdata.data == "object" && Prefs.notificationdata.data.notifications instanceof Array)
+      remoteData = Prefs.notificationdata.data.notifications;
 
     if (!(Prefs.notificationdata.shown instanceof Array))
     {
       Prefs.notificationdata.shown = [];
       saveNotificationData();
     }
 
     let {addonName, addonVersion, application, applicationVersion, platform, platformVersion} = require("info");
-    let notifications = Prefs.notificationdata.data.notifications;
-    let notificationToShow = null;
+    let notifications = localData.concat(remoteData);
+    let activeNotifications = [];
     for each (let notification in notifications)
     {
-      if ((typeof notification.severity === "undefined" || notification.severity === "information")
+      if ((typeof notification.type === "undefined" || notification.type === "information" || notification.type === "question")

[Felix Dahlke, 2014/02/11 10:35:27]

I think this would make more sense now:

if ((typeof notification.type === "undefined" || notification.type !==
"warning")

[Thomas Greiner, 2014/02/11 16:53:31]

Done.

           && Prefs.notificationdata.shown.indexOf(notification.id) !== -1)
         continue;
 
+      if (typeof url === "string" || notification.domains instanceof Array)

[Felix Dahlke, 2014/02/11 10:35:27]

notification.domains are not necessarily domains if it's arbitrary filters, so I
think it needs another name. notification.urls? notification.urlFilters?

[Thomas Greiner, 2014/02/11 16:53:31]

Done.

+      {
+        if (typeof url === "string" && notification.domains instanceof Array)
+        {
+          let matcher = new Matcher();

[Thomas Greiner, 2014/02/07 17:29:17]

Let me know if you don't like this approach. However, if we want to create the
matcher beforehand we'd need to store it somewhere and I couldn't find a good
location for that:
- attaching it to the notification would be problematic due to it being stored
in serialized form)
- having the data separately means that we'd need to keep it synchronized with
the notification

[Felix Dahlke, 2014/02/11 10:35:27]

Pretty fine to me, wouldn't like to complicate this without any real performance
issues. And I don't expect any here, from what I see, there's not much overhead.

+          for each (let filter in notification.domains)
+            matcher.add(Filter.fromText(filter));
+          if (!matcher.matchesAny(url, "DOCUMENT", url))
+            continue;
+        }
+        else
+          continue;

[Felix Dahlke, 2014/02/11 10:35:27]

So if the url parameter is passed, we ignore all notifications that don't have
notification.domains? Seems wrong to me.

[Thomas Greiner, 2014/02/11 16:53:31]

This means that some notifications could be shown for any URL. Can you think of
a good use-case for that?

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

Ah, I thought I'd change my mind on this after reviewing lib/ui.js. Indeed, this
function is called twice: Once with and once without URL. So yeah, fine.

+      }
+
       if (notification.targets instanceof Array)
       {
         let match = false;
         for each (let target in notification.targets)
         {
           if (checkTarget(target, "extension", addonName, addonVersion) &&
               checkTarget(target, "application", application, applicationVersion) &&
               checkTarget(target, "platform", platform, platformVersion))
           {
             match = true;
             break;
           }
         }
         if (!match)
           continue;
       }
 
-      if (!notificationToShow
-          || getNumericalSeverity(notification) > getNumericalSeverity(notificationToShow))
-        notificationToShow = notification;
+      activeNotifications.push(notification);

[Felix Dahlke, 2014/02/11 10:35:27]

Why did you change this logic? It looks to me like we're only ever interested in
the notification with the highest priority anyway.

[Thomas Greiner, 2014/02/11 16:53:31]

Done.

     }
 
+    return activeNotifications.sort(function(a, b)
+    {
+      return getNumericalSeverity(b) - getNumericalSeverity(a);
+    });
+  },
+
+  /**
+   * Determines which notification is to be shown next.
+   * @param {String} url URL to match notification to (optional)
+   * @return {Object} notification to be shown, or null if there is none
+   */
+  getNextToShow: function(url)
+  {
+    let [notificationToShow] = this._getActiveNotifications(url);
     if (notificationToShow && "id" in notificationToShow)
     {
-      Prefs.notificationdata.shown.push(notificationToShow.id);
-      saveNotificationData();
+      if (notificationToShow.type !== "question")
+        this.markAsShown(notificationToShow.id);
     }
+    else
+      notificationToShow = null;
 
     return notificationToShow;
   },
 
+  markAsShown: function(id)
+  {
+    if (Prefs.notificationdata.shown.indexOf(id) > -1)
+      return;
+
+    Prefs.notificationdata.shown.push(id);
+    saveNotificationData();
+  },
+
   /**
    * Localizes the texts of the supplied notification.
    * @param {Object} notification notification to translate
    * @param {String} locale the target locale (optional, defaults to the
    *                        application locale)
    * @return {Object} the translated texts
    */
   getLocalizedTexts: function(notification, locale)
   {
     locale = locale || Utils.appLocale;
     let textKeys = ["title", "message"];
     let localizedTexts = [];
     for each (let key in textKeys)
     {
       if (key in notification)
-        localizedTexts[key] = localize(notification[key], locale);
+      {
+        if (typeof notification[key] == "string")
+          localizedTexts[key] = notification[key];
+        else
+          localizedTexts[key] = localize(notification[key], locale);
+      }
     }
     return localizedTexts;
+  },
+
+  /**
+   * Adds a local notification.
+   * @param {Object} notification notification to add
+   */
+  addNotification: function(notification)
+  {
+    if (localData.indexOf(notification) == -1)
+      localData.push(notification);
+  },
+
+  /**
+   * Removes an existing local notification.
+   * @param {Object} notification notification to remove
+   */
+  removeNotification: function(notification)
+  {
+    if (localData.indexOf(notification) > -1)

[Felix Dahlke, 2014/02/11 10:35:27]

Nit: Save the index to a temp instead of calling indexOf twice.

[Thomas Greiner, 2014/02/11 16:53:31]

Done.

+      localData.splice(localData.indexOf(notification), 1);
   }
 };
 Notification.init();