Issue 5539 - Implement "Acceptable Ads notification"

new-options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 354 matching lines @@
   ]);
   collections.filterLists = new Collection([
     {
       id: "all-filter-lists-table",
       useOriginalTitle: true
     }
   ]);
 
   function addSubscription(subscription)
   {
-    let disabled = subscription.disabled;
+    let {disabled} = subscription;

[saroyanm, 2017/09/21 15:50:40]

Note: This is part of eslint error fix. I fixed couple of errors below as well.

     let collection = null;
     if (subscription.recommended)
     {
       if (subscription.recommended == "ads")
       {
         if (disabled == false)
           collection = collections.langs;
 
         collections.allLangs.addItem(subscription);
       }
@@ skipping 234 matching lines @@
           type: "filters.importRaw",
           text: E("custom-filters-raw").value,
           removeExisting: true
         },
         () =>
         {
           setCustomFiltersView("read");
         });
         break;
       case "switch-acceptable-ads":
-        let {value} = element;
+        let value = element.value || element.dataset.value;
         ext.backgroundPage.sendMessage({
           type: value == "privacy" ? "subscriptions.add" :
             "subscriptions.remove",
           url: acceptableAdsPrivacyUrl
         });
         ext.backgroundPage.sendMessage({
           type: value == "ads" ? "subscriptions.add" : "subscriptions.remove",
           url: acceptableAdsUrl
         });
         break;
@@ skipping 192 matching lines @@
     document.body.addEventListener("click", onClick, false);
     document.body.addEventListener("keyup", onKeyUp, false);
     let exampleValue = getMessage("options_whitelist_placeholder_example",
       ["www.example.com"]);
     E("whitelisting-textbox").setAttribute("placeholder", exampleValue);
     E("whitelisting-textbox").addEventListener("keyup", (e) =>
     {
       E("whitelisting-add-button").disabled = !e.target.value;
     }, false);
 
-
     getDocLink("contribute", (link) =>
     {
       E("contribute").href = link;
     });
     getDocLink("acceptable_ads_criteria", (link) =>
     {
       setLinks("enable-aa-description", link);
     });
 
     // Advanced tab
@@ skipping 148 matching lines @@
         document.forms["acceptable-ads"].classList.add("show-dnt-notification");
     }
     document.forms["acceptable-ads"]["acceptable-ads"].value = option;
   }
 
   function isAcceptableAds(url)
   {
     return url == acceptableAdsUrl || url == acceptableAdsPrivacyUrl;
   }
 
+  function hasPrivacyConflict()
+  {
+    let acceptableAdsList = subscriptionsMap[acceptableAdsUrl];
+    let privacyList = null;
+    for (let url in subscriptionsMap)
+    {
+      let subscription = subscriptionsMap[url];
+      if (subscription.recommended == "privacy")
+      {
+        privacyList = subscription;
+        break;
+      }
+    }
+    return acceptableAdsList && acceptableAdsList.disabled == false &&
+      privacyList && privacyList.disabled == false;
+  }
+
   function populateLists()
   {
     subscriptionsMap = Object.create(null);
     filtersMap = Object.create(null);
 
     // Empty collections and lists
     for (let property in collections)
       collections[property].clearAll();
 
     ext.backgroundPage.sendMessage({
@@ skipping 143 matching lines @@
         updateSubscription(subscription);
         break;
       case "downloading":
       case "downloadStatus":
       case "homepage":
       case "lastDownload":
       case "title":
         updateSubscription(subscription);
         break;
       case "added":
-        if (subscription.url in subscriptionsMap)
+        let {url} = subscription;
+        if (url in subscriptionsMap)
           updateSubscription(subscription);
         else
           addSubscription(subscription);
 
-        if (isAcceptableAds(subscription.url))
+        if (isAcceptableAds(url))
           setAcceptableAds();
 
+        if (url == acceptableAdsUrl || subscription.recommended == "privacy")
+        {
+          getPref("ui_warn_tracking", (showTrackingWarning) =>

[saroyanm, 2017/09/21 15:50:40]

I'm not sure if using promise is better in this case, please let me know if you
disagree, if I would use it here, it will be the only place that we are using
it.

As soon we will use this variable in multiple places I think in that case it
would make sense to use promises to make code more readable, but while we just
need it here, this code looks more compact in comparison to:

new Promise((resolve) =>
{
  getPref("ui_warn_tracking", resolve);
}).then((value) =>
{
  if (hasPrivacyConflict() && value)
    openDialog("tracking");
});

[Thomas Greiner, 2017/09/22 10:05:43]

Using a promise you could've avoided making that request to the background page
each time but this shouldn't happen too often anyway so this approach is also
totally fine with me since it keeps things simple.

It'd be great, however, if you could check for `hasPrivacyConflict()` before
retrieving the preference because we don't need to check for
`showTrackingWarning` if there's no privacy conflict.

[saroyanm, 2017/09/22 10:33:23]

I agree, done.

+          {
+            if (hasPrivacyConflict() && showTrackingWarning)
+              openDialog("tracking");
+          });
+        }
+
         collections.filterLists.addItem(subscription);
         break;
       case "removed":
         if (subscription.recommended)
         {
           subscription.disabled = true;
           onSubscriptionMessage("disabled", subscription);
         }
         else
         {
           delete subscriptionsMap[subscription.url];
           if (isAcceptableAds(subscription.url))
           {
             setAcceptableAds();
           }
           else
           {
             collections.more.removeItem(subscription);
           }
         }
         collections.filterLists.removeItem(subscription);
         break;
     }
-
   }
 
   function hidePref(key, value)
   {
     let element = document.querySelector("[data-pref='" + key + "']");
     if (element)
       element.setAttribute("aria-hidden", value);
   }
 
   function getPref(key, callback)
@@ skipping 105 matching lines @@
     type: "app.listen",
     filter: ["addSubscription", "focusSection"]
   });
   ext.backgroundPage.sendMessage({
     type: "filters.listen",
     filter: ["added", "loaded", "removed"]
   });
   ext.backgroundPage.sendMessage({
     type: "prefs.listen",
     filter: ["notifications_ignoredcategories", "notifications_showui",
-             "show_devtools_panel", "shouldShowBlockElementMenu"]
+             "show_devtools_panel", "shouldShowBlockElementMenu",
+             "ui_warn_tracking"]

[Thomas Greiner, 2017/09/22 10:05:43]

Detail: We're no longer reacting to changes for the "ui_warn_tracking"
preference so no need to listen to it anymore.

   });
   ext.backgroundPage.sendMessage({
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
              "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
 }