[adblockplusui] Issue 4915 - Define ext for background pages in antiadblockInit.js

lib/antiadblockInit.js

Index: lib/antiadblockInit.js
diff --git a/lib/antiadblockInit.js b/lib/antiadblockInit.js
index ac9ef424ab902bf0c3c18a0e0771a6b840d7a429..c7f23b74c27f55b8ccdd055ba39f3857250858a6 100644
--- a/lib/antiadblockInit.js
+++ b/lib/antiadblockInit.js
@@ -25,6 +25,9 @@ let {FilterNotifier} = require("filterNotifier");
 let {Subscription} = require("subscriptionClasses");
 let {Notification} = require("notification");
 
+let ext = typeof window != "undefined" &&

[kzar, 2017/03/02 09:36:30]

I'm not sure this is right, since if ext is already declared using let  then
redeclaring it here will throw an exception. Also are we expecting this code to
run somewhere where window doesn't exist?

I also wasn't sure about the best way to do this, see my attempt here
https://codereview.adblockplus.org/29375899/diff/29377713/messageResponder.js

Thomas what do you think?

[Wladimir Palant, 2017/03/02 11:14:39]

This part is unproblematic, it's a module which means that it always has its own
scope - this code isn't running in global scope and not conflicting with the
globally defined ext variable.
Yes, we do. Currently Firefox, in future maybe libadblockplus and more.
See my comment in this review as to why this approach is a bad one. Too bad that
Sebastian felt that he had to obfuscate my suggestion by misusing logical
operators.

[Thomas Greiner, 2017/03/02 11:41:16]

I assume the confusion here comes from the fact that only some files in the
adblockplusui repository are actual modules.

It'd probably help if we turn the other files in this repo into modules as well.
While I find it personally quite convenient to use `let foo = foo || "foo";` in
certain situations, I do agree with Wladimir that we're overdoing it here in
this case.

Is there any reason not to go with the below variant?

let ext;
if (typeof window != "undefined" && window.ext)
  ext = window.ext; // although ESLint probably forces us to use destructuring
here
else
  ext = require("ext_background");

[wspee, 2017/03/03 09:25:21]

Done. Any objections?

+  window.ext || require("ext_background");
+
 exports.initAntiAdblockNotification = function initAntiAdblockNotification()
 {
   let notification = {