Empty table cells should be ignored in table layout calculations in same case. See the attached test case, compare with firefox.
Created attachment 13648 [details] test case Empty table cells should be ignored in percent width calculations. Specifying border makes FF consider them non-empty (though border is still not drawn). Specifying background does not have this effect.
<rdar://problem/5067926>
Created attachment 13929 [details] more complete test case
Created attachment 13933 [details] special handling for empty table cells This behavior matches Firefox exactly (as far as the test case goes at least) and is pretty close to IE7 too. The patch causes failures in result of tests listed below. I went through the failures and they all seem to be meaningless, not visible in rendering or progressions. For example pixel results of acid2 do not change even though the render tree dump does. New results for these would need to be checked in as well. Test fast/css/percentage-non-integer relies on old empty cell behavior and would need modifications to look nice again. fast/block/positioning/negative-right-pos fast/css/acid2-pixel fast/css/acid2 fast/css/percentage-non-integer fast/repaint/table-cell-move fast/table/012 fast/table/032 fast/table/empty-section-crash http/tests/misc/acid2-pixel http/tests/misc/acid2 tables/mozilla/bugs/bug1188 tables/mozilla/bugs/bug1818-6 tables/mozilla/bugs/bug16012 tables/mozilla/bugs/bug78162 tables/mozilla/bugs/bug100334 tables/mozilla/bugs/bug222336 tables/mozilla/bugs/bug222467 tables/mozilla/core/misc tables/mozilla_expected_failures/bugs/bug14007-1 tables/mozilla_expected_failures/bugs/bug72393 tables/mozilla_expected_failures/other/empty_cells
Created attachment 13934 [details] even more complete test case
Comment on attachment 13933 [details] special handling for empty table cells + if(!m_layoutStruct[lastCol].emptyCellsOnly) Needs a space after the if. case Static: - numAuto++; - totalAuto += m_layoutStruct[i].effMaxWidth; - allocAuto += w; + if (m_layoutStruct[i].emptyCellsOnly) + numAutoEmptyCellsOnly++; + else { + numAuto++; + totalAuto += m_layoutStruct[i].effMaxWidth; + allocAuto += w; + } default: break; It's pretty cheesy how this shares the break with the default case. I suggest we remove the default altogether so we get the gcc warning about any missing case values. r=me
Comment on attachment 13933 [details] special handling for empty table cells We need a new patch that includes the additional test case and new results for all the existing tests where results change. So I'm clearing the review flag form this patch.
r21145