Closed
Bug 307632
Opened 19 years ago
Closed 19 years ago
Can't extend Selection Class anymore with Firefox 1.5b1
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
People
(Reporter: chregu, Assigned: dbradley)
References
()
Details
(Keywords: fixed1.8)
Attachments
(1 file)
(deleted),
patch
|
mrbkap
:
review+
brendan
:
superreview+
asa
:
approval1.8b5+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Extending the Selection class with new prototypes seems not to work anymore in
Firefox 1.5b1:
Selection.prototype.foo = function() {
alert("foo called");
}
did work with 1.0.x (and everything before), but now I get
Error: sel.foo is not a function
Now my editor (bxe.oscom.org) doesn't work anymore, which heavily relies on
extending the selection class. Others (like mozile) will have the same problem,
I assume.
Reproducible: Always
Steps to Reproduce:
1. Go to URL http://trash.chregu.tv/selection.prototype.bug.html
2. Click on click here
3.
Actual Results:
Nothing. Error in JS Console
Expected Results:
An alert box with "foo called" should appear.
Comment 1•19 years ago
|
||
bz, what are the conditions (if any) that people can set "expando" properties on
the dom in these days of XPC Wrappers ?
Component: JavaScript Engine → XPConnect
Summary: Can't extend Selection Class anymore with Firefox 1.5b1 → Can't extend Selection Class anymore with Firefox 1.5b1
Updated•19 years ago
|
Assignee: general → dbradley
QA Contact: general → pschwartau
Comment 2•19 years ago
|
||
There are no XPCNativeWrappers involved (but in any case, XPCNativeWrapper
expand stuff is documented at
http://developer.mozilla.org/en/docs/XPCNativeWrapper#Setting_.22expando.22_properties_on_XPCNativeWrapper
-- let me know if that's not clear enough, ok?).
This bug is almost certainly splitwindow fallout. The basic issue is that the
prototype of what getSelection() returns is something that lives on the outer
window, while the code in the page has changed Selection.prototype on the
_inner_ window. This happens because the getSelection call is made on the outer
window and hence in the outer XPConnect scope, and so that's the scope that
XPConnect uses when it does NativeInterface2JSObject.
Now selection _is_ something that lives on the outer window. Sorta. It lives
in the presshell, but we go through the docshell to get the presshell for some
odd reason. What would make more sense to me is to forward selection-getting to
the inner and to just get the presshell off mDocument.... And then we can
happily change things so selection is wrapped in the inner window scope (via
classinfo). Right?
Blocks: splitwindows
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8b5?
OS: MacOS X → All
Hardware: Macintosh → All
Comment 3•19 years ago
|
||
Though that approach really will not help with other things (eg window.location,
window.navigator) that _do_ live on the outer window and hence can't be wrapped
in the inner's scope.
Do we want to just fix selection, or try to come up with something that works in
general?
Comment 4•19 years ago
|
||
(In reply to comment #3)
I would say in general as this is a common use case.
Comment 5•19 years ago
|
||
Could we perhaps define the standard classes on the proto of the window, not on
the window itself? Since the two windows share the proto chain, maybe that will
work?
Comment 6•19 years ago
|
||
This fixes this bug by making us always wrap all DOM things in an inner scope.
We use the current inner here since that's our best guess here. This way we
won't end up with different wrappers for the selection object when using
"selection" vs "window.selection" (where in the latter case the scope we wrap
the selection object in is the window's scope, i.e. the outer).
Attachment #195818 -
Flags: superreview?(brendan)
Attachment #195818 -
Flags: review?(mrbkap)
Comment 7•19 years ago
|
||
Comment on attachment 195818 [details] [diff] [review]
Use the current inner scope when wrapping natives in an outer window's scope.
sr=me ahead of mrbkap, he better not embarrass me ;-).
/be
Attachment #195818 -
Flags: superreview?(brendan) → superreview+
Comment 8•19 years ago
|
||
Comment on attachment 195818 [details] [diff] [review]
Use the current inner scope when wrapping natives in an outer window's scope.
r=mrbkap
Attachment #195818 -
Flags: review?(mrbkap) → review+
Updated•19 years ago
|
Attachment #195818 -
Flags: approval1.8b5?
Comment 9•19 years ago
|
||
This bug is must-fix, it restores DOM compatibility that goes back over seven
years (almost nine years for DOM level 0 objects).
/be
Flags: blocking1.8b5? → blocking1.8b5+
Comment 10•19 years ago
|
||
Er... so how does wrapping in the inner scope work for things that outlive the
inner window? Won't they basically keep the inner window alive while they're
alive? And always have its permissions, which is worse, I think?
I'm thinking things like window.location, window.navigator, etc.
Comment 11•19 years ago
|
||
(In reply to comment #10)
> Er... so how does wrapping in the inner scope work for things that outlive the
> inner window? Won't they basically keep the inner window alive while they're
> alive? And always have its permissions, which is worse, I think?
>
> I'm thinking things like window.location, window.navigator, etc.
window.location and navigator, and DOM nodes, and window objects already have
pre-create hooks much like this one that does the exact same thing (well,
window.location and navigator's pre-create hooks actually do the opposite).
There's no security problems there AFAICT, the principals remain unchanged now,
unlike before the split window landing, and besides, that'd only be a problem if
a site could inject code onto an object and get it to inherit the objec's
principals, which AFAIK is not possible. Brendan, correct me if I'm wrong here.
This landed on the trunk btw, marking FIXED.
Status: NEW → RESOLVED
Closed: 19 years ago
Flags: blocking1.8b5+ → blocking1.8b5?
Resolution: --- → FIXED
Comment 12•19 years ago
|
||
Ah, ok. The precreate hooks on location and navigator are what makes this work.
Good.
Updated•19 years ago
|
Flags: blocking1.8b5? → blocking1.8b5+
Updated•19 years ago
|
Attachment #195818 -
Flags: approval1.8b5? → approval1.8b5+
You need to log in
before you can comment on or make changes to this bug.
Description
•