Fix: Notification popup is broken

background.js

Index: background.js
===================================================================
--- a/background.js
+++ b/background.js
@@ -292,16 +292,92 @@
   iconAnimation.update(activeNotification.severity);

[saroyanm, 2014/02/26 15:52:18]

Felix, what you think do we need this function here ? 
Actually not sure what we are using it for ?

[Felix Dahlke, 2014/02/26 17:22:58]

It animates the icon. When a notification arrives, we animate it and put a
message in the bubble. For critical notifications, we also show a desktop
notification.

[saroyanm, 2014/02/26 18:15:34]

Got it.

 }
 
+function openNotificationLinks() 
+{
+  if (activeNotification.links)
+  {
+    activeNotification.links.forEach(function(link)
+    {
+      ext.windows.getLastFocused(function(win)
+      {
+        win.openTab(Utils.getDocLink(link));
+      });
+    });
+  }
+}
+
+function notificationButtonClick(id, index)
+{
+  if (activeNotification.links && activeNotification.links[index])
+  {
+    ext.windows.getLastFocused(function(win)
+    {
+      win.openTab(Utils.getDocLink(activeNotification.links[index]));
+    });
+  }
+}
+
 function showNotification(notification)
 {
   activeNotification = notification;
-
-  if (activeNotification.severity === "critical"
-      && typeof webkitNotifications !== "undefined")
+  if (activeNotification.severity === "critical")
   {
-    var notification = webkitNotifications.createHTMLNotification("notification.html");
-    notification.show();
-    notification.addEventListener("close", prepareNotificationIconAndPopup);
+    var hasWebkitNotifications = typeof webkitNotifications !== "undefined";
+    if (hasWebkitNotifications && "createHTMLNotification" in webkitNotifications)
+    {
+      var notification = webkitNotifications.createHTMLNotification("notification.html");
+      notification.show();
+      notification.addEventListener("close", prepareNotificationIconAndPopup, false);
+      return;
+    }
+    
+    var texts = Notification.getLocalizedTexts(notification);
+    var title = texts.title || "";
+    var message = texts.message ? texts.message.replace(/<\/?(a|strong)>/g, "") : "";
+    var iconUrl = ext.getURL("icons/abp-128.png");
+    var hasLinks = activeNotification.links && activeNotification.links.length > 0;
+    if ("browserNotifications" in ext) 
+    {
+      var opts = {
+        type: "basic",
+        title: title,
+        message: message,
+        iconUrl: iconUrl,
+        buttons: []
+      };
+      var regex = /<a>(.*?)<\/a>/g;
+      var plainMessage = texts.message || "";
+      var match;
+      while (match = regex.exec(plainMessage))
+        opts.buttons.push({title: match[1]});
+      
+      var notification = ext.browserNotifications;
+      notification.create("", opts, function() {});
+      notification.onClosed.addListener(prepareNotificationIconAndPopup);
+      notification.onButtonClicked.addListener(notificationButtonClick);
+    }
+    else if (hasWebkitNotifications && "createNotification" in webkitNotifications)
+    {
+      if (hasLinks)
+        message += " " + ext.i18n.getMessage("notification_without_buttons");
+        
+      var notification = webkitNotifications.createNotification(iconUrl, title, message);
+      notification.show();
+      notification.addEventListener("close", prepareNotificationIconAndPopup, false);
+      notification.addEventListener("click", openNotificationLinks, false);
+    }
+    else
+    {
+      var message = title + "\n" + message;
+      if (hasLinks)
+        message += "\n\n" + ext.i18n.getMessage("notification_with_buttons");
+        
+      var notification = confirm(message);

[Felix Dahlke, 2014/02/26 17:22:58]

This is a boolean, was does "notification" mean here? How about something like
"openLinks"? Or you could just put the confirm() call in the condition below.

+      if (notification == true)

[Felix Dahlke, 2014/02/26 17:22:58]

== true is redundant

+        openNotificationLinks();
+      else
+        prepareNotificationIconAndPopup();

[Felix Dahlke, 2014/02/26 17:22:58]

We should call prepareNotificationIconAndPopup either way, not in the else case.

[saroyanm, 2014/02/26 18:00:29]

The current implementation is done depending on the original implementation, in
the original implementation prepareNotificationIconAndPopup is only call in case
the desktop notification is closed. @Felix Can you please confirm that
prepareNotificationIconAndPopup should be called anytime showNotification method
is called ?

[Felix Dahlke, 2014/02/26 18:02:53]

The else here means that it'd only be called if the user said "no" to opening
all the links. Whether he said yes or no, the notification will be closed here,
and we should call prepareNotificationIconAndPopup when the notification gets
closed.

+    }
   }
   else
     prepareNotificationIconAndPopup();