489479 - [FIX]"ASSERTION: heterogenous childlist" and more with table crashtest 55789-1.html

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Jesse Ruderman]

layout/tables/crashtests/55789-1.html triggers:

###!!! ASSERTION: heterogenous childlist: '(display->mDisplay == NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP) == (nextDisplay->mDisplay == NS_STYLE_DISPLAY_TABLE_COLUMN_GROUP)', file /Users/jruderman/central/layout/tables/nsTableFrame.cpp, line 2346

###!!! ASSERTION: Non-row-group primary frame list child of an nsTableFrame?  How come?: 'Not Reached', file /Users/jruderman/central/layout/tables/nsTableFrame.cpp, line 2781

Comment 1

[Bernd]

this might be a fallout from http://hg.mozilla.org/mozilla-central/rev/11714b1d9aa4 from bug 487449

     113 -  return frameManager->AppendFrames(aParentFrame, nsnull,
     114 -                                    aFrameList.childList);
     115 +  // Insert the frames after out aPrevSibling
     116 +  return aState.mFrameManager->InsertFrames(aParentFrame, nsnull, aPrevSibling,
     117 +                                            aFrameList.childList);
     118  }


which changes the way frames enter the table, the append is able to handle heterogneous lists the insert is not.

Comment 2

[Bernd]

the principal solution would be to fix the frame construction so that it does handle the colgroups on a additional childlist, this out of my reach. But I am always open for the small hack, to detect that a insert is a masked append and to redirect the call to nsTableFrame::AppendFrame. I guess the insert instead of the append is warranted at the cssframeconstructor.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The reason we do insert instead of append in the frame constructor is that we already know the previous sibling (at least in all the common cases).

It would certainly make sense to handle colgroups the same way we handle captions, but that's a bit of work and I'd like to think about a good way to handle it, in general.

Since we rarely have long rowgroups/colgroup lists, just checking whether prevSibling has a null nextSibling and calling into AppendFrames probably makes the most sense.

Comment 4

[Boris Zbarsky [:bzbarsky]]

And in particular, this assumption:

  // Asssume there's only one frame being inserted. 

in nsTableFrame::InsertFrames is not so good.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Even better

Comment 7

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/6a5b3b7bd3f2