370586 - [FIX]Crash [@ nsCellMapColumnIterator::GetNextFrame] with <td style="display: table-footer-group"> and rowspan

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Loading the testcase makes Firefox crash [@ nsCellMapColumnIterator::GetNextFrame].

Comment 1

[Jesse Ruderman]

In a debug build, I see this assertion before the crash:

###!!! ASSERTION: Bogus mOrigCells?: 'mCurMapRow < mCurMapRelevantRowCount', file /Users/jruderman/trunk/mozilla/layout/tables/nsCellMap.cpp, line 2792

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, nice!

So the issue is that we have a pseudo cellframe for that <td>.  And that pseudo cellframe claims that its rowspan is "3" if you ask it.  But when the rowspan is changed, the cellmap is NOT updated, since the cellframe is not the primary frame for the <td>.  As a result, we think that it has a span of 3, but it only has a span of 1 in the cellmap, hence crashes.

I don't think the rowspan of 3 should be happening here, frankly.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Non-crashing testcase showing bug -- the two tables should be identical

Jesse, that assertion is pretty much the only way crashes can happen here; I should probably give it more generic text since it seems to catch all sorts of cellmap issue.  :(

Comment 4

[Bernd]

>that assertion is pretty much the only way crashes can happen here

Yeah somebody has to lead the crowd. Before this BasicTableLayoutStrategy::AssignNonPctColumnWidths had this role. This is just one place where we access all cellmap coordinates and verify by this the correctness of the cellmap.


>So the issue is that we have a pseudo cellframe for that <td>.  And that pseudo
>cellframe claims that its rowspan is "3" if you ask it.

Thats basically a style issue?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 6

[Bernd]

Comment on attachment 255307 [details] [diff] [review]
Proposed fix

looks good

Comment 7

[Bernd]

fix checked in on 2007-02-18 17:10

Comment 8

[Boris Zbarsky [:bzbarsky]]

Oh, sorry.  I meant to mark this resolved...