Issue 4494 - Avoid causing some sandbox related warnings

inject.preload.js

Index: inject.preload.js
diff --git a/inject.preload.js b/inject.preload.js
index c35c071ce48f748401a0e3a3de6f267b51712f37..3b1e1f61bb9da682e7202f8a9549c82ad2fcb248 100644
--- a/inject.preload.js
+++ b/inject.preload.js
@@ -387,10 +387,15 @@ function injected(eventName, injectedIntoContentWindow)
 
 if (document instanceof HTMLDocument)
 {
-  let script = document.createElement("script");
-  script.type = "application/javascript";
-  script.async = false;
-  script.textContent = "(" + injected + ")('" + randomEventName + "');";
-  document.documentElement.appendChild(script);
-  document.documentElement.removeChild(script);
+  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.

+  {
+    let script = document.createElement("script");
+    script.type = "application/javascript";
+    script.async = false;
+    script.textContent = "(" + injected + ")('" + randomEventName + "');";
+    document.documentElement.appendChild(script);
+    document.documentElement.removeChild(script);
+  }
 }