385533 - Assertion failures from a 2-cell table, with "padding: 50%" on each cell

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase

Branching part of Bug 385132, using my simpler testcase that only triggers 2 of the 4 assertions in that bug.

Assertion failures are:

###!!! ASSERTION: assigned width smaller than min: 'col_width >=
colFrame->GetMinCoord()', file
/Users/jruderman/trunk/mozilla/layout/tables/BasicTableLayoutStrategy.cpp, line
960

###!!! ASSERTION: didn't subtract all that we added: 'space == 0 && ((l2t ==
FLEX_PCT_LARGE) ? (-0.001f < basis.f && basis.f < 0.001f) : (basis.c == 0))',
file /Users/jruderman/trunk/mozilla/layout/tables/BasicTableLayoutStrategy.cpp,
line 972


Note that any padding value >= 50% triggers the assertions.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: patch (handle eStyleUnit_Auto/nscoord_MAX in GetWidthInfo)

This patch seems to fix the problem.  Not sure if it's the right way to go, but it's one solution.

The bug boils down to this:
  1. nsLayoutUtils.cpp:AddPercents() gets called, with aPercent == 2 * the padding value (because there's padding on both sides).  For padding > 50%, this triggers it to fall into the case return nscoord_MAX. (a really big number)
  2. This value gets bubbled up to prefCoord at the beginning of  BasicTableLayoutStrategy.cpp:GetWidthInfo(). 
  3. For auto-height, this "bad" prefCoord value is not overridden in this function, and it gets used in constructing the CellWidthInfo on the last line.  Going up one level, this gets stored as the mPrefCoord for some column frame.
 *** Fast forward a bit...
  4. In BasicTableLayoutStrategy::ComputeColumnWidths, at around line 774, we use the value of colFrame->GetPrefCoord().  In this case, that value is nscoord_MAX, and it seems to be used without being sanity checked.  Consequently, we end up getting computations that are way too big and overflow.

My patch intercepts the problem at step #3 above, before we store nscoord_MAX as mPrefCoord for a column frame.

Note: This patch also fixes both testcases on Bug 385132.

Comment 2

[Daniel Holbert [:dholbert]]

Here are references on mxr, for the code I mentioned above:
  1. AddPercents() is at:
http://mxr.mozilla.org/seamonkey/source/layout/base/nsLayoutUtils.cpp#1222
  2. GetWidthInfo() is at:
http://mxr.mozilla.org/seamonkey/source/layout/tables/BasicTableLayoutStrategy.cpp#105
The value of prefCoord is set at line 113.
  3. prefCoord gets used to construct a CellWidthInfo at
http://mxr.mozilla.org/seamonkey/source/layout/tables/BasicTableLayoutStrategy.cpp#223
  4. prefCoord gets used in ComputeColumnWidths at
http://mxr.mozilla.org/seamonkey/source/layout/tables/BasicTableLayoutStrategy.cpp#768
 (lines 768-769)

Comment 3

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

Comment on attachment 269715 [details] [diff] [review]
patch (handle eStyleUnit_Auto/nscoord_MAX in GetWidthInfo)

The idea of returning nscoord_MAX was that things should behave as though the pref width of that cell, and thus that column, were infinitely large, so we shouldn't try to prevent it from reaching the columns.

Comment 4

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

See also bug 371483.

Comment 5

[Daniel Holbert [:dholbert]]

I posted an nscoord_MAX-checking patch on bug 367673, and it covers this bug as
well.

Comment 6

[Daniel Holbert [:dholbert]]

Fixed by checkin for bug 367673.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: reftest for assertion