Closed
Bug 820841
Opened 12 years ago
Closed 12 years ago
the site is not opening properly due to NS_ERROR_UNEXPECTED: Unexpected error
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
VERIFIED
FIXED
mozilla20
Tracking | Status | |
---|---|---|
firefox19 | --- | unaffected |
firefox20 | + | verified |
People
(Reporter: alice0775, Assigned: emk)
References
()
Details
(Keywords: regression)
Attachments
(2 files, 3 obsolete files)
144 bytes,
text/html
|
Details | |
7.42 KB,
patch
|
emk
:
review+
|
Details | Diff | Splinter Review |
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/4dfe323a663d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121211 Firefox/20.0 ID:20121211030855
See http://forums.mozillazine.org/viewtopic.php?p=12542837#p12542837
the site is not opening properly due to NS_ERROR_UNEXPECTED: Unexpected error
Steps to reproduce:
1. Open Error Console
2. Open http://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?path=Win/
3. Wait for a while 15-20sec
Actual results:
No Directory list.
And the following error shows in the Error Console:
Error: NS_ERROR_UNEXPECTED: Unexpected error
Source file: http://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?path=Win/
Line: 156
Expected results:
Directory list will be shown
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/6fa6e55a93b2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121204 Firefox/20.0 ID:20121204030754
Bad:
http://hg.mozilla.org/mozilla-central/rev/0035f77045bc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121204 Firefox/20.0 ID:20121204065427
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6fa6e55a93b2&tochange=0035f77045bc
Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/721a4d74f09e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121203 Firefox/20.0 ID:20121203163341
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f2e450529466
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121203 Firefox/20.0 ID:20121203172641
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=721a4d74f09e&tochange=f2e450529466
Suspected: Bug 816410
Assignee | ||
Comment 1•12 years ago
|
||
DOMParser appears to fail to get the doc here...
https://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMParser.cpp?rev=5ed9e56c7a5d#428
Comment 2•12 years ago
|
||
Duplicate or related to bug 771270 perhaps?
Comment 3•12 years ago
|
||
It is fine in loading the infinite loading is since start issue but this issue is not showing me folders of Chromium snapshots like they use to do last week.
Open this page in Chromium and wait for a while you will see Chromium snapshot folder while now in Nightly (current) I get only above FTP things but not folders.
Assignee | ||
Comment 4•12 years ago
|
||
Assignee | ||
Comment 5•12 years ago
|
||
We need to do ourselves what XPC binding did for us.
Attachment #691393 -
Flags: review?(bzbarsky)
Comment 6•12 years ago
|
||
Hmm. Is the page creating a DOMParser from an unloaded window or something?
Operating on the wrong inner is pretty weird, since it'll obviously give the wrong base URI....
I would rather we exposed the base URI and document URI on the window (forwarding to doc when we have it, caching when we do not, just like how we treat principals for nsGlobalWindow::GetPrincipal) if that's all we need "doc" for here.
Assignee | ||
Comment 7•12 years ago
|
||
The page is creating a DOMParser right after docment.write()-after-onload which changes the inner window. Although the document will be blanked, it's intentional. See the attached testcase.
Comment 8•12 years ago
|
||
Hmm, are we nowadays passing different inner window to DOMParser in that case?
IIRC pre-Paris-Bindings passed the current inner window, so in this case the new one.
Comment 9•12 years ago
|
||
Comment 10•12 years ago
|
||
This is somewhat similar to Bug 430276.
Comment 11•12 years ago
|
||
> Hmm, are we nowadays passing different inner window to DOMParser in that case?
Yes. WebIDL bindings pass the global of the object involved, which is the inner window it comes from, not whatever inner happens to be current in the outer right now. That's obviously the only sane thing from a security perspective (though the hackery in classinfo there was sort of ok, I guess).
Bobby, Peter, do you think we should switch to the old behavior?
Comment 12•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #11)
> Bobby, Peter, do you think we should switch to the old behavior?
The old behavior does indeed seem quite insane, because it effectively means that we're unwrapping a security wrapper someone in there (in order to link the DOMParser() in the old inner's scope to the document in the new inner). I think we should do everything possible to not do that.
Comment 13•12 years ago
|
||
Well, what is that "everything possible"? We need to be able to use DOMParser even after
document.write call. It is really document.write which is insane ;)
Assignee | ||
Comment 14•12 years ago
|
||
Implementing the idea written in comment #6.
Attachment #691393 -
Attachment is obsolete: true
Attachment #691393 -
Flags: review?(bzbarsky)
Attachment #691823 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 15•12 years ago
|
||
Forgot a null check...
Attachment #691823 -
Attachment is obsolete: true
Attachment #691823 -
Flags: review?(bzbarsky)
Attachment #691836 -
Flags: review?(bzbarsky)
Comment 16•12 years ago
|
||
> We need to be able to use DOMParser even after document.write call.
Yes. The question is how to do that, and whether it should affect what window is passed to _all_ constructors.
Comment 17•12 years ago
|
||
One interesting question is what |new Image()| should do after document.write...
Comment 18•12 years ago
|
||
Comment on attachment 691836 [details] [diff] [review]
Cache the document URI on the window so that DOMParser() can refer
r=me, I think. We should still figure out what should happen for things like Image, though. :(
Attachment #691836 -
Flags: review?(bzbarsky) → review+
Comment 19•12 years ago
|
||
I guess one could argue that there should be no difference between "new Image" and "new window.Image". Which right now on trunk there is, because the latter passes in the current inner.
So maybe we should in fact do what the old constructor code did.
Assignee | ||
Comment 20•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #19)
> Which right now on trunk there is, because
> the latter passes in the current inner.
Is Image already converted to Paris bindings?
Comment 21•12 years ago
|
||
> Is Image already converted to Paris bindings?
No, which is why it gets the current inner right now.
Assignee | ||
Comment 22•12 years ago
|
||
The previous patch didn't build on non-Windows.
https://tbpl.mozilla.org/?tree=Try&rev=9a38d01af35a
Currently panda and unagi seems to be busted on try.
Attachment #691836 -
Attachment is obsolete: true
Attachment #692030 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 23•12 years ago
|
||
Please file a separate bug for other constructors requiring the current inner.
Comment 24•12 years ago
|
||
It's not quite a separate bug, because the question is what window/document should be used for constructors in general... It'd be weird if XHR and DOMParser used different base URIs, for example.
Assignee | ||
Comment 25•12 years ago
|
||
Hm. Then should we fix codegen to ensure that aOwner is the current inner? If so, I'll withdraw the current patch atm.
Keywords: checkin-needed
Comment 26•12 years ago
|
||
That's what I'm trying to figure out, yes. Peter, thoughts?
Another option is to keep the aOwner as-is but expose a helper method for getting a window out of aOwner... but I'm not sure that's really that much better in terms of usability.
Assignee | ||
Comment 27•12 years ago
|
||
If we use the current inner anyway, why do we need to create a new inner in the first place?
Comment 28•12 years ago
|
||
I'm not sure I follow the question in comment 27. Create a new inner where?
Assignee | ||
Comment 29•12 years ago
|
||
Didn't document.write() create an inner window?
Anyway, I found it was required by the spec.
http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#dom-document-open
> 14. Replace the Document's singleton objects with new instances of those
> objects. (This includes in particular the Window, Location, History,
> ApplicationCache, and Navigator, objects, the various BarProp objects,
> the two Storage objects, the various HTMLCollection objects, and objects
> defined by other specifications, like Selection and the document's
> UndoManager. It also includes all the Web IDL prototypes in the JavaScript
> binding, including the Document object's prototype.)
So we actually need to live with the different inner.
Comment 30•12 years ago
|
||
Yes, the open() has to create a new inner, because it's completely clearing the global scope etc....
Updated•12 years ago
|
Assignee | ||
Comment 31•12 years ago
|
||
Landed as a workaround before uplifting.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6cff4956629
Flags: in-testsuite+
Target Milestone: --- → mozilla20
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Comment 32•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 33•12 years ago
|
||
I confirm the fix is verified on FF 20.b1:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0(20130220104816)
Status: RESOLVED → VERIFIED
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
•