Closed Bug 468562 Opened 11 years ago Closed 11 years ago

"ASSERTION: Inserting multiple children without flushing"

Categories

(Core :: DOM: HTML Parser, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1
Tracking Status
blocking1.9.1 --- .5+
status1.9.1 --- .5-fixed

People

(Reporter: jruderman, Assigned: mozilla+ben)

References

()

Details

(Keywords: assertion, testcase, verified1.9.1, Whiteboard: [needed on 1.9.1 to fix 525276])

Attachments

(3 files)

Attached file testcase 1
Loading the testcase triggers:

###!!! ASSERTION: Inserting multiple children without flushing.: 'mNumFlushed == mContent->GetChildCount()', file /Users/jruderman/central/content/html/document/src/nsHTMLContentSink.cpp, line 903

Reduced from http://nmuta.fri.macserver.jp/home149.html.
Attached file testcase 2
Adding a "</table>" makes it give me this assertion instead:

###!!! ASSERTION: Node at base of context stack not fully flushed.: 'bottom.mContent->GetChildCount() == bottom.mNumFlushed', file /Users/jruderman/central/content/html/document/src/nsHTMLContentSink.cpp, line 1933
Assignee: nobody → bnewman
This patch
1. reuses AddLeaf, unifying some code
2. keeps the assertions from failing by calling DidAddContent before it's too late (in AddLeaf)
3. no longer forces <link>s to be appended (they may be inserted at other positions now, too)
Attachment #352628 - Flags: review?(mrbkap)
Status: NEW → ASSIGNED
Attachment #352628 - Flags: superreview+
Attachment #352628 - Flags: review?(mrbkap)
Attachment #352628 - Flags: review+
Comment on attachment 352628 [details] [diff] [review]
using AddLeaf instead of AppendChildTo to add <link>s in ProcessLINKTag
[Checkin: Comment 4]

Looks good. r+sr=mrbkap
Keywords: checkin-needed
Comment on attachment 352628 [details] [diff] [review]
using AddLeaf instead of AppendChildTo to add <link>s in ProcessLINKTag
[Checkin: Comment 4]

http://hg.mozilla.org/mozilla-central/rev/4b94808ccb23
Attachment #352628 - Attachment description: using AddLeaf instead of AppendChildTo to add <link>s in ProcessLINKTag → using AddLeaf instead of AppendChildTo to add <link>s in ProcessLINKTag [Checkin: Comment 4]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Flags: in-testsuite+
Attachment #352628 - Flags: approval1.9.1.5?
blocking1.9.1: --- → .5+
Whiteboard: [needed on 1.9.1 to fix 525276]
Attachment #352628 - Flags: approval1.9.1.5? → approval1.9.1.5+
Comment on attachment 352628 [details] [diff] [review]
using AddLeaf instead of AppendChildTo to add <link>s in ProcessLINKTag
[Checkin: Comment 4]

Approved for 1.9.1.5, a=dveditz for release-drivers
Can we get this landed on 1.9.1 ASAP?
Jesse, this bug should be all/all, right?
Attachment #352628 - Flags: approval1.9.1.5+
This needs to land on the GECKO1914_20091006_RELBRANCH for 1.9.1.5.
blocking1.9.1: .6+ → .5+
Verified on 1.9.1 on OS X 10.6 with my own debug 1.9.1 build.
Keywords: verified1.9.1
Perhaps because it appears in release notes, the summary line of this bug 
seems to be drawing attention in some user circles.  Users are envisioning 
places where (a) flushing occurs, and (b) children are inserted   
(and browsers make assertions about these occurrences).

Perhaps we should take slightly more care in wording assertion messages.
As appalling as it may sound, Firefox flushes (passes off for rendering) trillions, literally trillions, of children (HTML elements) every day. I believe our users are stronger for knowing the truth. And if even just one volunteer joins the project in order to satisfy her morbid curiosity about the watery plight of all those page elements, we are, all of us, that much better off.
Soon we will make wikipedia.
"Perhaps we should take slightly more care in wording assertion messages."
I think the real fault is the terminology used.
You need to log in before you can comment on or make changes to this bug.