Bug 9012 - Row height not updated when cell heights change
Summary: Row height not updated when cell heights change
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tables (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-20 02:56 PDT by mitz
Modified: 2006-05-31 23:02 PDT (History)
0 users

See Also:


Attachments
Test case (1.32 KB, text/html)
2006-05-20 02:57 PDT, mitz
no flags Details
Patch w/test and change log (11.08 KB, patch)
2006-05-20 05:05 PDT, mitz
no flags Details | Formatted Diff | Diff
Revised patch (11.07 KB, patch)
2006-05-21 07:53 PDT, mitz
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2006-05-20 02:56:52 PDT
Grid row heights are initially increased as dictated by cells in the row (in RenderTableSection::addCell), but later changes to cell heights are not reflected in the grid. (Once bug 3297 is fixed, this becomes more apparent since it starts affecting rows other than the first row).

To reproduce: open the attached test case and click the Test button. After you click, the tables should be identical, but the table on the left maintains a taller first row due to this bug.
Comment 1 mitz 2006-05-20 02:57:27 PDT
Created attachment 8431 [details]
Test case
Comment 2 mitz 2006-05-20 05:05:30 PDT
Created attachment 8432 [details]
Patch w/test and change log

tables/mozilla_expected_failures/bugs/bug222846.html no longer fails with this patch, so perhaps it should be moved to mozilla (I don't know how to represent a move in a patch).
Comment 3 Dave Hyatt 2006-05-20 22:18:27 PDT
Shouldn't this:

+    if (parent() && section() && (!style() || style()->height() != newStyle->height()))
+        section()->setNeedCellRecalc();

be:

(style() && style()->height() != newStyle->height())

?

Doesn't seem like you need to call setNeedCellRecalc on a section when you don't even have a style() set yet.  (The act of just adding the cell to the DOM in the first place already took care of that case I think.)

Comment 4 mitz 2006-05-21 07:53:58 PDT
Created attachment 8445 [details]
Revised patch

Updated to address Hyatt's comment.
Comment 5 Dave Hyatt 2006-05-21 17:33:32 PDT
Comment on attachment 8445 [details]
Revised patch

r=me