Fix: Notification popup is broken

background.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 270 matching lines @@
   });
 }
 
 function prepareNotificationIconAndPopup()
 {
   activeNotification.onClicked = function()
   {
     iconAnimation.stop();
     activeNotification = null;
   };
-
+  
   iconAnimation.update(activeNotification.severity);

[Felix Dahlke, 2014/02/27 14:10:39]

Nit: Probably whitespace differences, this shouldn't register as a diff.

 }
 
 function openNotificationLinks() 
 {
   if (activeNotification.links)
   {
     activeNotification.links.forEach(function(link)
     {
       ext.windows.getLastFocused(function(win)
       {
         win.openTab(Utils.getDocLink(link));
       });
     });
   }
-  prepareNotificationIconAndPopup();
 }
 
 function notificationButtonClick(id, index)
 {
   if (activeNotification.links && activeNotification.links[index])
   {
     ext.windows.getLastFocused(function(win)
     {
       win.openTab(Utils.getDocLink(activeNotification.links[index]));
     });
   }
-  prepareNotificationIconAndPopup();
 }
 
 function showNotification(notification)
 {
   activeNotification = notification;
   if (activeNotification.severity === "critical")
   {
     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);

[Thomas Greiner, 2014/02/21 15:54:52]

Shouldn't this notification's message also include a similar text to
confirm_window_notification since it's also opening all links when clicking on
it?

However, there is no "OK" button on this one so we probably can't reuse the same
one for this which is a pity.

       notification.show();
       notification.addEventListener("close", prepareNotificationIconAndPopup, false);
       notification.addEventListener("click", openNotificationLinks, false);
     }
     else
     {
       var message = title + "\n" + message;
-      if (activeNotification.links && activeNotification.links.length > 0)
-      {
-        message += "\n\n" + ext.i18n.getMessage("confirm_window_notification");
-      }
-      var notification = confirm(message);
-      if (notification == true)
+      if (hasLinks)
+        message += "\n\n" + ext.i18n.getMessage("notification_with_buttons");
+        
+      if (confirm(message))

[Thomas Greiner, 2014/02/27 10:07:32]

Nit: Change to…

if (confirm(message))
  openNotificationLinks();
prepareNotificationIconAndPopup();

+      {
         openNotificationLinks();
+        prepareNotificationIconAndPopup();
+      }
+      else
+        prepareNotificationIconAndPopup();
     }
   }
   else
     prepareNotificationIconAndPopup();
 }
 
 ext.onMessage.addListener(function (msg, sender, sendResponse)
 {
   switch (msg.type)
   {
@@ skipping 105 matching lines @@
   tab.sendMessage({type: "clickhide-deactivate"});
   refreshIconAndContextMenu(tab);
 });
 
 setTimeout(function()
 {
   var notificationToShow = Notification.getNextToShow();
   if (notificationToShow)
     showNotification(notificationToShow);
 }, 3 * 60 * 1000);