Issue 4494 - Avoid causing some sandbox related warnings

inject.preload.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 370 matching lines @@
 
   if ("RTCPeerConnection" in window)
     window.RTCPeerConnection = boundWrappedRTCPeerConnection;
   if ("webkitRTCPeerConnection" in window)
     window.webkitRTCPeerConnection = boundWrappedRTCPeerConnection;
 }
 
 if (document instanceof HTMLDocument)
 {
   let sandbox = window.frameElement &&
-                  window.frameElement.getAttribute("sandbox");

[Sebastian Noack, 2017/05/30 09:41:16]

Nit: The indentation looks a bit off here.

[kzar, 2017/05/30 10:20:31]

Done.

-  if (typeof sandbox != "string" || sandbox.includes("allow-scripts"))

[Sebastian Noack, 2017/05/30 09:41:16]

What if allow-scripts is misspelled like "allow-scriptss"?

[kzar, 2017/05/30 10:20:31]

Well a false-positive doesn't matter here, it just results in the warning being
displayed.

[Sebastian Noack, 2017/05/30 10:36:46]

I wonder whether we even need this check. If scripts are not allowed this code
won't run anyway, right?

[Sebastian Noack, 2017/05/30 10:43:34]

Never mind. We are in the extension's content script here, I mistakenly assumed
that this is part of the injected code.

Anyway, what I was initially suggesting here, was to use
/(^|\s)allow-scripts($|\s)/.test(sandbox), which would be a little more
accurrate. But it's not too important. I leave it up to you. LGTM either way.

[kzar, 2017/05/30 12:27:15]

Doing some further reading I realised that
window.frameElement.sandbox.contains("allow-scripts") would be even better. But
then I realised DOMTokenList is case sensitive, so while "ALLOW-SCRIPTS" would
work, we wouldn't detect it! This was really bad, giving websites a way to
circumvent our wrappers. So instead I've gone with your approach, adding the
case insensitive flag. Done.

+                window.frameElement.getAttribute("sandbox");
+
+  if (typeof sandbox != "string" || /(^|\s)allow-scripts(\s|$)/i.test(sandbox))
   {
     let script = document.createElement("script");
     script.type = "application/javascript";
     script.async = false;
     script.textContent = "(" + injected + ")('" + randomEventName + "');";
     document.documentElement.appendChild(script);
     document.documentElement.removeChild(script);
   }
 }