Bug 11359

Summary: Incomplete repaint of table cell's collapsed border when changing only the cell
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Test case
none
Override repaint rect computation for table cells
hyatt: review-
Override repaint rect computation for table cells
none
Override repaint rect computation for table cells
none
Override repaint rect computation for table cells darin: review+

Description mitz 2006-10-19 11:43:52 PDT
See the attached test case, where the "table half" of the border doesn't repaint in green when you click the Test button, until you force a repaint.
Comment 1 mitz 2006-10-19 11:44:38 PDT
Created attachment 11150 [details]
Test case
Comment 2 mitz 2006-12-27 05:39:35 PST
Created attachment 12057 [details]
Override repaint rect computation for table cells

Includes change log and repaint test.
Comment 3 Dave Hyatt 2006-12-27 13:15:32 PST
Comment on attachment 12057 [details]
Override repaint rect computation for table cells

It seems like you aren't dealing with the case where a table cell has overflow and collapsed borders correctly.

For example:

<td><div style="width:100px">... put something in here that overflows...</div></td>
Comment 4 mitz 2006-12-27 15:09:39 PST
Created attachment 12072 [details]
Override repaint rect computation for table cells

Include horizontal overflow. I don't know if vertical overflow should ever occur. I managed to get vertical overflow but I think that's a bug.
Comment 5 mitz 2006-12-27 15:28:23 PST
(In reply to comment #4)
> I don't know if vertical overflow should ever
> occur. I managed to get vertical overflow but I think that's a bug.
> 

I guess it could happen with an inline-block that has overflow, but currently such overflow counts as "layout overflow".
Comment 6 Dave Hyatt 2006-12-27 18:03:57 PST
I might just write this code assuming that layout vertical overflow could occur just to be paranoid, since I think maybe there are weird degenerate span cases that might cause it to happen.
Comment 7 Darin Adler 2006-12-27 22:30:44 PST
Comment on attachment 12072 [details]
Override repaint rect computation for table cells

+            if (RenderTableCell* above = table()->cellAbove(this)) {
+            left = max(left, above->borderLeft(true));
+            right = max(right, above->borderRight(true));
+            }

Indenting mistake.

+    int borderLeft() const { return borderLeft(false); }

Why overloading instead of a default value for the parameter? This seems OK, but normally I'd only overload if I didn't want the parameter value inlined.

r=me
Comment 8 mitz 2006-12-27 23:44:47 PST
Created attachment 12078 [details]
Override repaint rect computation for table cells

Added vertical overflow and fixed indentation. I didn't want to add the 'outer' parameter to borderLeft() in the base class. Instead, now the function that takes a parameter (and is defined for RenderTableCell only) is named borderHalfLeft().
Comment 9 mitz 2006-12-27 23:48:05 PST
Comment on attachment 12072 [details]
Override repaint rect computation for table cells

Removing the r+ to keep this from showing in the commit queue (I thought obsoleting alone would do it).
Comment 10 Darin Adler 2006-12-28 01:06:32 PST
Comment on attachment 12078 [details]
Override repaint rect computation for table cells

r=me
Comment 11 Alexey Proskuryakov 2006-12-28 10:44:39 PST
Committed revision 18454.
Comment 12 Alexey Proskuryakov 2006-12-28 11:31:11 PST
This has caused an assertion failure in dom/html/level2/html/HTMLCollection01.html; reverted in r18455.
Comment 13 mitz 2006-12-28 11:32:41 PST
Comment on attachment 12078 [details]
Override repaint rect computation for table cells

Bad, bad patch. Clearing the review flag to protect the innocent.
Comment 14 mitz 2006-12-30 08:58:22 PST
Created attachment 12123 [details]
Override repaint rect computation for table cells

Fixed the crash by ignoring outside borders if the table grid is dirty. Added a long comment explaining why I think this is okay.

I named the getter needsSectionRecalc despite the setter being called setNeedSectionRecalc. I intend to clean up the naming in a future cleanup patch.
Comment 15 Darin Adler 2007-01-01 20:15:51 PST
Comment on attachment 12123 [details]
Override repaint rect computation for table cells

r=me again (I read your comment, and I believe it)
Comment 16 David Kilzer (:ddkilzer) 2007-01-01 20:53:32 PST
Committed revision 18516.