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;

[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 284 matching lines @@
         ext.backgroundPage.sendMessage({
           type: "subscriptions.update"
         });
         break;
       case "update-subscription":
         ext.backgroundPage.sendMessage({
           type: "subscriptions.update",
           url: findParentData(element, "access", false)
         });
         break;
+      case "validate-import-subscription":
+        let form = findParentData(element, "validation", true);
+        if (!form)
+          return;
+
+        if (form.checkValidity())
+        {
+          addEnableSubscription(E("import-list-url").value,
+            E("import-list-title").value);
+          form.reset();
+          closeDialog();
+        }
+        else
+        {
+          form.querySelector(":invalid").focus();
+        }
+        break;
     }
   }
 
   function setCustomFiltersView(mode)
   {
     let customFiltersElement = E("custom-filters-raw");
     updateCustomFiltersUi();
     if (mode == "read")
     {
       customFiltersElement.disabled = true;
@@ skipping 491 matching lines @@
         updateSubscription(subscription);
         break;
       case "downloading":
       case "downloadStatus":
       case "homepage":
       case "lastDownload":
       case "title":
         updateSubscription(subscription);
         break;
       case "added":
-        let {url} = subscription;
+        let {url, recommended} = subscription;
         if (url in subscriptionsMap)
           updateSubscription(subscription);
         else
           addSubscription(subscription);
 
         if (isAcceptableAds(url))
           setAcceptableAds();
 
-        if (url == acceptableAdsUrl || subscription.recommended == "privacy")
+        if ((url == acceptableAdsUrl || recommended == "privacy") &&
+          hasPrivacyConflict())
         {
           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)
+            if (showTrackingWarning)
               openDialog("tracking");
           });
         }
 
         collections.filterLists.addItem(subscription);
         break;
       case "removed":
         if (subscription.recommended)
         {
           subscription.disabled = true;
@@ skipping 133 matching lines @@
     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",
              "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);
 }