issue 6075 - Hide Acceptable Ads notification when corresponding subscription is removed

desktop-options.js

Index: desktop-options.js
===================================================================
--- a/desktop-options.js
+++ b/desktop-options.js
@@ -1101,7 +1101,7 @@
     return url == acceptableAdsUrl || url == acceptableAdsPrivacyUrl;
   }
 
-  function hasPrivacyConflict()
+  function setPrivacyConflict()
   {
     let acceptableAdsList = subscriptionsMap[acceptableAdsUrl];
     let privacyList = null;
@@ -1114,8 +1114,16 @@
         break;
       }
     }
-    return acceptableAdsList && acceptableAdsList.disabled == false &&
-      privacyList && privacyList.disabled == false;
+    E("acceptable-ads").classList.remove("show-warning");

[Thomas Greiner, 2017/11/23 14:46:12]

Detail: We can avoid this unnecessary call by moving it into the "else" case and
using `Element.classList.toggle()` instead of `Element.classList.add()`.

[saroyanm, 2017/11/23 16:41:38]

I agree, well spotted, done.

+    if (acceptableAdsList && acceptableAdsList.disabled == false &&
+      privacyList && privacyList.disabled == false)
+    {
+      getPref("ui_warn_tracking", (showTrackingWarning) =>
+      {
+        if (showTrackingWarning)
+          E("acceptable-ads").classList.add("show-warning");
+      });
+    }
   }
 
   function populateLists()
@@ -1266,6 +1274,7 @@
     switch (action)
     {
       case "disabled":
+        setPrivacyConflict();

[Thomas Greiner, 2017/11/23 14:46:12]

Detail: Let's do this after we've updated the UI. Because in case an error
occurs in either we probably care more about the UI being up-to-date rather than
the warning being shown.

Same applies to the other places where we call `setPrivacyConflict()`.

[saroyanm, 2017/11/23 16:41:38]

DOne.

         updateSubscription(subscription);
         break;
       case "downloading":
@@ -1291,16 +1300,7 @@
         if (isAcceptableAds(url))
           setAcceptableAds();
 
-        if ((url == acceptableAdsUrl || recommended == "privacy") &&
-          hasPrivacyConflict())
-        {
-          getPref("ui_warn_tracking", (showTrackingWarning) =>
-          {
-            if (showTrackingWarning)
-              E("acceptable-ads").classList.add("show-warning");
-          });
-        }
-
+        setPrivacyConflict();
         collections.filterLists.addItem(subscription);
         break;
       case "removed":
@@ -1321,6 +1321,8 @@
             collections.more.removeItem(subscription);
           }
         }
+
+        setPrivacyConflict();
         collections.filterLists.removeItem(subscription);
         break;
     }
@@ -1376,8 +1378,7 @@
         hidePref("notifications_ignoredcategories", !value);
         break;
       case "ui_warn_tracking":
-        let showWarning = (value && hasPrivacyConflict());
-        E("acceptable-ads").classList.toggle("show-warning", showWarning);
+        setPrivacyConflict();
         break;
     }