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
@@ skipping 219 matching lines @@
     }
 
     element.setAttribute("aria-label", title);
     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.

+        });
+      }
     }
 
     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 1214 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);