Closed
Bug 972478
Opened 11 years ago
Closed 11 years ago
Disabling script on a docshell should not affect chrome pages
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: bholley, Assigned: bholley)
References
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
While investigating things for bug 971650, I realized that this seems to be broken. Patch coming up.
Comment 1•11 years ago
|
||
Wait. Why shouldn't it?
Assignee | ||
Comment 2•11 years ago
|
||
(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.
Assignee | ||
Comment 3•11 years ago
|
||
(Chrome is unaffected by Domain Policy, for example, even in whitelist mode).
Comment 4•11 years ago
|
||
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.
Assignee | ||
Comment 5•11 years ago
|
||
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.
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #8375685 -
Flags: review?(bzbarsky)
Comment 7•11 years ago
|
||
Comment on attachment 8375685 [details] [diff] [review]
Docshell scriptability should only affect non-immune principals. v1
r=me
Attachment #8375685 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Assignee | ||
Comment 9•11 years ago
|
||
Assignee | ||
Comment 10•11 years ago
|
||
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?
Comment 11•11 years ago
|
||
I will wait it reaches m-c before uplifting it.
Comment 12•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Assignee | ||
Comment 13•11 years ago
|
||
(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.
Comment 14•11 years ago
|
||
No worries! I am just explaining since some developers have different expectations ;)
Updated•11 years ago
|
Attachment #8375685 -
Flags: approval-mozilla-beta?
Attachment #8375685 -
Flags: approval-mozilla-beta+
Attachment #8375685 -
Flags: approval-mozilla-aurora?
Attachment #8375685 -
Flags: approval-mozilla-aurora+
Comment 15•11 years ago
|
||
Comment 16•11 years ago
|
||
Updated•11 years ago
|
status-b2g-v1.3:
--- → fixed
status-b2g-v1.4:
--- → fixed
Updated•11 years ago
|
status-b2g-v1.3T:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•