Summary: | Incomplete repaint of table cell's collapsed border when changing only the cell | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | mitz | ||||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | ||||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 420+ | ||||||||||||||
Hardware: | Mac | ||||||||||||||
OS: | OS X 10.4 | ||||||||||||||
Attachments: |
|
Description
mitz
2006-10-19 11:43:52 PDT
Created attachment 11150 [details]
Test case
Created attachment 12057 [details]
Override repaint rect computation for table cells
Includes change log and repaint test.
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>
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.
(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". 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 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
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 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 on attachment 12078 [details]
Override repaint rect computation for table cells
r=me
Committed revision 18454. This has caused an assertion failure in dom/html/level2/html/HTMLCollection01.html; reverted in r18455. Comment on attachment 12078 [details]
Override repaint rect computation for table cells
Bad, bad patch. Clearing the review flag to protect the innocent.
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 on attachment 12123 [details]
Override repaint rect computation for table cells
r=me again (I read your comment, and I believe it)
Committed revision 18516. |