Issue 6865 - Update ABP dependency to version 3.2

adblockplus/extensionBridge.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 20 matching lines @@
 const TITLE_PROP = "title";
 const URL_PROP = "url";
 
 function init()
 {
   Promise.all([FilterNotifier.once("load"), Prefs.untilLoaded]).then(onLoaded);
 }
 
 function onLoaded()
 {
-  browser.runtime.abbRegisterRequestListener(handleRequest);
-  browser.runtime.abbSendRequest("Abb:OnLoaded");

[Thomas Greiner, 2018/08/27 15:24:41]

Detail: The "Abb:" prefix seems redundant here since we're already calling
`abbSendRequest()`. Instead, we could just prepend it in ext-c-runtime.js.

[diegocarloslima, 2018/08/27 21:19:16]

Acknowledged.

+  browser.runtime.registerAbbMessageListener(handleMessage);
+  browser.runtime.sendAbbMessage("OnLoaded");
   FilterNotifier.on("save", onFiltersSaved);
 }
 
 function onFiltersSaved()
 {
-  browser.runtime.abbSendRequest("Abb:OnFiltersSaved");
-}
-
-function handleRequest(data)
-{
-

[anton, 2018/08/27 05:57:10]

do we need this empty line?

[diegocarloslima, 2018/08/27 21:19:16]

Acknowledged.

+  browser.runtime.sendAbbMessage("OnFiltersSaved");
+}
+
+function handleMessage(data)
+{
   switch ((data["action"]))
   {
     case "getAdblockPlusEnabled":
       return successData(Prefs.enabled);
 
     case "setAdblockPlusEnabled":
       if (!checkData(data, ENABLE_PROP)) break;
       Prefs.enabled = !!data[ENABLE_PROP];
       return successData();
 
     case "getAcceptableAdsEnabled":
       return successData(isSubscriptionEnabled(
           Prefs.subscriptions_exceptionsurl));
 
     case "setAcceptableAdsEnabled":
       if (!checkData(data, ENABLE_PROP)) break;
       const acceptableAdsTitle = "Allow non-intrusive advertising";
       setSubscriptionEnabled(!!data[ENABLE_PROP],
           Prefs.subscriptions_exceptionsurl, acceptableAdsTitle);
-      return successData(getAllSubscriptions());

[Thomas Greiner, 2018/08/27 15:24:41]

The previous version didn't return anything but I couldn't find where this
function is being used to check whether the return value is actually being used.

My assumption would be that it isn't since the `getAllSubscriptions()` function
is preceded by a "remove" comment. If that's the case let's remove the return
value and the `getAllSubscriptions()` function.

[diegocarloslima, 2018/08/27 21:19:16]

Yeah, this was some test code that I forgot to remove ^^

+      return successData();
 
     case "getEnabledSubscriptions":
       return successData(getEnabledSubscriptions());
 
     case "isSubscriptionEnabled":
       if (!checkData(data, URL_PROP)) break;
       return successData(isSubscriptionEnabled(data[URL_PROP]));
 
     case "addSubscription":
       if (!checkData(data, URL_PROP)) break;
@@ skipping 12 matching lines @@
       if (!checkData(data, [URL_PROP, HOST_PROP])) break;
       return successData(isDomainWhitelisted(data[URL_PROP], data[HOST_PROP]));
 
     case "whitelistDomain":
       if (!checkData(data, [ENABLE_PROP, URL_PROP, HOST_PROP])) break;
       setDomainWhitelisted(!!data[ENABLE_PROP], data[URL_PROP], data[HOST_PROP]);
       return successData();
 
   }
   return errorData("Malformed request");
-}
-
-// remove
-function getAllSubscriptions()
-{
-  return FilterStorage.subscriptions.map(sub => ({"title": sub.title, "url": sub.url, "downloadable": (sub instanceof DownloadableSubscription), "special": (sub instanceof SpecialSubscription), "disabled": sub.disabled}));
 }
 
 function getEnabledSubscriptions()
 {
   return FilterStorage.subscriptions.filter(sub => !sub.disabled).map(sub =>
       ({[TITLE_PROP]: sub.title, [URL_PROP]: sub.url}));
 }
 
 function isSubscriptionEnabled(url)
 {
@@ skipping 37 matching lines @@
 {
   FilterStorage.removeFilter(filter);
   if (filter.subscriptions.length) filter.disabled = true;
 }
 
 function getWhitelistedDomains()
 {
   const whitelistRegex = /^@@\|\|([^/:]+)\^\$document$/;
   const results = [];
 
-  FilterStorage.subscriptions.forEach(function(sub) {
-    if (!(sub instanceof SpecialSubscription) || sub.disabled) return;
-    sub.filters.forEach(function(filter) {

[Thomas Greiner, 2018/08/27 15:24:41]

Suggestion: If you want you can use a for-of loop instead of
`Array.prototype.forEach()`.

i.e. for (let filter of filters)

That way those loops will also be compatible with `Set` when we change the
implementation of the data structure in Core (see #6559).

[diegocarloslima, 2018/08/27 21:19:16]

Acknowledged.

+  for (const sub of FilterStorage.subscriptions)
+  {
+    if (!(sub instanceof SpecialSubscription) || sub.disabled) continue;
+    for (const filter of sub.filters)
+    {
       const match = filter.text.match(whitelistRegex);
       if (match) results.push({[URL_PROP]: match[1]});
-    });
-  });
+    }
+  }
 
   return results;
-}
-
-function flatIt(array)

[Thomas Greiner, 2018/08/27 15:24:41]

Detail: You can more efficiently implement that using

Array.prototype.concat.apply([], array)

at least until Firefox 62 which will add `Array.prototype.flat()` so we could
even polyfill it until then.

[diegocarloslima, 2018/08/27 21:19:16]

This was also some test code ^^

-{
-  let flatArray = [];
-  array.forEach(function(item) {
-    flatArray = flatArray.concat(item);
-  });
-  return flatArray;
 }
 
 function isDomainWhitelisted(url, host)
 {
   return !!getWhitelistingFilter(url, host);
 }
 
 function setDomainWhitelisted(isWhitelisted, url, host)
 {
   if (isDomainWhitelisted(url, host) == isWhitelisted) return;
@@ skipping 24 matching lines @@
         url, RegExpFilter.typeMap.DOCUMENT, host, false, null, false);
   }
   catch (e) {}
   return null;
 }
 
 function checkData(data, check)
 {
   if (!data) return false;
   const properties = [].concat(check || []);
-  return properties.every(function(item) {
+  return properties.every(function(item)
+  {
     return item in data;
   });
 }
 
 function successData(value)
 {
   const data = {};
   data.success = true;
   if (value != null) data.value = value;
   return data;
 }
 
 function errorData(errorMsg)
 {
   const data = {};
-  if (errorMsg) data.errorMsg = errorMsg;

[Thomas Greiner, 2018/08/27 15:24:42]

The endpoint in ext-c-runtime.js expects this property to be called "error", not
"errorMsg".

Also the success value doesn't appear to be accessed in ext-c-runtime.js so
maybe I'm just misunderstanding how the implementation works. So feel free to
ignore my comment in case it doesn't apply.

[diegocarloslima, 2018/08/27 21:19:16]

Acknowledged.

+  if (errorMsg) data.error = errorMsg;
   return data;
 }
 
 init();