Issue 5169 - Add whitelisted tab to the new options page

new-options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 19 matching lines @@
   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"],
     ["synchronize_checksum_mismatch",
      "options_filterList_lastDownload_checksumMismatch"]
   ]);
+  const timestampUI = Symbol();
 
   function Collection(details)
   {
     this.details = details;
     this.items = [];
   }
 
-  Collection.prototype._setEmpty = function(table, text)
+  Collection.prototype._setEmpty = function(table, texts)
   {
-    let placeholder = table.querySelector(".empty-placeholder");
-    if (text && !placeholder)
+    let placeholders = table.querySelectorAll(".empty-placeholder");
+
+    if (texts && placeholders.length == 0)
     {
-      placeholder = document.createElement("li");
-      placeholder.className = "empty-placeholder";
-      placeholder.textContent = getMessage(text);
-      table.appendChild(placeholder);
+      for (let text of texts)
+      {
+        let placeholder = document.createElement("li");
+        placeholder.className = "empty-placeholder";
+        placeholder.textContent = getMessage(text);
+        table.appendChild(placeholder);
+      }
     }
-    else if (placeholder)
-      table.removeChild(placeholder);
+    else if (placeholders.length > 0)
+    {
+      for (let placeholder of placeholders)
+        table.removeChild(placeholder);
+    }
   };
 
   Collection.prototype._createElementQuery = function(item)
   {
     let access = (item.url || item.text).replace(/'/g, "\\'");
     return function(container)
     {
       return container.querySelector("[data-access='" + access + "']");
     };
   };
@@ skipping 16 matching lines @@
     this.items.sort((a, b) =>
     {
       // Make sure that Acceptable Ads is always last, since it cannot be
       // disabled, but only be removed. That way it's grouped together with
       // the "Own filter list" which cannot be disabled either at the bottom
       // of the filter lists in the Advanced tab.
       if (a.url == acceptableAdsUrl)
         return 1;
       if (b.url == acceptableAdsUrl)
         return -1;
+      if (a[timestampUI])
+        return b[timestampUI] ? b[timestampUI] - a[timestampUI] : -1;
+      if (b[timestampUI])
+        return a[timestampUI] ? a[timestampUI] - b[timestampUI] : 1;

[Thomas Greiner, 2017/05/26 11:10:39]

This condition is duplicated and contradictory because if `a` and `b` both have
timestamps it'll already return `b - a` above.

You could write it instead as

if (a[timestampUI] || b[timestampUI])
  return (b[timestampUI] || 0) - (a[timestampUI] || 0);

or if you want to spread it out a bit more:

let aTimestamp = a[timestampUI] || 0;
let bTimestamp = b[timestampUI] || 0;
if (a || b)
  return b - a;

[Thomas Greiner, 2017/05/26 11:10:40]

Detail: Please add a comment as we did with the special treatment for Acceptable
Ads above.

[saroyanm, 2017/05/31 08:30:27]

Done.

[saroyanm, 2017/05/31 08:30:28]

Done.

 
       let aTitle = this._getItemTitle(a, 0).toLowerCase();
       let bTitle = this._getItemTitle(b, 0).toLowerCase();
       return aTitle.localeCompare(bTitle);
     });
 
     for (let j = 0; j < this.details.length; j++)
     {
       let table = E(this.details[j].id);
       let template = table.querySelector("template");
@@ skipping 85 matching lines @@
       let title = this._getItemTitle(item, i);
       element.querySelector(".display").textContent = title;
       element.setAttribute("aria-label", title);
       if (this.details[i].searchable)
         element.setAttribute("data-search", title.toLowerCase());
       let control = element.querySelector(".control[role='checkbox']");
       if (control)
       {
         control.setAttribute("aria-checked", item.disabled == false);
         if (item.url == acceptableAdsUrl && this == collections.filterLists)
-          control.setAttribute("disabled", true);
+          control.disabled = true;
       }
 
       let dateElement = element.querySelector(".date");
       let timeElement = element.querySelector(".time");
       if (dateElement && timeElement)
       {
         let message = element.querySelector(".message");
         if (item.isDownloading)
         {
           let text = getMessage("options_filterList_lastDownload_inProgress");
@@ skipping 67 matching lines @@
   }
 
   collections.popular = new Collection([
     {
       id: "recommend-list-table"
     }
   ]);
   collections.langs = new Collection([
     {
       id: "blocking-languages-table",
-      emptyText: "options_dialog_language_added_empty"
+      emptyText: ["options_dialog_language_added_empty"]
     },
     {
       id: "blocking-languages-dialog-table",
-      emptyText: "options_dialog_language_added_empty"
+      emptyText: ["options_dialog_language_added_empty"]
     }
   ]);
   collections.allLangs = new Collection([
     {
       id: "all-lang-table",
-      emptyText: "options_dialog_language_other_empty",
+      emptyText: ["options_dialog_language_other_empty"],
       searchable: true
     }
   ]);
   collections.acceptableAds = new Collection([
     {
       id: "acceptableads-table"
     }
   ]);
   collections.custom = new Collection([
     {
       id: "custom-list-table"
     }
   ]);
   collections.whitelist = new Collection([
     {
       id: "whitelisting-table",
-      emptyText: "options_whitelisted_empty"
+      emptyText: ["options_whitelist_empty_1", "options_whitelist_empty_2"]
     }
   ]);
   collections.customFilters = new Collection([
     {
       id: "custom-filters-table",
-      emptyText: "options_customFilters_empty"
+      emptyText: ["options_customFilters_empty"]
     }
   ]);
   collections.filterLists = new Collection([
     {
       id: "all-filter-lists-table",
       useOriginalTitle: true
     }
   ]);
 
   function toggleShowLanguage(subscription)
@@ skipping 156 matching lines @@
         let dialog = E("dialog-content-predefined");
         let title = dialog.querySelector("h3").textContent;
         let url = dialog.querySelector(".url").textContent;
         addEnableSubscription(url, title);
         closeDialog();
         break;
       }
       case "cancel-custom-filters":
         E("custom-filters").classList.remove("mode-edit");
         break;
-      case "cancel-domain-exception":
-        E("whitelisting-textbox").value = "";
-        document.querySelector("#whitelisting .controls").classList
-          .remove("mode-edit");
-        break;
       case "close-dialog":
         closeDialog();
         break;
       case "edit-custom-filters":
         editCustomFilters();
         break;
-      case "edit-domain-exception":
-        document.querySelector("#whitelisting .controls").classList
-          .add("mode-edit");
-        E("whitelisting-textbox").focus();
-        break;
       case "import-subscription": {
         let url = E("blockingList-textbox").value;
         addEnableSubscription(url);
         closeDialog();
         break;
       }
       case "open-context-menu": {
         let listItem = findParentData(element, "access", true);
         if (listItem && !listItem.classList.contains("show-context-menu"))
           listItem.classList.add("show-context-menu");
@@ skipping 219 matching lines @@
 
     updateShareLink();
     updateTooltips();
 
     // Initialize interactive UI elements
     document.body.addEventListener("click", onClick, false);
     document.body.addEventListener("keyup", onKeyUp, false);
     let placeholderValue = getMessage("options_dialog_language_find");
     E("find-language").setAttribute("placeholder", placeholderValue);
     E("find-language").addEventListener("keyup", onFindLanguageKeyUp, false);
-    E("whitelisting-textbox").addEventListener("keypress", (e) =>
+    let exampleValue = getMessages("options_whitelist_placeholder_example", 
+      ["www.example.com"]);
+    E("whitelisting-textbox").setAttribute("placeholder", exampleValue);
+    E("whitelisting-textbox").addEventListener("keyup", (e) =>
     {
+      let addWhitelistButton = E("whitelisting-add-button");
+      addWhitelistButton.disabled = false;
       if (getKey(e) == "Enter")
-        addWhitelistedDomain();
+      {
+        if (!addWhitelistButton.disabled)
+          addWhitelistedDomain();
+      }
+      else
+      {
+        if (!e.target.value)
+        {
+          addWhitelistButton.disabled = true;

[Thomas Greiner, 2017/05/26 11:10:39]

Detail: Now that we reduced it to a simple check, you could simplify this logic
a bit by writing it as:

addWhitelistButton.disabled = !e.target.value;


Also note that resetting `addWhitelistButton.disabled` to `false` causes the
above condition `!addWhitelistButton.disabled` to always be `true`.

[saroyanm, 2017/05/31 08:30:27]

Done.

+        }
+      }
     }, false);
 
     // Advanced tab
     let tweaks = document.querySelectorAll("#tweaks li[data-pref]");
     tweaks = Array.prototype.map.call(tweaks, (checkbox) =>
     {
       return checkbox.getAttribute("data-pref");
     });
     for (let key of tweaks)
     {
@@ skipping 188 matching lines @@
       {
         for (let subscription of subscriptions)
           onSubscriptionMessage("added", subscription);
       });
     });
   }
 
   function addWhitelistedDomain()
   {
     let domain = E("whitelisting-textbox");
+    for (let whitelistItem of collections.whitelist.items)
+    {
+      if (whitelistItem.title == domain.value)
+      {
+        collections.whitelist.removeItem(whitelistItem);
+        whitelistItem[timestampUI] = Date.now();
+        collections.whitelist.addItem(whitelistItem);

[Thomas Greiner, 2017/05/26 11:10:40]

Removing and then adding the item again is quite a hack. Calling
`collections.whitelist.updateItem()` seems more appropriate.

[saroyanm, 2017/05/30 12:30:43]

I agree with you.

The problem is that we are currently sorting items only when we are adding them
(which I think is a bug), I think we can fix that behavior in current review, by
sorting items both when adding and updating.

[saroyanm, 2017/05/31 08:30:27]

Done.

+        domain.value = "";

[Thomas Greiner, 2017/05/26 11:10:39]

No need to continue the loop because we already found the item we were searching
for. So let's `break` here.

[saroyanm, 2017/05/31 08:30:27]

Done.

+      }
+    }
     if (domain.value)
     {
       sendMessageHandleErrors({
         type: "filters.add",
         text: "@@||" + domain.value.toLowerCase() + "^$document"
       });
     }
 
     domain.value = "";
-    document.querySelector("#whitelisting .controls")
-      .classList.remove("mode-edit");
+    E("whitelisting-add-button").disabled = true;
   }
 
   function editCustomFilters()
   {
     if (!isCustomFiltersLoaded)
     {
       console.error("Custom filters are not loaded");
       return;
     }
 
@@ skipping 303 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);
 }