394540 - Recurring into document.write using DOMCharacterDataModified event can hang Firefox

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P5)

Description

[Jesse Ruderman]

Attachment: testcase (hangs Firefox when loaded)

Loading the testcase triggers:

###!!! ASSERTION: Tag mismatch.  Closing tag on wrong context or something?: 'nodeType == eHTMLTag_form || nodeType == aTag', file /Users/jruderman/trunk/mozilla/content/html/document/src/nsHTMLContentSink.cpp, line 928

###!!! ASSERTION: Error: invalid tag stack position: 'mBodyContext->GetCount() > 0', file /Users/jruderman/trunk/mozilla/parser/htmlparser/src/CNavDTD.cpp, line 2719

Firefox hangs, repeatedly reporting the second assertion.

I'm not sure this is a parser bug.  The bug might be that the mutation event fires despite the page being in a "loading" state (see bug 90983).

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Might not be a real blocker since it's not exploitable. But would be great if you could have a look at this blake. We'll probably degrade this to [wanted-1.9] once we get closer to release.

Comment 2

[Jesse Ruderman]

This isn't the only bad thing that happens with the combination of mutation event handlers and document.write, fwiw.

Comment 3

[Johnny Stenback (:jst)]

Bug 394418 may have an impact on the behavior here.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Jesse, does this happen still now that bug 394418 is fixed?

Comment 5

[Jesse Ruderman]

I still get the two assertions in comment 0.  I don't know whether it still leads to a hang because I turned the second assertion into an abort locally.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Isn't the issue that we end up notifying sync from inside the content sink when we append text to an already-flushed and existing text node?  See also bug 235255.  Maybe we should just get that patch in?

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

suer since there is a patch we might as well land it

Comment 8

[Boris Zbarsky [:bzbarsky]]

Well.  That patch isn't quite done is the problem....  ;)  Otherwise it would _be_ in.

Comment 9

[Damon Sicore (:damons)]

Marking as wanted for a dot release per driver reprio triage w/ jst and sicking.

Comment 10

[Jesse Ruderman]

Still happens with html5.enable false, but not with html5.enable true.