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 1128 matching lines @@
   for (let property in collections)
     collections[property].clearAll();
 
   setCustomFiltersView("empty");
   browser.runtime.sendMessage({
     type: "subscriptions.get",
     special: true
   },
   (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.

-
+    let customFilterPromises = subscriptions.map(getSubscriptionFilters);
     Promise.all(customFilterPromises).then((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) =>
   {
@@ skipping 160 matching lines @@
       }
 
       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);
-  });
+  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);