364512 - ASSERTION: unexpected frame type: 'IS_TABLE_CELL(frameType) || nsLayoutAtoms::tableRowFrame == frameType || nsLayoutAtoms::tableRowGroupFrame == frameType || nsLayoutAtoms::tableFrame == frameType'

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Jesse Ruderman]

Loading this testcase triggers:

###!!! ASSERTION: unexpected frame type: 'IS_TABLE_CELL(frameType) || nsLayoutAtoms::tableRowFrame == frameType || nsLayoutAtoms::tableRowGroupFrame == frameType || nsLayoutAtoms::tableFrame == frameType', file /Users/admin/trunk/mozilla/layout/tables/nsTableFrame.cpp, line 1798

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Bernd]

yep the assertion 
1794     NS_ASSERTION(IS_TABLE_CELL(frameType) ||
1795                  nsLayoutAtoms::tableRowFrame == frameType ||
1796                  nsLayoutAtoms::tableRowGroupFrame == frameType ||
1797                  nsLayoutAtoms::tableFrame == frameType,
1798                  "unexpected frame type");
is not complete, as rowgroups can be wrapped in a scroll frame

Comment 3

[Bernd]

Attachment: patch

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This assertion is documenting an assumption that I was making when I wrote the code -- will the special height reflow actually propagate through the scrollframe correctly, or is additional code needed for that?

I really should have rewritten the comment above RequestSpecialHeightReflow to reflect what I was thinking.  I'll need to have a closer look tomorrow and try to remember.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 249334 [details] [diff] [review]
patch

I'

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 249334 [details] [diff] [review]
patch

I'm trying to think of what will be broken in this case.

So far, I think that:
 * if the table is IsGeometryDirty, we'll need to mark the row group (and scroll frame containing it?) as NS_FRAME_HAS_DIRTY_CHILDREN to make the reflow propagate down to it.

We may also need to propagate the mHResize flag the way nsTableRowGroupFrame::ReflowChildren does, although maybe we should replace that propagation with SetGeometryDirty.

But this patch looks fine.  I can't think of anything else broken in this case, although there probably is something since I'd forgotten about it.

I suppose I should make sure there are bugs on file for the above problems...

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I filed bug 386401.

Comment 8

[Bernd]

Comment on attachment 249334 [details] [diff] [review]
patch

its a safe change, it adjusts the assertion to the code reality.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 249334 [details] [diff] [review]
patch

a1.9=dbaron

Comment 10

[Bernd]

fix checked in

Comment 11

[Jesse Ruderman]

Crashtest checked in.