Issue 6420 - Fixed poping up whitelisted website notification for several custom 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 450 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)
+    let customFilterPromises = subscriptions.map((subscription) =>
+      getSubscriptionFilters(subscription));

[Thomas Greiner, 2018/03/05 16:43:33]

Detail: This arrow function is redundant because you can write the same code as:

let customFilterPromises = subscriptions.map(getSubscriptionFilters);

[saroyanm, 2018/03/05 16:51:13]

I like this, will change.

+
+    Promise.all(customFilterPromises).then((filters) =>
     {
-      browser.runtime.sendMessage({
-        type: "filters.get",
-        subscriptionUrl: subscription.url
-      },
-      (filters) =>
-      {
-        loadCustomFilters(filters);
-      });
-    }
+      loadCustomFilters([].concat(...filters));

[Thomas Greiner, 2018/03/05 16:43:33]

What is this concatenation for?

[saroyanm, 2018/03/05 16:51:13]

It's for flattening the array.

Promise.all returns a matrix in this case, because getSubscriptionFilters
promises an array.

[Thomas Greiner, 2018/03/06 13:08:47]

Sorry, I missed that. Maybe because I'm used to writing this as
`Array.prototype.concat.apply([], filters)`. Your approach is actually cleaner
so looks good.

+      isCustomFiltersLoaded = true;
+    });
   });
   loadRecommendations();
   browser.runtime.sendMessage({
     type: "prefs.get",
     key: "subscriptions_exceptionsurl"
   },
   (url) =>
   {
     acceptableAdsUrl = url;
 
@@ skipping 156 matching lines @@
           collections.more.removeItem(subscription);
         }
       }
 
       collections.filterLists.removeItem(subscription);
       setPrivacyConflict();
       break;
   }
 }
 
+function getSubscriptionFilters(subscription)
+{
+  return new Promise((resolve, reject) =>
+  {
+    browser.runtime.sendMessage({

[Thomas Greiner, 2018/03/05 16:43:33]

`browser.runtime.sendMessage()` already returns a promise because our
abstraction layer also supports how Firefox implements it those APIs. Therefore
we don't need to wrap it inside our own promise here.

See also
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/sendMes...

[saroyanm, 2018/03/05 16:51:13]

Didn't know that we already return a promise, will update this.
THanks.

[saroyanm, 2018/03/05 17:18:39]

Hmm, Apparently the polyfill doesn't return Promise ->
https://hg.adblockplus.org/adblockplusui/file/tip/ext/content.js#l67

[Thomas Greiner, 2018/03/06 13:08:47]

It's not actually a polyfill and more of a mock so I don't know why we label it
as such in ext/content.js. But yes, we need to update that.

+      type: "filters.get",
+      subscriptionUrl: subscription.url
+    }, resolve);
+  });
+}
+
 function hidePref(key, value)
 {
   let element = document.querySelector("[data-pref='" + key + "']");
   if (element)
     element.setAttribute("aria-hidden", value);
 }
 
 function getPref(key, callback)
 {
   let checkPref = getPref.checks[key] || getPref.checkNone;
@@ skipping 117 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);