Issue 3885 - Disable the checkbox to toggle the Acceptable Ads list on the options page

options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 (function()
 {
   var subscriptionsMap = Object.create(null);
   var recommendationsMap = Object.create(null);
   var filtersMap = Object.create(null);
   var collections = Object.create(null);
+  var acceptableAdsUrl = null;
   var maxLabelId = 0;
   var getMessage = ext.i18n.getMessage;
   var filterErrors =
   {
     "synchronize_invalid_url": "options_filterList_lastDownload_invalidURL",
     "synchronize_connection_error": "options_filterList_lastDownload_connectionError",
     "synchronize_invalid_data": "options_filterList_lastDownload_invalidData",
     "synchronize_checksum_mismatch": "options_filterList_lastDownload_checksumMismatch"
   };
 
@@ skipping 19 matching lines @@
 
   Collection.prototype._createElementQuery = function(item)
   {
     var access = (item.url || item.text).replace(/'/g, "\\'");
     return function(container)
     {
       return container.querySelector("[data-access='" + access + "']");
     };
   };
 
+  Collection.prototype._isControlDisabled = function(item, i)
+  {
+    return item.url == acceptableAdsUrl &&
+           this.details[i].onClick == toggleDisableSubscription;
+  };
+
+  Collection.prototype._getItemTitle = function(item, i)
+  {
+    var title = null;
+    if (this.details[i].useOriginalTitle)
+      title = item.originalTitle;
+    if (!title)
+      title = item.title || item.url || item.text;
+    return title;
+  };
+
   Collection.prototype.addItems = function() 
   {
     var length = Array.prototype.push.apply(this.items, arguments);
     if (length == 0)
       return;
 
     this.items.sort(function(a, b)
     {
-      var aValue = (a.title || a.text || a.url).toLowerCase();
-      var bValue = (b.title || b.text || b.url).toLowerCase();
-      return aValue.localeCompare(bValue);
-    });
+      var aDisabled = this._isControlDisabled(a, 0);
+      var bDisabled = this._isControlDisabled(b, 0);

[Thomas Greiner, 2016/04/07 17:21:15]

It's a nice idea to group together all disabled checkboxes but it's not ideal
that we have to hardcode the table index since this special ordering is only
useful for Acceptable Ads and only for this one table. That is especially since
we intend to get rid of disabled checkboxes later on. What about instead
checking specifically for the whitelist here to ensure that it's at the end of
the list?

e.g.
this.items.sort(function(a, b)
{
  if (a.url == acceptableAdsUrl)
    return 1;
  
  ...
});

[Sebastian Noack, 2016/04/08 14:22:17]

Yeah, hard-coding the index isn't great, but we have to do so for
_getItemTitle() anyway. And the way I did it it was more obvious that we are
grouping disabled/enabled items respectively. Now it need a comment to explain
what that is about. But fair enough. Done.

+      if (aDisabled != bDisabled)
+        return aDisabled - bDisabled;
+
+      var aTitle = this._getItemTitle(a, 0).toLowerCase();

[Thomas Greiner, 2016/04/07 17:21:15]

Detail: Hardcoding the table index here might cause items to be not
alphabetically ordered. However, I assume that fixing this will require major
changes in the way we keep the UI in sync with the data so feel free to ignore
it for this review.

[Sebastian Noack, 2016/04/08 14:22:17]

I know. This is kinda a hack. But at least more correct than previously.

+      var bTitle = this._getItemTitle(b, 0).toLowerCase();
+      return aTitle.localeCompare(bTitle);
+    }.bind(this));
 
     for (var j = 0; j < this.details.length; j++) 
     {
       var table = E(this.details[j].id);
       var template = table.querySelector("template");
       for (var i = 0; i < arguments.length; i++) 
       {
         var item = arguments[i];
         var listItem = document.createElement("li");
         listItem.appendChild(document.importNode(template.content, true));
@@ skipping 74 matching lines @@
   Collection.prototype.updateItem = function(item)
   {
     var access = (item.url || item.text).replace(/'/g, "\\'");
     for (var i = 0; i < this.details.length; i++)
     {
       var table = E(this.details[i].id);
       var element = table.querySelector("[data-access='" + access + "']");
       if (!element)
         continue;
 
-      var title = null;
-      if (this.details[i].useOriginalTitle)
-        title = item.originalTitle;
-      if (!title)
-        title = item.title || item.url || item.text;
+      var title = this._getItemTitle(item, i);
       element.querySelector(".display").textContent = title;
       if (title)
         element.setAttribute("data-search", title.toLowerCase());
       var control = element.querySelector(".control[role='checkbox']");
       if (control)
+      {
         control.setAttribute("aria-checked", item.disabled == false);
+        if (this._isControlDisabled(item, i))
+          control.setAttribute("disabled", true);
+      }
 
       var dateElement = element.querySelector(".date");
       var timeElement = element.querySelector(".time");
       if (dateElement && timeElement)
       {
         var message = element.querySelector(".message");
         if (item.isDownloading)
         {
           var text = getMessage("options_filterList_lastDownload_inProgress");
           message.textContent = text;
@@ skipping 235 matching lines @@
       {
         if (property == "title" && subscriptionUrl in recommendationsMap)
           knownSubscription.originalTitle = subscription.title;
         else
           knownSubscription[property] = subscription[property];
       }
     }
     else
     {
       observeSubscription(subscription);
-      getAcceptableAdsURL(function(acceptableAdsUrl)
+
+      var collection;
+      if (subscriptionUrl in recommendationsMap)
       {
-        var collection = null;
-        if (subscriptionUrl in recommendationsMap)
-        {
-          var recommendation = recommendationsMap[subscriptionUrl];
-          if (recommendation.type != "ads")
-            collection = collections.popular;
-          else if (subscription.disabled == false)
-            collection = collections.langs;
-          else
-            collection = collections.allLangs;
-        }
-        else if (subscriptionUrl == acceptableAdsUrl)
-          collection = collections.acceptableAds;
+        var recommendation = recommendationsMap[subscriptionUrl];
+        if (recommendation.type != "ads")
+          collection = collections.popular;
+        else if (subscription.disabled == false)
+          collection = collections.langs;
         else
-          collection = collections.custom;
+          collection = collections.allLangs;
+      }
+      else if (subscriptionUrl == acceptableAdsUrl)
+        collection = collections.acceptableAds;
+      else
+        collection = collections.custom;
 
-        collection.addItems(subscription);
-        subscriptionsMap[subscriptionUrl] = subscription;
-      });
+      collection.addItems(subscription);
+      subscriptionsMap[subscriptionUrl] = subscription;
     }
   }
 
   function updateFilter(filter)
   {
     var match = filter.text.match(/^@@\|\|([^\/:]+)\^\$document$/);
     if (match && !filtersMap[filter.text])
     {
       filter.title = match[1];
       collections.whitelist.addItems(filter);
@@ skipping 368 matching lines @@
           subscriptionUrl: subscriptions[i].url
         }, 
         function(filters)
         {
           for (var i = 0; i < filters.length; i++)
             updateFilter(filters[i]);
         });
       }
     });
     loadRecommendations();
-    getAcceptableAdsURL(function(acceptableAdsUrl)
+    ext.backgroundPage.sendMessage(
     {
-      var subscription = Object.create(null);
-      subscription.url = acceptableAdsUrl;
-      subscription.disabled = true;
-      subscription.title = getMessage("options_acceptableAds_description");
-      updateSubscription(subscription);
+      type: "prefs.get",
+      key: "subscriptions_exceptionsurl"
+    },
+    function(url)
+    {
+      acceptableAdsUrl = url;
+      updateSubscription({
+        url: acceptableAdsUrl,
+        disabled: true,
+        title: getMessage("options_acceptableAds_description")
+      });
 
       // Load user subscriptions
       ext.backgroundPage.sendMessage(
       {
         type: "subscriptions.get",
         downloadable: true
       }, 
       function(subscriptions)
       {
         for (var i = 0; i < subscriptions.length; i++)
@@ skipping 20 matching lines @@
 
   function editCustomFilters()
   {
     var customFilterItems = collections.customFilters.items;
     var filterTexts = [];
     for (var i = 0; i < customFilterItems.length; i++)
       filterTexts.push(customFilterItems[i].text);
     E("custom-filters-raw").value = filterTexts.join("\n");
   }
 
-  function getAcceptableAdsURL(callback)
-  {
-    getPref("subscriptions_exceptionsurl", function(value)
-    {
-      getAcceptableAdsURL = function(callback)
-      {
-        callback(value);
-      };
-      getAcceptableAdsURL(callback);
-    });
-  }
-
   function addEnableSubscription(url, title, homepage)
   {
     var messageType = null;
     var knownSubscription = subscriptionsMap[url];
     if (knownSubscription && knownSubscription.disabled == true)
       messageType = "subscriptions.toggle"
     else
       messageType = "subscriptions.add"
 
     var message = {
@@ skipping 42 matching lines @@
           collections.filterLists.addItems(knownSubscription);
         else
           collections.filterLists.addItems(subscription);
         break;
       case "disabled":
         updateSubscription(subscription);
         updateShareLink();
         break;
       case "removed":
         var knownSubscription = subscriptionsMap[subscription.url];
-        getAcceptableAdsURL(function(acceptableAdsUrl)
+        if (subscription.url == acceptableAdsUrl)
         {
-          if (subscription.url == acceptableAdsUrl)
-          {
-            subscription.disabled = true;
-            updateSubscription(subscription);
-          }
+          subscription.disabled = true;
+          updateSubscription(subscription);
+        }
+        else
+        {
+          if (subscription.url in recommendationsMap)
+            knownSubscription.disabled = true;
           else
           {
-            if (subscription.url in recommendationsMap)
-              knownSubscription.disabled = true;
-            else
-            {
-              collections.custom.removeItem(knownSubscription);
-              delete subscriptionsMap[subscription.url];
-            }
+            collections.custom.removeItem(knownSubscription);
+            delete subscriptionsMap[subscription.url];
           }
-          updateShareLink();
-          collections.filterLists.removeItem(knownSubscription);
-        });
+        }
+        updateShareLink();
+        collections.filterLists.removeItem(knownSubscription);
         break;
       default:
         updateSubscription(subscription);
         break;
     }
   }
 
   function hidePref(key, value)
   {
     var element = document.querySelector("[data-pref='" + key + "']");
@@ skipping 146 matching lines @@
   });
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
         "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();