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)
-    {
-      browser.runtime.sendMessage({
-        type: "filters.get",
-        subscriptionUrl: subscription.url
-      },
-      (filters) =>
-      {
-        // Reset each time as there might be several custom filters
-        if (isCustomFiltersLoaded)
-          isCustomFiltersLoaded = false;

[Thomas Greiner, 2018/03/02 11:43:57]

Why not instead just move `isCustomFiltersLoaded = true` to after
`loadCustomFilters()` here?

Because this is the only location where we load custom filters to initialize
them so I don't see a reason why to set it to `true` any other time we call
`loadCustomFilters()`.

[saroyanm, 2018/03/02 15:59:14]

Agree, done.

[Thomas Greiner, 2018/03/02 16:40:43]

What about the code you initially added?

[saroyanm, 2018/03/02 16:43:46]

Which code ?

[Thomas Greiner, 2018/03/02 17:09:04]

The new code from the first patch set.

[saroyanm, 2018/03/02 17:16:51]

You mean the current one, right ?
I don't remember adding any other patch sets.

I can see current one and that small change you have mentioned here as part of
patchset 2 ->
https://codereview.adblockplus.org/29712599/diff/29713620/js/desktop-options.js 

[Thomas Greiner, 2018/03/02 17:24:16]

I mean this one:
https://codereview.adblockplus.org/29712599/diff/29712600/js/desktop-options.js

[saroyanm, 2018/03/05 12:14:52]

We are using loop on top, but now we decided to refactor the code and use
promises.

[saroyanm, 2018/03/05 14:52:27]

Done.

-        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;
 
@@ skipping 154 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);