Closed
Bug 741267
Opened 13 years ago
Closed 13 years ago
UserScript's XMLHttpRequest is undefined in 20120401 nightly
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: dindog, Assigned: peterv)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
bzbarsky
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Both Greasemonkey and Scriptish run userscript with XMLHttpRequest will dump "ReferenceError: XMLHttpRequest is not defined".
I'm pretty sure those script work fine in 20120330 nightly build.
Probably regression from Bug 741163.
Sounds like XrayWrapper fun.
Assignee | ||
Comment 2•13 years ago
|
||
We're resolving on the global of the XPCWrappedNativeScope we get from the sandbox global's proto, but we should resolve on the proto itself.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Comment 3•13 years ago
|
||
Looks like we are hitting the same bug in Jetpack, because of this:
var Cu = Components.utils;
var s = Cu.Sandbox(content);
s.win = content;
Cu.evalInSandbox("win.XMLHttpRequest()", s);
// Exception on nightly: win.XMLHttpRequest is not a function
// While it runs without exception in FF11
This is breaking Jetpack tests, so it's a P1 from our view.
Assignee | ||
Comment 5•13 years ago
|
||
Not sure how to make the difference between aObject and aGlobal clearer. aDefineOn for aObject?
Attachment #612686 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 6•13 years ago
|
||
Sorry, wrong patch.
Attachment #612686 -
Attachment is obsolete: true
Attachment #612686 -
Flags: review?(bzbarsky)
Attachment #612688 -
Flags: review?(bzbarsky)
Comment 7•13 years ago
|
||
Comment on attachment 612688 [details] [diff] [review]
v1
So is the |obj| passed to CreateInterfaceObjects sometimes not a global now? Does using it as the parent for the protos and interface objects still make sense? Or should we use the global there? Will |obj| in practice end up being a security wrapper here, or the window itself?
As far as naming goes, perhaps aReceiver for aObject?
Assignee | ||
Comment 8•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #7)
> So is the |obj| passed to CreateInterfaceObjects sometimes not a global now?
Yes. It can be a wrapper for a global, set as the proto of the sandbox global.
> Does using it as the parent for the protos and interface objects still make
> sense? Or should we use the global there?
I think it's fine, but are you worried about something in particular.
> Will |obj| in practice end up
> being a security wrapper here, or the window itself?
Security wrapper.
Comment 9•13 years ago
|
||
> I think it's fine, but are you worried about something in particular.
Are we putting these things that are parented to "obj" in the list attached to global?
Assignee | ||
Comment 10•13 years ago
|
||
Attachment #612688 -
Attachment is obsolete: true
Attachment #612688 -
Flags: review?(bzbarsky)
Attachment #617019 -
Flags: review?(bzbarsky)
Comment 11•13 years ago
|
||
Comment on attachment 617019 [details] [diff] [review]
v1.1
Please write a more useful checkin comment, one that describes the receiver/global split, and r=me
One question, though: With this setup we stash the interface/proto object pointers in the sandbox global's proto array, right? But we resolve them on the receiver (the xray for the window), and stick the actual JSObject refs into the sandbox's holder? And we parent the objects to the sandbox global? Is that basically the setup?
Attachment #617019 -
Flags: review?(bzbarsky) → review+
Comment 12•13 years ago
|
||
Also, does this cause us to preserve the xray?
Comment 13•13 years ago
|
||
This breaks XHR from jetpack right now so we'd really like to get this landed a.s.a.p. and look into putting it on aurora too.
tracking-firefox14:
--- → ?
Assignee | ||
Comment 14•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #11)
> One question, though: With this setup we stash the interface/proto object
> pointers in the sandbox global's proto array, right? But we resolve them on
> the receiver (the xray for the window), and stick the actual JSObject refs
> into the sandbox's holder? And we parent the objects to the sandbox global?
Yes, assuming that by "into the sandbox's holder" you mean "on the receiver's holder".
(In reply to Boris Zbarsky (:bz) from comment #12)
> Also, does this cause us to preserve the xray?
Not preserve as in PreserveWrapper. Of course the prototypes hold their parent (the sandbox global) and the global holds its prototype.
Assignee | ||
Comment 15•13 years ago
|
||
Comment 16•13 years ago
|
||
I guess what I was really trying to ask is this. If another Xray for the window is created, will it have those objects already in its holder? Or do we not coalesce Xrays?
Comment 17•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Comment 18•13 years ago
|
||
Tracking for FF14. Please nominate for Aurora approval asap - my understanding is that this fixes broken jetpack tests.
Comment 19•13 years ago
|
||
It doesn't only fix jetpack tests but fix XMLHttpRequest usages jetpack addons too!
So it is really important to get this included Aurora.
Otherwise, XMLHttpRequest() now works, but some other stuff are still broken, like:
var Cu = Components.utils;
var s = Cu.Sandbox(content);
s.win = content;
Cu.evalInSandbox("win.XMLHttpRequest.toString()", s);
/*
Exception: Function.prototype.toString called on incompatible object
@Scratchpad:4
*/
Should I open another bug? Or reopen this one?
Comment 20•13 years ago
|
||
> Exception: Function.prototype.toString called on incompatible object
Bug 742156 covers this.
Comment 21•13 years ago
|
||
Though the patch in bug 748983 would also fix this for the case of XHR.
Assignee | ||
Comment 22•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #16)
> I guess what I was really trying to ask is this. If another Xray for the
> window is created, will it have those objects already in its holder?
Every Xray has its own holder.
Assignee | ||
Comment 23•13 years ago
|
||
Comment on attachment 617019 [details] [diff] [review]
v1.1
[Approval Request Comment]
Regression caused by (bug #): bug 740069
User impact if declined: Jetpack-based addons using XMLHttpRequest will be broken
Testing completed (on m-c, etc.): landed on m-c on 2012-04-29, has an automated test (and is also tested by Jetpack tests)
Risk to taking this patch (and alternatives if risky): low-risk
String changes made by this patch: None
Attachment #617019 -
Flags: approval-mozilla-aurora?
Comment 24•13 years ago
|
||
Comment on attachment 617019 [details] [diff] [review]
v1.1
[Triage Comment]
Given where we are in the dev cycle, and our concern with the impact of this bug, approving for Aurora 14.
Attachment #617019 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 25•13 years ago
|
||
status-firefox14:
--- → fixed
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•