1631305 - Remove unnecessary content policy check to prevent loading scripts.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, task, P3)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1631305 - Remove unnecessary and incorrect content policy check. r=bholley

This check is unnecessary. We have a central place to compute whether
scriptability is allowed that already accounts for the docshell flag,
see Scriptability::SetDocShellAllowsScript, from bug 840488:

https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/docshell/base/nsDocShell.cpp#2532

It is also incorrect, for two reasons:

• It should really be GetCanExecuteScripts(), so that it works properly
  in subframes.

• We have a whitelist of principals that should always be allowed to
  execute script (see xpc::PrincipalImmuneToScriptPolicy), which is not
  accounted for here. So if we load an add-on document (or pdf.js after
  bug 866634) in a docshell with disabled script execution, we still
  won't load the out-of-line scripts.

The latter is what's happening in bug 810815, as TB uses the docshell
flag to disable script execution on emails.

This is still a bit of a behavior change in the sense that after this,
IIUC, scripts should download but not execute. I think that's fine and
we have no test that depends on the current behavior.

An alternative to this patch would be to change this check to get the
docshell's document's node principal, and check that against
PrincipalImmuneToScriptPolicy (and while at it fix the check to use
GetCanExecuteScripts()).

But this seems simpler.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b987703f21f0
Remove unnecessary and incorrect content policy check. r=ckerschb

Comment 3

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/b987703f21f0