Fixed miscellaneous issues with anti-adblock notification port

background.js

Index: background.js
===================================================================
--- a/background.js
+++ b/background.js
@@ -18,7 +18,6 @@
 with(require("filterClasses"))
 {
   this.Filter = Filter;
-  this.ActiveFilter = ActiveFilter;
   this.RegExpFilter = RegExpFilter;
   this.BlockingFilter = BlockingFilter;
   this.WhitelistFilter = WhitelistFilter;
@@ -47,6 +46,12 @@
 RegExpFilter.typeMap.OBJECT_SUBREQUEST = RegExpFilter.typeMap.OBJECT;
 RegExpFilter.typeMap.MEDIA = RegExpFilter.typeMap.FONT = RegExpFilter.typeMap.OTHER;
 
+// Chrome on Linux does not fully support chrome.notifications until version 35
+// https://code.google.com/p/chromium/issues/detail?id=291485
+var canUseChromeNotifications = require("info").platform == "chromium"
+  && "notifications" in chrome
+  && (navigator.platform.indexOf("Linux") == -1 || parseInt(require("info").applicationVersion) > 34);
+
 var isFirstRun = false;
 var seenDataCorruption = false;
 require("filterNotifier").FilterNotifier.addListener(function(action)
@@ -65,6 +70,8 @@
         addSubscription(prevVersion);
     }
 
+    if (canUseChromeNotifications)
+      initChromeNotifications();
     initAntiAdblockNotification();
   }
 
@@ -314,7 +321,7 @@
   {
     if (animateIcon)
       iconAnimation.stop();
-    activeNotification = null;
+    notificationClosed();
   };
   if (animateIcon)
     iconAnimation.update(activeNotification.type);
@@ -334,23 +341,52 @@
   }
 }
 
-function notificationButtonClick(id, index)
+function notificationButtonClick(buttonIndex)
 {
   if (activeNotification.type === "question")
   {
-    Notification.triggerQuestionListeners(activeNotification.id, index === 0);
+    Notification.triggerQuestionListeners(activeNotification.id, buttonIndex === 0);
     Notification.markAsShown(activeNotification.id);
     activeNotification.onClicked();
   }
-  else if (activeNotification.links && activeNotification.links[index])
+  else if (activeNotification.links && activeNotification.links[buttonIndex])
   {
     ext.windows.getLastFocused(function(win)
     {
-      win.openTab(Utils.getDocLink(activeNotification.links[index]));
+      win.openTab(Utils.getDocLink(activeNotification.links[buttonIndex]));
     });
   }
 }
 
+function notificationClosed()
+{
+  activeNotification = null;
+}
+
+function initChromeNotifications()
+{
+  // Chrome hides notifications in notification center when clicked so we need to close them
+  function clearActiveNotification(notificationId)
+  {
+    if (activeNotification && activeNotification.type != "question")

[Felix Dahlke, 2014/03/28 16:16:39]

Why check for the notification type here? Seems to me this should be done for
all notifications that trigger a chrome.notification to be shown.

[Thomas Greiner, 2014/03/28 17:26:51]

What this function does is remove the notification from the notification center
- not close the notification if that's what you thought. It'd be nice to use the
notification center for non-interactive notifications but we can also go with
your suggestion for now.

Additional context:

While it's no problem that regular notifications are being kept in there, in
case users want to check for them, it's problematic for URL-specific
notifications because we can only show a notification again if it's not hidden
in the notification center.

In addition to that, we only save the last notification which means that
interacting with old notifications in the notificiation center wouldn't do
anything. So actually, we have three options:

1) Don't show any notifications in the notification center.

2) Change the check to the one below to only show notifications in the
notification center which do not offer any interaction.

if (activeNotification && (activeNotification.type == "question" || "links" in
activeNotification))

3) Keep all notifications in memory until they are properly closed to make use
of the notification center.

While I chose to go with (2) to at least keep non-interactive notifications in
the notification center, I could also live with (1). (3) would require more
effort to implement and is therefore not an option for now IMHO.

[Felix Dahlke, 2014/03/28 17:33:46]

Ah, no I get it. No it's fine let's go with 2 if you prefer that.

[Thomas Greiner, 2014/03/28 18:23:17]

For completeness' sake, what I meant for (2) was:

if (activeNotification && activeNotification.type != "question" && !("links" in
activeNotification))

+      return;
+    
+    chrome.notifications.clear(notificationId, function(wasCleared)
+    {
+      if (wasCleared)
+        notificationClosed();
+    });
+  }
+
+  chrome.notifications.onButtonClicked.addListener(function(notificationId, buttonIndex)
+  {
+    notificationButtonClick(buttonIndex);
+    clearActiveNotification(notificationId);

[Thomas Greiner, 2014/03/28 15:57:08]

I noticed that even notifications that were dismissed by clicking on one of the
buttons were only hidden in the notification center and not cleared so I added
this here.

+  });
+  chrome.notifications.onClicked.addListener(clearActiveNotification);
+  chrome.notifications.onClosed.addListener(notificationClosed);
+}
+
 function showNotification(notification)
 {
   if (activeNotification && activeNotification.id === notification.id)
@@ -374,9 +410,7 @@
     var iconUrl = ext.getURL("icons/abp-128.png");
     var hasLinks = activeNotification.links && activeNotification.links.length > 0;
     
-    // Chrome on Linux does not fully support chrome.notifications yet
-    // https://code.google.com/p/chromium/issues/detail?id=291485
-    if (require("info").platform == "chromium" && "notifications" in chrome && navigator.platform.indexOf("Linux") == -1)
+    if (canUseChromeNotifications)
     {
       var opts = {
         type: "basic",
@@ -401,7 +435,6 @@
       }
       
       chrome.notifications.create("", opts, function() {});
-      chrome.notifications.onButtonClicked.addListener(notificationButtonClick);
     }
     else if (hasWebkitNotifications && "createNotification" in webkitNotifications && activeNotification.type !== "question")
     {
@@ -411,6 +444,7 @@
       var notification = webkitNotifications.createNotification(iconUrl, title, message);
       notification.show();
       notification.addEventListener("click", openNotificationLinks, false);
+      notification.addEventListener("close", notificationClosed, false);
     }
     else
     {