Closed Bug 431703 Opened 12 years ago Closed 12 years ago

Crash [@ nsXMLContentSink::HandleEndElement] setting innerHTML with SVG, onload

Categories

(Core :: SVG, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: jruderman, Assigned: longsonr)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(2 files)

Loading the testcase makes Firefox crash with a null mDocument at nsXMLContentSink::HandleEndElement:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xml/document/src/nsXMLContentSink.cpp&rev=1.442&mark=1172#1157

Based on code inspection, I think this is a regression from bug 380417.
Doubt it.  The code dereferenced mDocument before as well, and nsXMLFragmentContentSink simply never calls Init() and hence never sets mDocument as far as I can tell.  So any use of mDocument in the XML sink that doesn't null-check it would crash in innerHTML sets if it gets hit.

This code just needs a null-check, and has all along.
You're right.  Sorry for blaming you incorrectly.
Keywords: regression
Attached patch null checkSplinter Review
Assignee: nobody → longsonr
Status: NEW → ASSIGNED
Attachment #318988 - Flags: superreview?(bzbarsky)
Attachment #318988 - Flags: review?(bzbarsky)
OS: Mac OS X → All
Hardware: PC → All
Comment on attachment 318988 [details] [diff] [review]
null check

Makes sense.  I doubt anyone expects this event to fire inside innerHTML sets.
Attachment #318988 - Flags: superreview?(bzbarsky)
Attachment #318988 - Flags: superreview+
Attachment #318988 - Flags: review?(bzbarsky)
Attachment #318988 - Flags: review+
Comment on attachment 318988 [details] [diff] [review]
null check

Simple null check to prevent null dereference crash.
Attachment #318988 - Flags: approval1.9?
Whiteboard: [needs approval][needs landing]
Comment on attachment 318988 [details] [diff] [review]
null check

a1.9=beltzner
Attachment #318988 - Flags: approval1.9? → approval1.9+
Whiteboard: [needs approval][needs landing] → checkin-wanted
It's "checkin-needed", and it's a keyword, not a status whiteboard note. :)
Keywords: checkin-needed
Whiteboard: checkin-wanted
Checking in content/xml/document/src/nsXMLContentSink.cpp;
/cvsroot/mozilla/content/xml/document/src/nsXMLContentSink.cpp,v  <--  nsXMLContentSink.cpp
new revision: 1.443; previous revision: 1.442
done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Can someone crashtestify this?
Flags: in-testsuite?
Added crashtest:
http://hg.mozilla.org/mozilla-central/rev/63120a5301f2
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsXMLContentSink::HandleEndElement]
You need to log in before you can comment on or make changes to this bug.