Issue 6432 - Hide remove button for additional filter lists

js/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
  * 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/>.
  */
 
 /* globals checkShareResource, getDocLink, i18nFormatDateTime, openSharePopup,
            setLinks, E */
 
 "use strict";
 
 let subscriptionsMap = Object.create(null);
 let filtersMap = Object.create(null);
 let collections = Object.create(null);
 let acceptableAdsUrl = null;
 let acceptableAdsPrivacyUrl = null;
 let isCustomFiltersLoaded = false;
+let additionalSubscriptions = [];
 let {getMessage} = browser.i18n;
 let {setElementText} = ext.i18n;
 let customFilters = [];
 let filterErrors = new Map([
   ["synchronize_invalid_url",
    "options_filterList_lastDownload_invalidURL"],
   ["synchronize_connection_error",
    "options_filterList_lastDownload_connectionError"],
   ["synchronize_invalid_data",
    "options_filterList_lastDownload_invalidData"],
@@ skipping 194 matching lines @@
     if (this.details[i].searchable)
       element.setAttribute("data-search", title.toLowerCase());
     let controls = element.querySelectorAll(".control[role='checkbox']");
     for (let control of controls)
     {
       control.setAttribute("aria-checked", item.disabled == false);
       if (isAcceptableAds(item.url) && this == collections.filterLists)
       {
         control.disabled = true;
       }
-      else if (this == collections.filterLists)

[saroyanm, 2018/03/01 21:52:34]

I wonder if I also should consider the case of removing checkbox from the
Privacy & Security section for the Block additional tracking and Block social
media icons tracking, when matched ? 

Should we still should allow changing or removing the language subscriptions
from the General tab ?

[Thomas Greiner, 2018/03/02 11:33:35]

This if-statement is redundant because in the CSS selector you're already making
sure that only `#all-filter-lists-table` is affected by this.

There's no harm in marking filter lists as "preconfigured" irregardless of the
table they're in.

[Thomas Greiner, 2018/03/02 11:33:36]

You're right that we should also consider other places in the UI where the user
can remove filter lists. But I'd say let's base such changes on the spec. I've
created https://gitlab.com/eyeo/specs/spec/issues/154 to update the spec
accordinly.

For now let's tackle anything that's easy to fix and that doesn't require any
large changes so that we can at least reestablish the previous behavior.

[saroyanm, 2018/03/02 13:50:29]

Done.

[saroyanm, 2018/03/02 13:50:29]

Done. I moved implementation out of the for loop, as we need to make
"preconfigured" element, rather than each checkbox.

-      {
-        // List preinstalled by administrators

[Thomas Greiner, 2018/03/02 11:33:35]

Detail: This is already documented under
https://hg.adblockplus.org/adblockpluschrome/file/ffd89bbfcd6f/lib/prefs.js#l194
so let's not duplicate that here.

[saroyanm, 2018/03/02 13:50:29]

Done.

-        getPref("additional_subscriptions", (additionalSubscriptions) =>
-        {
-          if (additionalSubscriptions.includes(item.url))
-            element.classList.add("non-removable");

[Thomas Greiner, 2018/03/02 11:33:35]

Detail: What about keeping the name more objective (e.g. "preconfigured")? That
way we could easily reuse it in the future without having to add more classes.

[saroyanm, 2018/03/02 13:50:29]

Done.

-        });
-      }
+    }
+    if (additionalSubscriptions.includes(item.url))
+    {
+      element.classList.add("preconfigured");
+      let disablePreconfigures =
+        element.querySelectorAll("[data-disable~='preconfigured']");
+      for (let disablePreconfigure of disablePreconfigures)
+        disablePreconfigure.disabled = true;
     }
 
     let lastUpdateElement = element.querySelector(".last-update");
     if (lastUpdateElement)
     {
       let message = element.querySelector(".message");
       if (item.isDownloading)
       {
         let text = getMessage("options_filterList_lastDownload_inProgress");
         message.textContent = text;
@@ skipping 210 matching lines @@
 
   filtersMap[filter.text] = filter;
 }
 
 function loadCustomFilters(filters)
 {
   for (let filter of filters)
     updateFilter(filter);
 
   setCustomFiltersView("read");
-  isCustomFiltersLoaded = true;
 }
 
 function removeCustomFilter(text)
 {
   let index = customFilters.indexOf(text);
   if (index >= 0)
     customFilters.splice(index, 1);
 
   updateCustomFiltersUi();
 }
@@ skipping 658 matching lines @@
   for (let property in collections)
     collections[property].clearAll();
 
   setCustomFiltersView("empty");
   browser.runtime.sendMessage({
     type: "subscriptions.get",
     special: true
   },
   (subscriptions) =>
   {
-    // Load filters
-    for (let subscription of subscriptions)
-    {
-      browser.runtime.sendMessage({
-        type: "filters.get",
-        subscriptionUrl: subscription.url
-      },
-      (filters) =>
-      {
-        loadCustomFilters(filters);
-      });
-    }
+    let customFilterPromises = subscriptions.map(getSubscriptionFilters);
+    Promise.all(customFilterPromises).then((filters) =>
+    {
+      loadCustomFilters([].concat(...filters));
+      isCustomFiltersLoaded = true;
+    });
   });
   loadRecommendations();
   browser.runtime.sendMessage({
     type: "prefs.get",
     key: "subscriptions_exceptionsurl"
   },
   (url) =>
   {
     acceptableAdsUrl = url;
 
     browser.runtime.sendMessage({
       type: "prefs.get",
       key: "subscriptions_exceptionsurl_privacy"
     },
     (urlPrivacy) =>
     {
       acceptableAdsPrivacyUrl = urlPrivacy;
 
-      // Load user subscriptions
-      browser.runtime.sendMessage({
-        type: "subscriptions.get",
-        downloadable: true
-      },
-      (subscriptions) =>
-      {
-        for (let subscription of subscriptions)
-          onSubscriptionMessage("added", subscription);
-
-        setAcceptableAds();
+      getPref("additional_subscriptions", (subscriptionUrls) =>
+      {
+        additionalSubscriptions = subscriptionUrls;
+
+        // Load user subscriptions
+        browser.runtime.sendMessage({
+          type: "subscriptions.get",
+          downloadable: true
+        },
+        (subscriptions) =>
+        {
+          for (let subscription of subscriptions)
+            onSubscriptionMessage("added", subscription);
+
+          setAcceptableAds();
+        });
       });
     });
   });
 }
 
 function addWhitelistedDomain()
 {
   let domain = E("whitelisting-textbox");
   for (let whitelistItem of collections.whitelist.items)
   {
@@ skipping 125 matching lines @@
         else
         {
           collections.more.removeItem(subscription);
         }
       }
 
       collections.filterLists.removeItem(subscription);
       setPrivacyConflict();
       break;
   }
+}
+
+function getSubscriptionFilters(subscription)
+{
+  return browser.runtime.sendMessage({
+    type: "filters.get",
+    subscriptionUrl: subscription.url});
 }
 
 function hidePref(key, value)
 {
   let element = document.querySelector("[data-pref='" + key + "']");
   if (element)
     element.setAttribute("aria-hidden", value);
 }
 
 function getPref(key, callback)
@@ skipping 119 matching lines @@
            "ui_warn_tracking"]
 });
 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);