Issue 5778 - Hide More filters section when there is no filter

new-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 34 matching lines @@
   const minuteInMs = 60000;
   const hourInMs = 3600000;
   const fullDayInMs = 86400000;
 
   function Collection(details)
   {
     this.details = details;
     this.items = [];
   }
 
   Collection.prototype._setEmpty = function(table, detail, removeEmpty)

[ire, 2017/09/26 08:37:55]

NIT: This function and the nested if statements are a bit confusing to follow. I
would suggest making it flatter, e.g.:


if (removeEmpty && detail.removeEmptyAction)
  ...
else if (removeEmpty)
  ...
else if (detail.emptyText)
  ...
else if (detail.setEmptyAction)
  ...

[saroyanm, 2017/09/26 16:35:25]

Absolutely, my bad, now should be more readable I assume.

   {
     if (removeEmpty)
     {
       let placeholders = table.querySelectorAll(".empty-placeholder");
-      if (placeholders.length > 0)
-      {
-        for (let placeholder of placeholders)
-          table.removeChild(placeholder);
-      }
-
-      if (detail.removeEmptyAction)
-        execAction(detail.removeEmptyAction, table);
+      for (let placeholder of placeholders)
+        table.removeChild(placeholder);
+
+      execAction(detail.removeEmptyAction, table);
     }
     else
     {
-      if (detail.emptyText)
-      {
-        for (let text of detail.emptyText)
-        {
-          let placeholder = document.createElement("li");
-          placeholder.className = "empty-placeholder";
-          placeholder.textContent = getMessage(text);
-          table.appendChild(placeholder);
-        }
-      }
-
-      if (detail.setEmptyAction)
-        execAction(detail.setEmptyAction, table);
+      let {emptyTexts = []} = detail;
+      for (let text of emptyTexts)
+      {
+        let placeholder = document.createElement("li");
+        placeholder.className = "empty-placeholder";
+        placeholder.textContent = getMessage(text);
+        table.appendChild(placeholder);
+      }
+
+      execAction(detail.setEmptyAction, table);
     }
   };
 
   Collection.prototype._createElementQuery = function(item)
   {
     let access = (item.url || item.text).replace(/'/g, "\\'");
     return function(container)
     {
       return container.querySelector("[data-access='" + access + "']");
     };
@@ skipping 253 matching lines @@
   }
 
   collections.protection = new Collection([
     {
       id: "recommend-protection-list-table"
     }
   ]);
   collections.langs = new Collection([
     {
       id: "blocking-languages-table",
-      emptyText: ["options_language_empty"]
+      emptyTexts: ["options_language_empty"]
     }
   ]);
   collections.allLangs = new Collection([
     {
       id: "all-lang-table-add",
-      emptyText: ["options_dialog_language_other_empty"]
+      emptyTexts: ["options_dialog_language_other_empty"]
     }
   ]);
   collections.more = new Collection([
     {
       id: "more-list-table",
-      setEmptyAction: "hide-more-section",
-      removeEmptyAction: "show-more-section"

[ire, 2017/09/26 08:37:55]

NIT: Calling it "hide-more-section" seems to general when this is specific to
the more filters section. Suggest:
"hide-more-filters-section"/"show-more-filters-section" etc.

[saroyanm, 2017/09/26 16:35:25]

I agree with you, done.

+      setEmptyAction: "hide-more-filters-section",
+      removeEmptyAction: "show-more-filters-section"
     }
   ]);
   collections.whitelist = new Collection([
     {
       id: "whitelisting-table",
-      emptyText: ["options_whitelist_empty_1", "options_whitelist_empty_2"]
+      emptyTexts: ["options_whitelist_empty_1", "options_whitelist_empty_2"]
     }
   ]);
   collections.filterLists = new Collection([
     {
       id: "all-filter-lists-table",
       useOriginalTitle: true
     }
   ]);
 
   function addSubscription(subscription)
@@ skipping 202 matching lines @@
             break;
           }
         }
         break;
       case "close-dialog":
         closeDialog();
         break;
       case "edit-custom-filters":
         setCustomFiltersView("write");
         break;
-      case "hide-more-section":
+      case "hide-more-filters-section":
         E("more-filters").setAttribute("aria-hidden", true);
         break;
       case "hide-notification":
         hideNotification();
         break;
       case "import-subscription": {
         let url = E("blockingList-textbox").value;
         addEnableSubscription(url);
         closeDialog();
         break;
@@ skipping 25 matching lines @@
         sendMessageHandleErrors({
           type: "filters.importRaw",
           text: E("custom-filters-raw").value,
           removeExisting: true
         },
         () =>
         {
           setCustomFiltersView("read");
         });
         break;
-      case "show-more-section":
+      case "show-more-filters-section":
         E("more-filters").setAttribute("aria-hidden", false);
         break;
       case "switch-acceptable-ads":
         let value = element.value || element.dataset.value;
         ext.backgroundPage.sendMessage({
           type: value == "privacy" ? "subscriptions.add" :
             "subscriptions.remove",
           url: acceptableAdsPrivacyUrl
         });
         ext.backgroundPage.sendMessage({
@@ skipping 221 matching lines @@
     }, false);
 
     getDocLink("contribute", (link) =>
     {
       E("contribute").href = link;
     });
     getDocLink("acceptable_ads_criteria", (link) =>
     {
       setLinks("enable-aa-description", 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) =>
     {
       return checkbox.getAttribute("data-pref");
     });
     for (let key of customize)
     {
       getPref(key, (value) =>
@@ skipping 29 matching lines @@
       setLinks("report-issue", link);
     });
     getDocLink("adblock_plus_report_ad", (link) =>
     {
       setLinks("report-ad", link);
     });
     getDocLink("adblock_plus_report_bug", (link) =>
     {
       setLinks("report-bug", link);
     });
-    getDocLink("reporter_other_link", (link) =>
-    {
-      setLinks("report-forum", link);
+    getDocLink("{browser}_support", url =>
+    {
+      setLinks("visit-forum", url);
     });
     getDocLink("social_twitter", (link) =>
     {
       E("twitter").setAttribute("href", link);
     });
     getDocLink("social_facebook", (link) =>
     {
       E("facebook").setAttribute("href", link);
     });
     getDocLink("social_gplus", (link) =>
@@ skipping 447 matching lines @@
   });
   ext.backgroundPage.sendMessage({
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
              "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
 }