Flip xpcnativewrappers default to yes.

RESOLVED FIXED in mozilla1.8beta4

Status

()

Core
DOM
P1
normal
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: dveditz, Assigned: Benjamin Smedberg)

Tracking

({fixed1.8})

Trunk
mozilla1.8beta4
fixed1.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8b4 +
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [no l10n impact] has-patch, needs review/approval brendan+bz)

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
We implemented the ability for chrome to ask for xpcnativewrappers=yes, now we
need to make "yes" the default and force extensions that don't want wrappers to
explicitly request the insecure default.

We announced we were going to do this at
http://developer.mozilla.org/devnews/index.php/2005/07/13/xpcnativewrappers-to-be-turned-on-by-default/
now we need to follow through.

This *will* break some extensions. It *must* be done by beta if we are going to
do it.

Broken extensions can quickly go back to unbroken (but probably exploitable) by
turning off automatic wrappers. Since they will have to make at least some
change to deal with the version update to "1.4" the beta is a doubly good time
to make this switch, rather than break extensions twice.

The manifest file setting should be added to the documentation at
http://developer.mozilla.org/en/docs/Safely_accessing_content_DOM_from_chrome

Brendan though bsmedberg had signed up for this. I hope this is not a dupe, but
I didn't find a bug and want to make sure this does not slip through the cracks
(Reporter)

Updated

13 years ago
Flags: blocking1.8b4+
(Assignee)

Comment 1

13 years ago
Created attachment 190699 [details] [diff] [review]
Flip the default
(Assignee)

Updated

13 years ago
Attachment #190699 - Flags: superreview?(bzbarsky)
Attachment #190699 - Flags: review?(brendan)
(Assignee)

Updated

13 years ago
Priority: -- → P1
Whiteboard: [no l10n impact] has-patch, needs review/approval brendan+bz
Target Milestone: --- → mozilla1.8beta4
Comment on attachment 190699 [details] [diff] [review]
Flip the default

This will break our in-tree extensions (DOM inspector at the very least). 
Please don't make this change without at the same time changing all packages in
the tree that don't use xpcnativewrappers=yes to using xpcnativewrappers=no...
(perhaps we should have explicitly done that up front :( ).
Attachment #190699 - Flags: superreview?(bzbarsky) → superreview-
Blocks: 290324
Blocks: 302022
Blocks: 302177

Updated

13 years ago
No longer blocks: 302022

Updated

13 years ago
Assignee: benjamin → general
Flags: blocking1.8b5+
(Assignee)

Comment 3

13 years ago
Schrep: is there a reason you reassigned this?
Assignee: general → benjamin
Comment on attachment 190699 [details] [diff] [review]
Flip the default

Benjamin, are you still working on this?  We need at least a domi patch along
with this one.

/be
Attachment #190699 - Flags: review?(brendan)

Comment 5

13 years ago
Nope - mistake while updating other flags.  Sorry about that.  
(Assignee)

Comment 6

13 years ago
Created attachment 193736 [details] [diff] [review]
Flip DOMI
Attachment #193736 - Flags: superreview?(brendan)
Attachment #193736 - Flags: review?(bzbarsky)
(Assignee)

Updated

13 years ago
Attachment #190699 - Flags: superreview?(brendan)
Attachment #190699 - Flags: superreview-
Attachment #190699 - Flags: review?(bzbarsky)
Comment on attachment 193736 [details] [diff] [review]
Flip DOMI

Don't you need to change contents.rdf to fix this for seamonkey?  Or am I
missing something?  Certainly the various places that use "yes" have
chrome:xpcNativeWrappers="true"  in their contents.rdf
(Assignee)

Comment 8

13 years ago
I am not flipping the default for the seamonkey chrome registry (indeed I do not
know how to do so).
Also, doesn't that change still leave at least venkman stuck with
XPCNativeWrapper, which it may or may not want?  Same for chatzilla.  I know
they wanted to be able to not flip wrappers on until they'd done some testing
with it first.

Do we just need a separate bug for the other chrome registry?  I bet Neil can
flip that one...  Then the contents.rdf changes can happen there, of course.
We don't ship chatzilla or venkman, and they can set whatever
xpcnativewrappers=no  they want in the packages they ship against the beta we're
trying to ship.  If that patch fixes the software that we ship, we should take
it and get closer to getting the beta out -- chatzilla and venkman have had
_months_ with which to test xpcnativewrappers=yes for their own edification, and
they shouldn't be blocking a Firefox beta release.
My point was that I see no indication that we _are_ fixing the software that we
ship.  That is, that someone has actually checked that for all packages we ship
by default either: 1) we have xpcnativewrappers=yes right now, 2) we have
xpcnativewrappers=no after this patch, or 3) we're planning on testing right now
whether they handle xpcnativewrappers.

My apologies if someone has done this checking and simply didn't mention that in
the bug.

That said, as for venkman in particular, it's rather useful for actually
debugging our app most of the time, so if we do break it it gets that much
harder to debug things...  But I'll admit that it's lower priority if we really
need to ship right now and need to get this patch in like tonight.
I want to test it by checking it in on the trunk, because previous attempts to
get that stuff tested have obviously not been sufficiently successful. =)
Attachment #193736 - Flags: review?(bzbarsky) → review+
Comment on attachment 190699 [details] [diff] [review]
Flip the default

r=bzbarsky if we're making sure to turn this off for things we know or suspect
can't handle it.
Attachment #190699 - Flags: review?(bzbarsky) → review+

Comment 14

13 years ago
It can't be flipped on Suite, because we only turn it on if we find any
chrome:xpcNativeWrappers="true" arcs (which means that they survive profile
switch, even to an older extension that doesn't request them, bleh).
Comment on attachment 193736 [details] [diff] [review]
Flip DOMI

sr=shaver
Attachment #193736 - Flags: superreview?(brendan) → superreview+
Comment on attachment 190699 [details] [diff] [review]
Flip the default

sr=shaver
Attachment #190699 - Flags: superreview?(brendan) → superreview+
(Assignee)

Updated

13 years ago
Attachment #190699 - Flags: approval1.8b4?
(Assignee)

Updated

13 years ago
Attachment #193736 - Flags: approval1.8b4?
(Assignee)

Comment 17

13 years ago
Landed on trunk. Still needs to land on branch.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 18

13 years ago
We need verification and insurance that nothing has broken here before we
evaluate for the branch.
This appears to have caused bug 306108.
Depends on: 306108

Updated

13 years ago
Attachment #193736 - Flags: approval1.8b4? → approval1.8b4+

Updated

13 years ago
Attachment #190699 - Flags: approval1.8b4? → approval1.8b4+
(Assignee)

Comment 20

13 years ago
Committed on 1.8 branch.
Keywords: fixed1.8

Updated

12 years ago
Depends on: 306754
You need to log in before you can comment on or make changes to this bug.