Closed Bug 382636 Opened 13 years ago Closed 13 years ago
.clone Node(true) triggers "ASSERTION: Could not get loadgroup; onload may fire too early" for each image in the document
###!!! ASSERTION: Could not get loadgroup; onload may fire too early: 'loadGroup', file /Users/jruderman/trunk/mozilla/content/base/src/nsContentUtils.cpp, line 2127 The document has to be .xml (not .xhtml). The number of times the assertion fires is equal to the number of images in the document.
So... We need to either set the container (as we do for documents created via createDocument()) or set the document as loaded as data for the clone. We're trying to do network loads off of it, which is a really bad idea...
Marking blocking for now to keep it on my list. Will punt it if I run out of time though as this doesn't seem very critical.
Assignee: nobody → jonas
Flags: blocking1.9? → blocking1.9+
Er... don't we have the data document content policy for a reason? With this bug, if someone clones a data document, we'll do the loads, which seems like a bad thing... I think we should mark the clone as loaded as data, myself.
I think this is what we want; if we're cloning a "data" document, then mark the clone to be "data" too, and always set the container to be the same as in the original document.
Do you really want to set the container to be the same? Isn't the container only supposed to be set when the document lives inside a docshell?
In general, yes. But not for documents created via createDocument off a DOMImplementation. See bug 325818. Do the same considerations about cx apply here? Or can we just fix that bug instead of perpetuating the hack?
I would say that the same considerations apply. IMHO documents created via createDocument and documents created via cloneNode are more or less equivalent as far as behavior goes.
Sounds like Smaug's copying the container is the right thing to do, if we can't just fix bug 325818. Can we?
Comment on attachment 278032 [details] [diff] [review] possible patch >Index: content/base/public/nsIDocument.h >+ void SetLoadedAsData(PRBool aLoadedAsData) I don't really like having this mutable. Can we make this a (protected?) method on nsXMLDocument and add an arg to NS_NewDOMDocument? And let's get the container thing sorted out. I'd really rather not give non-docshell docs containers if we can at all avoid it...
Comment on attachment 278032 [details] [diff] [review] possible patch This is wrong. And createDocument does bad too. Or at least something must be done to nsDocument::PreHandleEvent. We don't want to propagate events from data documents to |window|, I think.
Or should change nsDocument::GetScriptGlobalObject(). Removing nsDocument::Destroy will do that.
Or we can remove the container thing and see whether the problem that prompted adding it is there... I would vote for that.
Fixed in Bug 393762.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Crashtest checked in.
Flags: in-testsuite? → in-testsuite+
This was fixed in 18.104.22.168 as part of bug 393762, then backed out because of regression bug 414856. Addons can work around it on the trunk, we don't need to back this out except on branch for compatibility reasons.
You need to log in before you can comment on or make changes to this bug.