Issue 5632 - Use checkboxes for toggling acceptable ads

desktop-options.js

Index: desktop-options.js
===================================================================
--- a/desktop-options.js
+++ b/desktop-options.js
@@ -559,6 +559,9 @@
 
   function execAction(action, element)
   {
+    if (element.getAttribute("aria-disabled") == "true")
+      return;
+
     switch (action)
     {
       case "add-domain-exception":
@@ -652,18 +655,23 @@
       case "show-more-filters-section":
         E("more-filters").setAttribute("aria-hidden", false);
         break;
-      case "switch-acceptable-ads":
+      case "switch-acceptable-ads": {
+        let isChecked = element.getAttribute("aria-checked") == "true";
         let value = element.value || element.dataset.value;
+        let isAcceptableAds = value == "ads";
+
         browser.runtime.sendMessage({
-          type: value == "privacy" ? "subscriptions.add" :
+          type: isAcceptableAds != isChecked ? "subscriptions.add" :
             "subscriptions.remove",
+          url: acceptableAdsUrl
+        });
+        browser.runtime.sendMessage({
+          type: isAcceptableAds || isChecked ? "subscriptions.remove" :
+            "subscriptions.add",
           url: acceptableAdsPrivacyUrl
         });
-        browser.runtime.sendMessage({
-          type: value == "ads" ? "subscriptions.add" : "subscriptions.remove",
-          url: acceptableAdsUrl
-        });
         break;
+      }
       case "switch-tab":
         switchTab(element.getAttribute("href").substr(1));
         break;
@@ -889,7 +897,11 @@
     });
     getDocLink("acceptable_ads_criteria", (link) =>
     {
-      setLinks("enable-aa-description", link);
+      setLinks("enable-acceptable-ads-description", link);
+    });
+    getDocLink("privacy_friendly_ads", (link) =>
+    {
+      E("enable-acceptable-ads-privacy-description").href = link;
     });
     getDocLink("adblock_plus_{browser}_dnt", url =>
     {
@@ -1034,20 +1046,31 @@
 
   function setAcceptableAds()
   {
-    let option = "none";
-    document.forms["acceptable-ads"].classList.remove("show-dnt-notification");
+    let acceptableAdsForm = E("acceptable-ads");
+    let acceptableAds = E("acceptable-ads-allow");
+    let acceptableAdsPrivacy = E("acceptable-ads-privacy-allow");
+    acceptableAdsForm.classList.remove("show-dnt-notification");
+    acceptableAds.setAttribute("aria-checked", false);
+    acceptableAdsPrivacy.setAttribute("aria-checked", false);
     if (acceptableAdsUrl in subscriptionsMap)
     {
-      option = "ads";
+      acceptableAds.setAttribute("aria-checked", true);
+      acceptableAdsPrivacy.setAttribute("aria-disabled", false);
     }
     else if (acceptableAdsPrivacyUrl in subscriptionsMap)
     {
-      option = "privacy";
+      acceptableAds.setAttribute("aria-checked", true);
+      acceptableAdsPrivacy.setAttribute("aria-checked", true);
+      acceptableAdsPrivacy.setAttribute("aria-disabled", false);
 
-      if (!navigator.doNotTrack)
-        document.forms["acceptable-ads"].classList.add("show-dnt-notification");
+      if (navigator.doNotTrack != 1)
+        acceptableAdsForm.classList.add("show-dnt-notification");
     }
-    document.forms["acceptable-ads"]["acceptable-ads"].value = option;
+    else
+    {
+      // Using aria-disabled in order to keep the focus
+      acceptableAdsPrivacy.setAttribute("aria-disabled", true);

[ire, 2017/10/18 17:48:45]

This is a fine compromise. We should also set the tabindex to -1 when disabled,
to stop users being able to focus on it via keyboard when it is disabled. (I
tried this and it doesn't stop the element being focused when toggling it
specifically)

[saroyanm, 2017/10/19 20:41:56]

Good point, done.

+    }
   }
 
   function isAcceptableAds(url)