Closed Bug 972478 Opened 6 years ago Closed 6 years ago
Disabling script on a docshell should not affect chrome pages
While investigating things for bug 971650, I realized that this seems to be broken. Patch coming up.
Wait. Why shouldn't it?
(In reply to Boris Zbarsky [:bz] from comment #1) > Wait. Why shouldn't it? Well, it didn't historically - CheckFunctionAccess would always bail out in the SystemPrincipal case. In general, the pattern seems to be that chrome script always gets to run. I'm certainly open to changing this if there are reasons to though.
(Chrome is unaffected by Domain Policy, for example, even in whitelist mode).
Seems to me like if people disable script on a docshell altogether they really mean for it to be disabled no matter what's loaded in there.... but it probably doesn't matter much in practice.
I'm mostly just worried about sloppy addons accidentally disabling browser UI. The docshell flag is a pretty big hammer, and I think it's not great to expand its scope.
Comment on attachment 8375685 [details] [diff] [review] Docshell scriptability should only affect non-immune principals. v1 r=me
Attachment #8375685 - Flags: review?(bzbarsky) → review+
Comment on attachment 8375685 [details] [diff] [review] Docshell scriptability should only affect non-immune principals. v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 840488 User impact if declined: Addons might break browser UI Testing completed (on m-c, etc.): Just pushed to m-i. Risk to taking this patch (and alternatives if risky): low risk. String or IDL/UUID changes made by this patch: None.
I will wait it reaches m-c before uplifting it.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
(In reply to Sylvestre Ledru [:sylvestre] from comment #11) > I will wait it reaches m-c before uplifting it. Yeah totally. With patches like this I just write the approval request early-on so that I don't forget about it, with the assumption that the release drivers will let it bake for a few days before approving it.
No worries! I am just explaining since some developers have different expectations ;)
You need to log in before you can comment on or make changes to this bug.