147225 - innerHTML in xml document crashes mozilla

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Leo Savernik]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc3) Gecko/20020523
BuildID:    2002052309

When assigning any value to document.innerHTML in an XML document the browser
immediately crashes.

Reproducible: Always
Steps to Reproduce:
1. Load the attached testcase
2. Click on "Click me"


Actual Results:  Mozilla crashes with SIGSEGV

Expected Results:  The "Click me" should be replaced by "Changed!" in double
size surrounded by a blue dotted border

Interestingly, this bug does not occur in Mozilla 0.8.1 and Mozilla 0.9.

Here is a talkback ID from the last crash under Mozilla 1.0 RC3: TB6690739Q

A testcase that reproducably causes a crash for me is attached below.

Comment 1

[Leo Savernik]

Attachment: xml file whose innerhtml invocation causes a crash

This testcases reliably crashes Mozilla 1.0 RC3.

Comment 2

[Leo Savernik]

Attachment: html file whose innerhtml invocation works as expected

This is an HTML file which does virtually the same innerHTML assignment as
attachment 85111 [details] but is HTML 4.01 strict instead of XHTML 1.0 strict.
Interestingly this testcase's innerHTML works as expected.

Comment 3

[Stefan [:stefanh]]

Doesn't crash for me with Mac OS 9.1 build 2002052305, but when i use the
testcase (xml) and click on "Click me", "Click me" disappears and I end up with
a blank page instead of the word "Changed". Get this message in JavaScript Console:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004003 (NS_ERROR_INVALID_POINTER) [nsIDOMNSHTMLElement.innerHTML]" 
nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame ::
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=85111 :: clickHandler
:: line 10"  data: no]

Using build 2002052305 and Mac OS 9.1

Comment 4

[Igor Furlan]

confirmed on RH6.2 with mozilla 1.0 RC3 running the test case.

Additional note: MSIE5.5 works OK
OPERA 6.0 DOES NOT crash  but it does not do anything either

Comment 5

[joakim_46]

Confirming it with build 2002052306 under Windows ME.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Igor, I have no idea how you even loaded the testcase in IE 5.5, much less 
tested how innerHTML works.  IE does not handle that MIME type.

Stephan, are you using a trunk or branch build?

David, are you using a trunk or branch build?  That build ID looks suspiciously 
like an RC3 date....

Can someone seeing this post a talkback ID?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Oh, and this is almost certainly a crash in range code...

Comment 8

[Stefan [:stefanh]]

Boris: Using the 1.0 RC3 branch build, downloaded from
ftp://ftp.mozilla.org/pub/mozilla/releases/mozilla1.0rc3/mozilla-mac-100rc3-full.bin

Comment 9

[Leo Savernik]

Boris: I *did* provide a talkback ID in my original report. It's TB6690739Q
And I've got another one when testing my xml testcase: TB6691761Y

If I have the chance to test Mozilla under W2k tomorrow, I'll post another
talkback ID.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Leo, my apologies.

stephend, could you get the stack?  TB6690739Q and TB6691761Y

Comment 11

[[e-mail] (gone - use [e-mail] instead)]

0x40a0b27a
0x409ec4ef
0x40a02740
0x40a009a4
0x40a0062c
0x40a008b2
0x40d2da3f
0x40b9b3ce
0x40ba669a
0x4018bee1
0x406908b9
0x406962b7
0x40098b42
0x40098d8c
0x400ac724
0x4009f93e
0x40098b90
0x40098d8c
0x4007aa9b
0x40af54ae
0x40b21929
0x40b85dfe
0x40b8660c
0x40d1f508
0x40d198ee
0x41051407
0x41051300
0x40b8eae8
0x40b8d1fb
0x410514d8
0x4105128c
0x411eb0b1
0x411e0e39
0x411ea748
0x411e096d
0x408daeba
0x408dadb5
0x408daf37
0x408db904
0x408deab1
0x408ded2c
0x408d5f47
0x408d5d99
0x403719e4
0x403a2be6
0x403a3213
0x403a33dc
0x402bf04c
0x408cf53c
0x408ae7c6
0x08050fe9
0x080517d7
0x404c7baf 

Comment 12

[Boris Zbarsky [:bzbarsky]]

That's not a very useful stack... I just sent in another talkback -- ID
TB6727242Y (the email on that one is [e-mail]).  Stephend, could you
see whether that stack includes something like function names?

As a note, this worksforme in a trunk build but crashes the branch.

Comment 13

[[e-mail] (gone - use [e-mail] instead)]

XML_UnblockParser [d:\builds\seamonkey\mozilla\expat\xmlparse\xmlparse.c, line 754]
nsExpatDriver::ConsumeToken
[d:\builds\seamonkey\mozilla\htmlparser\src\nsExpatDriver.cpp, line 822]
nsParser::Tokenize [d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp,
line 2507]
nsParser::ResumeParse [d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp,
line 1731]
nsParser::Parse [d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 1635]
nsParser::ParseFragment
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 1668]
nsRange::CreateContextualFragment
[d:\builds\seamonkey\mozilla\content\base\src\nsRange.cpp, line 2820]
nsGenericHTMLElement::SetInnerHTML
[d:\builds\seamonkey\mozilla\content\html\content\src\nsGenericHTMLElement.cpp,
line 931]
nsGenericHTMLElementTearoff::SetInnerHTML
[d:\builds\seamonkey\mozilla\content\html\content\src\nsGenericHTMLElement.cpp,
line 209]
XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp,
line 106]
XPCWrappedNative::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp, line 2028]
XPC_WN_GetterSetter
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp,
line 1291]
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 790]
js_InternalInvoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 881]
js_SetProperty [d:\builds\seamonkey\mozilla\js\src\jsobj.c, line 2614]
js_Interpret [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2586]
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 806]
js_InternalInvoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 881]
JS_CallFunctionValue [d:\builds\seamonkey\mozilla\js\src\jsapi.c, line 3426]
nsJSContext::CallEventHandler
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp, line 1019]
nsJSEventListener::HandleEvent
[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp, line 182]
nsEventListenerManager::HandleEventSubType
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp, line
1220]
nsEventListenerManager::HandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp, line
1397]
nsGenericElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericElement.cpp, line 1665]
nsGenericDOMDataNode::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericDOMDataNode.cpp, line 898]
PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6105]
PresShell::HandleEventWithTarget
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6073]
nsEventStateManager::CheckForAndDispatchClick
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp, line 2640]
nsEventStateManager::PostHandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp, line 1721]
PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6126]
PresShell::HandleEvent
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6028]
nsViewManager::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 2076]
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 306]
nsViewManager::DispatchEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 1887]
HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 83]
nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 891]
nsWindow::DispatchWindowEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 908]
nsWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 4745]
ChildWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 5000]
0x01f30010
nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 1153]
KERNEL32.DLL + 0x363b (0xbff7363b)
KERNEL32.DLL + 0x24407 (0xbff94407)
0x00648bfa 

Comment 14

[Boris Zbarsky [:bzbarsky]]

Same stack as bug 133827

Comment 15

[Boris Zbarsky [:bzbarsky]]

CreateContextualFragment uses an nsIHTMLFragmentContentSink implementation
(nsHTMLFragmentContentSink) as the sink.  This sink does not implement
nsIExpatSink.  The document in question has an XML type, so it's passed to the
nsExpatDriver DTD to parse.  This DTD returns an error from WillBuildModel,
since the sink is not usable (mSink is null).

Now WillBuildModel is called by nsParser::ResumeParse.  On the trunk, the return
value is checked and an error is returned.  Hence the exception.  On the branch,
the return value is ignored and the parser goes on to tokenize the input stream
while the DTD has a null sink.  This crashes, probably because the mExpatParser
being passed around is null (WillBuildModel never got around to creating it). 
The checkin that fixed the crash on the trunk was for bug 138071.

Bug 133827 covers the fact that innerHTML does not work at all in non-text/html
content.  Attaching a small patch that should fix this crash on the branch.  I
have no way to test it, so testing would be greatly appreciated.  This is a very
small and safe crash fix and should be landable for 1.0.1 or maybe even 1.0 if
people move quickly...

Comment 16

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 17

[Johnny Stenback (:jst)]

Comment on attachment 85204 [details] [diff] [review]
Proposed patch

We should make sure this goes in for Netscape RTM.

sr=jst

Comment 18

[Leo Savernik]

When I saw that a patch had already been provided I didn't do any testing on
W2k, so no new talkback ID from me (mine seem to be useless anyway :-( ).

Btw, as far as I understand the provided patch it only fixes the crash but does
not fix the use of createContextualFragment() inside xml documents. This almost
certainly means that createContextualFragment() in xml won't work in Mozilla
1.0,  will it?

I hope there are plans to fix this for a later release because it did not only
work in earlier releases (around 0.8.1) but it is also the easiest way of
creating document fragments from parsed markup I can imagine.

Comment 19

[harishd]

Boris: I've to request again ( my first request was denied :-( ) for drivers
approval to land the fix in bug 138071.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Ok.  If that lands then we can ignore this bug.  If not, we can use this 
patch.  The fact that it fixes a crash should make that patch more palatable to 
drivers... ;)

Leo, this patch does indeed just fix the crash.  You're correct that 
createContextualFragment will not work in XML for 1.0.

I'll put more comments in bug 133827 describing what needs to be done to make 
it work.

Comment 21

[harishd]

Bug 138071 is fixed on the branch as well. Marking this bug FIXED.