bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

Invalid xml crashes mozilla after reload [@nsXMLContentSink::Observe]

VERIFIED FIXED in mozilla1.0



16 years ago
16 years ago


(Reporter: Johannes Koskinen, Assigned: peterv)




Firefox Tracking Flags

(Not tracked)


(Whiteboard: [ADT2 RTM][fixed on trunk] [ETA 05/15],custrtm-, crash signature, URL)


(11 attachments, 6 obsolete attachments)

851 bytes, text/xml
850 bytes, text/xml
2.82 KB, text/xml
2.82 KB, text/xml
2.82 KB, text/xml
2.82 KB, text/xml
17.74 KB, patch
Axel Hecht
: review+
Details | Diff | Splinter Review
20.28 KB, patch
Details | Diff | Splinter Review
60 bytes, text/css
66 bytes, text/css
1.40 KB, text/xml


16 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.9.8+) Gecko/20020304
BuildID:    2002030403

When loading xml file, which is invalidate, the mozilla crashes to the next
validate xml file.

The invalidate file could be like

<comment name="foo>sometext</comment>
<item><comment name="foo">sometext</comment</item>

Reproducible: Always
Steps to Reproduce:
1.Generate an invalidate xml file 
2.Correct the file and press Reload
3.The mozilla crashes

Actual Results:  Crash

Expected Results:  In invalidate file Mozilla should tell user that file
couldn't be loaded (now nothing happends). The correct file should be loaded
without crash.

I think this could be duplicate to bug #123953.

Comment 1

16 years ago
TalkBack ID: TB3653801Q

To crash load invalidate.xml and after that try load validate.xml from the same URL.


16 years ago
Severity: major → critical
Keywords: crash
Confirming after I tested this on local disc. I did not see a crash from the
remote site. Download these files:


Then open invalidate.xml, next open validate.xsl and you get a crash here
because mXSLTransformMediator is null:

nsXMLContentSink::Observe(nsXMLContentSink * const 0x041aa9ac, nsISupports *
0x040d1bd8, const char * 0x056add70, const unsigned short * 0x00000000) line 435
+ 38 bytes
nsObserverService::NotifyObservers(nsObserverService * const 0x01226fd0,
nsISupports * 0x040d1bd8, const char * 0x056add70, const unsigned short *
0x00000000) line 213
XSLTProcessor::SignalTransformEnd() line 2495
XSLTProcessor::TransformDocument(XSLTProcessor * const 0x040a32f0, nsIDOMNode *
0x04147d14, nsIDOMNode * 0x0407e1f4, nsIDOMDocument * 0x0413f57c, nsIObserver *
0x040864c4) line 2439
nsTransformMediator::TryToTransform() line 109
nsTransformMediator::SetStyleSheetContentModel(nsTransformMediator * const
0x0414fe40, nsIDOMNode * 0x0407e1f4) line 133
nsXSLContentSink::DidBuildModel(nsXSLContentSink * const 0x0407e7a8, int 0) line 137
nsExpatDriver::DidBuildModel(nsExpatDriver * const 0x03663078, unsigned int 0,
int 1, nsIParser * 0x0407dfe0, nsIContentSink * 0x0407e7a8) line 847 + 23 bytes
nsParser::DidBuildModel(unsigned int 0) line 1388 + 41 bytes
nsParser::ResumeParse(int 1, int 1, int 1) line 1932
nsParser::OnStopRequest(nsParser * const 0x0407dfe4, nsIRequest * 0x041b3098,
nsISupports * 0x00000000, unsigned int 0) line 2542 + 21 bytes
nsFileChannel::OnStopRequest(nsFileChannel * const 0x041b30a0, nsIRequest *
0x041b31ec, nsISupports * 0x00000000, unsigned int 0) line 481 + 41 bytes
nsOnStopRequestEvent::HandleEvent() line 213
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x041b2ffc) line 116
PL_HandleEvent(PLEvent * 0x041b2ffc) line 590 + 10 bytes

I also noticed that the server gives a wrong mime type for .xsl files and opened
an evangelism bug 129096 for it.
Assignee: heikki → kvisco
Component: XML → XSLT
Ever confirmed: true
Keywords: nsbeta1+
QA Contact: petersen → kvisco
Should have been assigned to peterv..
Assignee: kvisco → peterv

Comment 4

16 years ago
Are you sure? my interpretation of the lines assuming bonsai is current

427 mXSLTransformMediator->GetResultDocument(getter_AddRefs(resultDOMDoc));
// should we also early return here?
428 nsCOMPtr<nsIDocument> resultDoc = do_QueryInterface(resultDOMDoc);
// we should early return here if resultDoc is null
430 nsCOMPtr<nsIDocument> sourceDoc = mDocument;
431 NS_RELEASE(mDocument);
433 mDocument = resultDoc;
434 NS_ADDREF(mDocument);
435 nsCOMPtr<nsIContent> root;
436 mDocument->GetRootContent(getter_AddRefs(root));
// we probably crash here, mDocument == 0
Summary: Invalidate xml crashes mozilla after reload → Invalidate xml crashes mozilla after reload [@nsXMLContentSink::Observe]
Yes, I am sure. I have changes in my tree which is why the line numbers are

Comment 6

16 years ago
timeless: the result document can't be null if this code gets called, the caller
needs it too. If it is null here, we're in serious trouble.

Comment 7

16 years ago
I build a debug version of Mozilla form nightly sources and I also found out 
that the crash occurs because mXSLTransformMediator is null in the valid file.

Heikki, you were absolutely right - I tested it with the local files and then 
copy them to the server. When I tested it afterwards, the mozilla didn't even 
show those files from the server* while IE6.0 did.

* nsXMLContentSink::Observe line 425 if (NS_SUCCEEDED(rv)) drops the execution 
to the else-path and nothing will happend.

When adding line 427:
if (!mXSLTransformMediator) return rv;
the crash didn't happend, but there might appear some other problems.

Comment 8

16 years ago
Created attachment 72781 [details] [diff] [review]
don't transform the error doc

I'm not sure this is the complete fix, or just part of wallpaper.
Nevertheless, this patch makes us display the error doc, and I guess that's
part of the game. I'm not sure if setting mXSLTransformMediator to 0 is enough,

do we have to talk to the documentViewer here, too?

Anyway, this patch has two good points, we display the parse error, and we
Comment on attachment 72781 [details] [diff] [review]
don't transform the error doc


One style nit: please replace |0| with |nsnull|.
Attachment #72781 - Flags: review+

Comment 10

16 years ago
Comment on attachment 72781 [details] [diff] [review]
don't transform the error doc

talked with peterv, missing points (at least):
unsetting the mediator in the docviewer
stopping the load of the stylesheet
Attachment #72781 - Flags: needs-work+
Yeah, I realized I should have checked what was set up prior to this point after
I was going home :(

Anyway this is important. Moving to peterv's 1.0 list but if someone else is
working on this you'd better take ownership.
Priority: -- → P2
Target Milestone: --- → mozilla1.0
btw, if the xslt doc contains a parse-error we currently show that error-page 
(which IMHO is great). Though this could be because the parse-error is 
interpreted as an LRE stylesheet.


16 years ago
OS: Windows NT → All
Hardware: PC → All
Summary: Invalidate xml crashes mozilla after reload [@nsXMLContentSink::Observe] → Invalid xml crashes mozilla after reload [@nsXMLContentSink::Observe]

Comment 13

16 years ago
Created attachment 78323 [details]
Valid stylesheet
Attachment #72781 - Attachment is obsolete: true

Comment 14

16 years ago
Created attachment 78326 [details]
Invalid stylesheet

Comment 15

16 years ago
Created attachment 78329 [details]
Valid source - Links to valid stylesheet

Comment 16

16 years ago
Created attachment 78330 [details]
Invalid source - Links to valid stylesheet

Comment 17

16 years ago
Created attachment 78331 [details]
Valid source - Links to invalid stylesheet

Comment 18

16 years ago
Created attachment 78332 [details]
Invalid source - Links to invalid stylesheet

Comment 19

16 years ago
Created attachment 78556 [details] [diff] [review]

Should fix the crash, but doesn't stop the stylesheet load for invalid source.
Still looking how to do that.
Does it really matter much if we don't stop loading of the stylesheet? I mean,
yeah, it is a bug but the user probably would never even know...? If this is the
case I would suggest filing a new bug for that and fixing this bug with the
patch you have now.
I agree as long as we don't style the errorous xml or the error-page (or at 
least don't show the result).

Comment 22

16 years ago
applied the patch, and valid source, invalid stylesheet doesn't display anything
for me.
The others are fine though.

Comment 23

16 years ago
Created attachment 79734 [details] [diff] [review]

This works for the four testcases (apart from some history bug which prevents
going back after a transform, I'll verify in the trunk if this patch causes it
but I doubt it). If the stylesheet is invalid, we wait until the source is
done: if the source is invalid too we show the error page for the source, if
the source is valid we copy the error message from the stylesheet to the result
document and notify the observer as if we did a transform which will show the
error message we just copied.
Attachment #78556 - Attachment is obsolete: true

Comment 24

16 years ago
Comment on attachment 79734 [details] [diff] [review]

>@@ -465,33 +463,41 @@
>       // Start the layout process
>       StartLayout();
>+      ScrollToRef();

Shouldn't this scroll only for non-history, too? If so, #if 0 like below.
Is there a bug number for this?

>Index: xsl/document/src/nsTransformMediator.cpp
>+      if (originalRoot) {
>+        mResultDoc->ReplaceChild(originalRoot, newRoot, getter_AddRefs(root));

ReplaceChild returns the old node, not the new one. Which you want, right?

Comment 25

16 years ago
Created attachment 80484 [details] [diff] [review]

Looking for reviews.
Attachment #79734 - Attachment is obsolete: true

Comment 26

16 years ago
Created attachment 80702 [details] [diff] [review]
Attachment #80484 - Attachment is obsolete: true

Comment 27

16 years ago
Created attachment 80763 [details] [diff] [review]
Attachment #80702 - Attachment is obsolete: true

Comment 29

16 years ago
Created attachment 80831 [details] [diff] [review]

Address jst's "but it can be faster". Only QI to nsIStyleLinkingElement for
html:style and html:link.
But doesn't xml-stylesheet PI also implement nsIStyleSheetLinkingElement, this
code would break PIs, no?
Comment on attachment 80831 [details] [diff] [review]

The code that checks the tag name is in element specific code, so no, it won't
affect PI's.

Attachment #80831 - Flags: superreview+

Comment 32

16 years ago
Checked in on the trunk
Last Resolved: 16 years ago
Keywords: adt1.0.0
Resolution: --- → FIXED
Whiteboard: [ADT2] → [ADT2][fixed on trunk]

Comment 33

16 years ago
verified on the trunk

Comment 34

16 years ago
Sent mail to drivers for checkin to the branch.

Comment 35

16 years ago
Chris - Can you take a look at this one, and make sure it resolves the bug
without any regressions?
Keywords: approval

Comment 36

16 years ago
I checked on this on the OS X trunk build (2002-04-25-08). None of the url's
produce a crash for me. I cc'd Rakesh M to check on Windows and Linux trunk
builds. He will add his comments when finished.

Comment 37

16 years ago
Checked on this with the attachments "Invalid stylesheet" and "Valid stylesheet"
on the trunk build 2002-04-24-12-trunk on  Windows 2000 and Linux trunk build
2002-04-25-10-trunk. None of them produce a crash.

Comment 38

16 years ago
Comment on attachment 80831 [details] [diff] [review]

a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #80831 - Flags: approval+
Comment on attachment 80831 [details] [diff] [review]

>Index: mozilla/content/base/src/nsDocumentViewer.cpp
>Index: mozilla/content/xml/document/src/nsXMLContentSink.cpp
>+#if 0 /* Disable until this works for XML */
>+      //  Scroll to Anchor only if the document was *not* loaded through history means. 
>+      PRUint32 documentLoadType = 0;
>+      docShell->GetLoadType(&documentLoadType);
>+      if (!(documentLoadType & nsIDocShell::LOAD_CMD_HISTORY)) {
>+        ScrollToRef();
>+      }
>+      ScrollToRef();

Maybe your source tree is not up to date, I thought these were already checked
Or was this XSLT specific that I did not do earlier?

>Index: mozilla/content/xml/document/src/nsXMLContentSink.h
>Index: mozilla/content/xsl/document/src/nsITransformMediator.h
>Index: mozilla/content/xsl/document/src/nsTransformMediator.cpp
>Index: mozilla/content/xsl/document/src/nsTransformMediator.h
>   PRBool mEnabled;
>+  PRBool mStyleInvalid;

You could save a little space by changing these to PRPackedBool.

>Index: mozilla/content/xsl/document/src/nsXSLContentSink.cpp
>+  rv = AddContentAsLeaf(node);
>+  return rv;

You could just 

return AddContentAsLeaf(node);

>Index: mozilla/content/xsl/document/src/nsXSLContentSink.h

Up to you if you want to make these changes/attach new patch.
Created attachment 81239 [details]
testcase: CSS style tests

This is meant for regression testing. This testcase should still work after
this patch (testing implied style, XHTML inline style element, XHTML style
attribute, XHTML link stylesheet, xml-stylesheet PI).
Created attachment 81241 [details]
testcase: correct URLs to stylesheets
Attachment #81239 - Attachment is obsolete: true

Comment 44

16 years ago
Testcase in comment #43 works correctly. Verfied with Trunk Build ID 2002043010.

Comment 45

16 years ago
Let's get this one in after RC2. adt1.0.0- [adt2 RTM]
Keywords: adt1.0.0 → adt1.0.0-
Whiteboard: [ADT2][fixed on trunk] → [ADT2 RTM][fixed on trunk] [ETA 05/15]

Comment 46

16 years ago
adding adt1.0.0+ for checkin to the 1.0 branch.  Please get drivers approval
again since it's been more than 3 days since the last approval and afterwards
check into the 1.0 branch.
Keywords: adt1.0.0- → adt1.0.0

Comment 47

16 years ago
Comment on attachment 80831 [details] [diff] [review]

please check in to 1.0 branch asap to get this in rc3.

Comment 48

16 years ago
adding the + to match my previous comment.
Keywords: adt1.0.0 → adt1.0.0+
Fix checked in on the mozilla1.0 branch.
Keywords: fixed1.0.0


16 years ago
Whiteboard: [ADT2 RTM][fixed on trunk] [ETA 05/15] → [ADT2 RTM][fixed on trunk] [ETA 05/15],custrtm-

Comment 50

16 years ago
*** Bug 155588 has been marked as a duplicate of this bug. ***

Comment 51

16 years ago
*** Bug 157408 has been marked as a duplicate of this bug. ***
Crash Signature: [@nsXMLContentSink::Observe]
You need to log in before you can comment on or make changes to this bug.