Closed Bug 820841 Opened 11 years ago Closed 11 years ago

the site is not opening properly due to NS_ERROR_UNEXPECTED: Unexpected error

Categories

(Core :: DOM: Core & HTML, defect)

20 Branch
x86
Windows 7
defect
Not set
normal

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)

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
Duplicate or related to bug 771270 perhaps?
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.
Attached file Reduced testcase
Attached patch Ensure the current inner window (obsolete) — Splinter Review
We need to do ourselves what XPC binding did for us.
Attachment #691393 - Flags: review?(bzbarsky)
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.
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.
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.
This is somewhat similar to Bug 430276.
> 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?
(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.
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 ;)
Implementing the idea written in comment #6.
Attachment #691393 - Attachment is obsolete: true
Attachment #691393 - Flags: review?(bzbarsky)
Attachment #691823 - Flags: review?(bzbarsky)
Forgot a null check...
Attachment #691823 - Attachment is obsolete: true
Attachment #691823 - Flags: review?(bzbarsky)
Attachment #691836 - Flags: review?(bzbarsky)
> 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.
One interesting question is what |new Image()| should do after document.write...
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+
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.
(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?
> Is Image already converted to Paris bindings?

No, which is why it gets the current inner right now.
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+
Keywords: checkin-needed
Please file a separate bug for other constructors requiring the current inner.
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.
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
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.
If we use the current inner anyway, why do we need to create a new inner in the first place?
I'm not sure I follow the question in comment 27.  Create a new inner where?
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.
Yes, the open() has to create a new inner, because it's completely clearing the global scope etc....
Landed as a workaround before uplifting.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6cff4956629
Flags: in-testsuite+
Target Milestone: --- → mozilla20
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/d6cff4956629
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: