Closed Bug 972478 Opened 6 years ago Closed 6 years ago

Disabling script on a docshell should not affect chrome pages

Categories

(Core :: XPConnect, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox28 --- fixed
firefox29 --- fixed
firefox30 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

(Keywords: regression)

Attachments

(1 file)

While investigating things for bug 971650, I realized that this seems to be broken. Patch coming up.
(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.
Attachment #8375685 - Flags: approval-mozilla-beta?
Attachment #8375685 - Flags: approval-mozilla-aurora?
I will wait it reaches m-c before uplifting it.
https://hg.mozilla.org/mozilla-central/rev/60e089842cb6
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
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 ;)
Attachment #8375685 - Flags: approval-mozilla-beta?
Attachment #8375685 - Flags: approval-mozilla-beta+
Attachment #8375685 - Flags: approval-mozilla-aurora?
Attachment #8375685 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.