Issue 3024 - Added opt-out to Chrome notifications

lib/notificationHelper.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 42 matching lines @@
   activeNotification.onClicked = function()
   {
     if (animateIcon)
       stopIconAnimation();
     notificationClosed();
   };
   if (animateIcon)
     startIconAnimation(activeNotification.type);
 }
 
-function getNotificationButtons(message)
-{
-  activeButtons = [];

[Sebastian Noack, 2015/09/11 14:15:39]

I find it kinda unexpected that a function that is supposed to "get" something,
changes global state. Also the function would be easier to debug, if you could
call it from the console, without these unexpcted side effects. So why don't you
set the global variable in the calling code?

[Thomas Greiner, 2015/09/11 14:43:58]

Done. The idea was to ensure that `activeButtons` is always kept in sync but I
do see your point.

-  if (activeNotification.type == "question")
-  {
-    activeButtons.push({
+function getNotificationButtons(notificationType, message)
+{
+  let buttons = [];
+  if (notificationType == "question")
+  {
+    buttons.push({
       type: "question",
       title: ext.i18n.getMessage("overlay_notification_button_yes")
     });
-    activeButtons.push({
+    buttons.push({
       type: "question",
       title: ext.i18n.getMessage("overlay_notification_button_no")
     });
   }
   else
   {
     let regex = /<a>(.*?)<\/a>/g;
     let match;
     while (match = regex.exec(message))
     {
-      activeButtons.push({
+      buttons.push({
         type: "link",
         title: match[1]
       });
     }
 
     // Chrome only allows two notification buttons so we need to fall back
     // to a single button to open all links if there are more than two.
-    let maxButtons = (activeNotification.type == "critical") ? 2 : 1;
-    if (activeButtons.length > maxButtons)
-    {
-      activeButtons = [
+    let maxButtons = (notificationType == "critical") ? 2 : 1;
+    if (buttons.length > maxButtons)
+    {
+      buttons = [
         {
           type: "open-all",
           title: ext.i18n.getMessage("notification_open_all")
         }
       ];
     }
-    if (activeNotification.type != "critical")
-    {
-      activeButtons.push({
+    if (notificationType != "critical")
+    {
+      buttons.push({
         type: "configure",
         title: ext.i18n.getMessage("notification_configure")
       });
     }
   }
 
-  return activeButtons.map(button => ({title: button.title}));
+  return buttons;
 }
 
 function openNotificationLinks()
 {
   if (activeNotification.links)
   {
     for (let link of activeNotification.links)

[Sebastian Noack, 2015/09/11 14:51:00]

This one is unrelated.

       ext.pages.open(Utils.getDocLink(link));
   }
 }
 
 function notificationButtonClick(buttonIndex)
 {
   switch (activeButtons[buttonIndex].type)
   {
     case "link":
       ext.pages.open(Utils.getDocLink(activeNotification.links[buttonIndex]));
@@ skipping 73 matching lines @@
   if (shouldDisplay("notification", activeNotification.type))
   {
     let texts = NotificationStorage.getLocalizedTexts(notification);
     let title = texts.title || "";
     let message = texts.message ? texts.message.replace(/<\/?(a|strong)>/g, "") : "";
     let iconUrl = ext.getURL("icons/detailed/abp-128.png");
     let linkCount = (activeNotification.links || []).length;
 
     if (canUseChromeNotifications)
     {
+      activeButtons = getNotificationButtons(activeNotification.type, texts.message);
       let opts = {
         type: "basic",
         title: title,
         message: message,
-        buttons: getNotificationButtons(texts.message),
+        buttons: activeButtons.map(button => ({title: button.title})),
         priority: 2 // We use the highest priority to prevent the notification from closing automatically
       };
 
       imgToBase64(iconUrl, function(iconData)
       {
         opts.iconUrl = iconData;
         chrome.notifications.create("", opts, function() {});
       });
     }
     else if ("Notification" in window && activeNotification.type != "question")
@@ skipping 56 matching lines @@
 /**
  * Gets the active notification to be shown if any.
  *
  * @return {?object}
  */
 exports.getActiveNotification = function()
 {
   return activeNotification;
 };
 
 let shouldDisplay =

[Sebastian Noack, 2015/09/11 14:51:00]

This one is unrelated.

 /**
  * 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 = function(method, notificationType)
 {
   let methods = displayMethods[notificationType] || defaultDisplayMethods;
   return methods.indexOf(method) > -1;
 }
 
 NotificationStorage.addShowListener(showNotification);