Issue 5632 - Use checkboxes for toggling acceptable ads

desktop-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 541 matching lines @@
     });
   }
 
   function switchTab(id)
   {
     location.hash = id;
   }
 
   function execAction(action, element)
   {
+    if (element.getAttribute("aria-disabled") == "true")
+      return;
+
     switch (action)
     {
       case "add-domain-exception":
         addWhitelistedDomain();
         break;
       case "add-language-subscription":
         addEnableSubscription(findParentData(element, "access", false));
         break;
       case "add-predefined-subscription": {
         let dialog = E("dialog-content-predefined");
@@ skipping 73 matching lines @@
           removeExisting: true
         },
         () =>
         {
           setCustomFiltersView("read");
         });
         break;
       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;
       case "toggle-disable-subscription":
         browser.runtime.sendMessage({
           type: "subscriptions.toggle",
           keepInstalled: true,
           url: findParentData(element, "access", false)
         });
         break;
@@ skipping 205 matching lines @@
     {
       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);
+      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 =>
     {
       setLinks("dnt", url);
     });
 
     // Advanced tab
     let customize = document.querySelectorAll("#customize li[data-pref]");
     customize = Array.prototype.map.call(customize, (checkbox) =>
     {
@@ skipping 124 matching lines @@
   }
 
   function hideNotification()
   {
     E("notification").setAttribute("aria-hidden", true);
     E("notification-text").textContent = "";
   }
 
   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)
   {
     return url == acceptableAdsUrl || url == acceptableAdsPrivacyUrl;
   }
 
   function populateLists()
   {
     subscriptionsMap = Object.create(null);
@@ skipping 319 matching lines @@
   });
   browser.runtime.sendMessage({
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
              "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
 }