The CSS height property is ignored on table rows. Test cases: 1. height applied to tr elements. http://aurore.net/safari/tr.html 2. height applied to td elements. http://aurore.net/safari/td.html Results: Safari misrenders the table if height is applied to tr elements. It generally ignores the height property. Sometimes the first row gets the correct height, and all other rows get auto height. Works fine if height is applied to td elements.
Created attachment 2147 [details] Another testcase
Same behaviour on Mac OS X 10.3.9 / Safari 1.3 (v312)
Uploaded yet another test case to <http://nypop.com/~ap/webkit/rowheight.html>. Safari, Firefox and WinIE all render it differently: - Firefox always honors the height (Opera 8.50 does the same); - WinIE sets the height to 1 if the row is empty, honors it otherwise. The test case includes a rather weird usage from www.fenzin.ru - <tr height=9><td><img src="images/ spacer.gif" border=0 hspace=0 vspace=0 width=1 height=6 align=left></td></tr> (why specify a different height for the spacer image itself?).
Still a bug in TOT.
(In reply to comment #3) > why specify a different height for the spacer image itself? Perhaps the other three pixels are intercell spacing? Perhaps the web developer can't tell the difference between 6 and 9?
The main culprit here is in RenderTableSection::calcRowHeight() at RenderTableSection.cpp:293: int pos = rowPos[r + 1] + ch + spacing; should really be int pos = rowPos[r] + ch + spacing; That change alone fixes a few of the table layout tests, but breaks a few others, so there's further tweaking that needs to be done beyond that.
Created attachment 8437 [details] Patch
Created attachment 8516 [details] Patch, including change log and updated layout test results Note that the patch doesn't include updated pixel results (or checksums).
Comment on attachment 8516 [details] Patch, including change log and updated layout test results r=me... did you go over the tests to make sure the new renderings are all correct?
(In reply to comment #9) > (From update of attachment 8516 [details] [edit]) > r=me... did you go over the tests to make sure the new renderings are all > correct? > Yes :-) There is one test that the patch will break if applied before the patch for bug 9012. (I didn't include the broken results in the patch).