Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090428 Minefield/3.6a1pre

You could trigger this with any node, not just <style>, if you set textContent instead of innerHTML. The problem is this loop in SetNodeTextContent: for (PRUint32 i = 0; i < childCount; ++i) { nsIContent* child = aContent->GetChildAt(removeIndex); if (removeIndex == 0 && child->IsNodeOfType(nsINode::eTEXT)) { nsresult rv = child->SetText(aValue, PR_TRUE); NS_ENSURE_SUCCESS(rv, rv); removeIndex = 1; } else { aContent->RemoveChildAt(removeIndex, PR_TRUE); } } The RemoveChildAt call triggers the mutation handler, which changes the number of kids, so we end up with a null |child| on a later call. You could probably accomplish this by using a mutation listener for the text content of the first textnode kid as well. I suspect that just null-checking |child| is the way to go here. Jonas?

(In reply to comment #2) > You could trigger this with any node, not just <style>, if you set textContent > instead of innerHTML. No, because setting textContent doesn't ever reuse inner text node. (That is per DOM 3 spec, and according to HTML5 innerHTML should behave the same way.)

Oh, right. Well, you could do option.text, title.text, script.text, textarea.defaultValue... In any case, the problem is that SetNodeTextContent is very much non-atomic.

Taking bug.

Comment on attachment 377572 [details] [diff] [review] patch fixes crash Looks good! :)

Please add the testcase as a crashtest when checking this in.

It seems to me that SetNodeTextContent should be atomic with respect to mutation events, but I checked in the patch anyway. http://hg.mozilla.org/mozilla-central/rev/cac77c7bacc8 But I forgot to check in the test. Sorry.

> It seems to me that SetNodeTextContent should be atomic One could wish, but there's no way to make that work while following the DOM spec... Basically, mutation events are severely broken by design. The correct way to make them work would be to lock the DOM while firing the event or something. Except the spec didn't do that.

Pushed test as http://hg.mozilla.org/mozilla-central/rev/ce5e54260f1d and http://hg.mozilla.org/mozilla-central/rev/c5c18dde8bb9

Comment on attachment 377572 [details] [diff] [review] patch fixes crash a191=beltzner

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/41bfc30af3aa http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4079e1bbfbdc

Verified fixed on trunk and 1.9.1 with builds on Windows and OS X: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090526 Minefield/3.6a1pre ID:20090526031623 Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090526 Shiretoko/3.5pre ID:20090526031155

Comment on attachment 377572 [details] [diff] [review] patch fixes crash Approved for 1.9.0.15, a=dveditz for release-drivers

Checking in nsContentUtils.cpp; /cvsroot/mozilla/content/base/src/nsContentUtils.cpp,v <-- nsContentUtils.cpp new revision: 1.314; previous revision: 1.313 done

Verified for 1.9.0 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.0.15pre) Gecko/2009092404 GranParadiso/3.0.15pre after verifying crash in 1.9.0.14 using attached testcase.