Bugzilla@Mozilla – Bug 464009
Crash [@ nsGenericHTMLFormElement::UnbindFromTree] with DOMAttrModified removing file input in form
Last modified: 2009-03-11 10:11:44 PDT
Summon comment box
Created attachment 347272 [details] testcase See testcase, which crashes current trunk build within 100ms. It also crashes Firefox 3, so marking security sensitive for now. http://crash-stats.mozilla.com/report/index/eb42a7b8-af24-11dd-8d45-001a4bd43ed6?p=1 0 xul.dll xul.dll@0x2e8342 1 xul.dll nsGenericHTMLFormElement::UnbindFromTree content/html/content/src/nsGenericHTMLElement.cpp:2513 2 xul.dll nsHTMLInputElement::UnbindFromTree content/html/content/src/nsHTMLInputElement.cpp:1917 3 xul.dll nsGenericElement::doRemoveChildAt content/base/src/nsGenericElement.cpp:3366 4 xul.dll nsGenericElement::RemoveChildAt content/base/src/nsGenericElement.cpp:3293 5 xul.dll nsGenericElement::doRemoveChild content/base/src/nsGenericElement.cpp:3955 6 xul.dll nsGenericElement::RemoveChild content/base/src/nsGenericElement.cpp:3521 7 xul.dll nsIDOMNode_RemoveChild obj-firefox/js/src/xpconnect/src/dom_quickstubs.cpp:2757 8 js3250.dll js_Interpret js/src/jsinterp.cpp:4998
I get also the following assertions ###!!! ASSERTION: Child not in controls: 'index != controls.NoIndex', file /Users/smaug/mozilla/hg/mozilla/content/html/content/src/nsHTMLFormElement.cpp, line 1478 ###!!! ASSERTION: Invalid start index: 'count == 0 || start < Length()', file ../../../../dist/include/xpcom/nsTArray.h, line 597 ###!!! ASSERTION: QueryInterface needed: 'query_result.get() == mRawPtr', file ../../../dist/include/xpcom/nsCOMPtr.h, line 521 (But have to wait a bit before I can really debug this. 64bit Linux builds don't really work atm :( )
I thought I assigned this to myself already :/
Created attachment 347618 [details] [review] NS_ASSERTION to NS_ENSURE_STATE This is enough to fix the crash.
Created attachment 347619 [details] [review] but this is what I prefer Makes mutation events to fire after AfterSetAttr, fixing the crash. This way things stay more consistent while changing attributes. The first patch might be enough for 1.9.0 but I'd like to take these both for 1.9.1.
Comment on attachment 347619 [details] [review] but this is what I prefer Yeah, i like this one better too
Comment on attachment 347618 [details] [review] NS_ASSERTION to NS_ENSURE_STATE ok for branch
Comment on attachment 347618 [details] [review] NS_ASSERTION to NS_ENSURE_STATE a191=beltzner
Comment on attachment 347619 [details] [review] but this is what I prefer a191=beltzner
Blocking, but this has approval etc so it's ready to go either way.
Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081204 Minefield/3.2a1pre
Fixed on trunk and 1.9.1
Comment on attachment 347618 [details] [review] NS_ASSERTION to NS_ENSURE_STATE Approved for 1.9.0.6 , a=dveditz for release-drivers.
Verified fix for 1.9.0.6 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2009011304 GranParadiso/3.0.6pre. It no longe crashes (but still crashes 3.0.5).
This was crashing with a consistent null deref, but the nsTArray assertion was a little worrying so I can't say it was safe, either.
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3