Issue 3022 - Implemented new notification type for normal messages

lib/notificationHelper.js

Index: lib/notificationHelper.js
===================================================================
--- a/lib/notificationHelper.js
+++ b/lib/notificationHelper.js
@@ -24,6 +24,12 @@
 let {initAntiAdblockNotification} = require("antiadblockInit");
 
 let activeNotification = null;
+let defaultDisplayMethods = ["popup"];
+let displayMethods = Object.create(null);
+displayMethods.critical = ["icon", "notification", "popup"];
+displayMethods.question = ["notification"];
+displayMethods.normal = ["notification"];
+displayMethods.information = ["icon", "popup"];
 
 // Chrome on Linux does not fully support chrome.notifications until version 35
 // https://code.google.com/p/chromium/issues/detail?id=291485
@@ -42,7 +48,7 @@
 
 function prepareNotificationIconAndPopup()
 {
-  let animateIcon = (activeNotification.type != "question");
+  let animateIcon = shouldDisplay("icon", activeNotification.type);
   activeNotification.onClicked = function()
   {
     if (animateIcon)
@@ -135,7 +141,7 @@
     return;
 
   activeNotification = notification;
-  if (activeNotification.type == "critical" || activeNotification.type == "question")
+  if (shouldDisplay("notification", activeNotification.type))
   {
     let texts = NotificationStorage.getLocalizedTexts(notification);
     let title = texts.title || "";
@@ -239,4 +245,19 @@
   return activeNotification;
 };
 
+/**
+ * Determines whether a given display method should be used for a
+ * specified notification type.
+ * 
+ * @param {string} method Display method: icon, notification or popup
+ * @param {string} notificationType
+ * @return {boolean}
+ */
+exports.shouldDisplay = shouldDisplay;

[Sebastian Noack, 2015/09/11 13:15:46]

I just realized that this way shouldDisplay is documented as static member
rather than a method of the module:
https://adblockplus.org/jsdoc/adblockpluschrome/module-notificationHelper.html

Also see: http://usejsdoc.org/howto-commonjs-modules.html

Mind writing a follow-up patch to fix that? I'd go for:

  let shouldDisplay =
  /**
   * ...
   */
  exports.shouldDisplay = function()
  {
    ...
  };

We already do it that way in other modules.

+function shouldDisplay(method, notificationType)
+{
+  let methods = displayMethods[notificationType] || defaultDisplayMethods;
+  return methods.indexOf(method) > -1;
+}
+
 NotificationStorage.addShowListener(showNotification);