Bug 13209

Summary: REGRESSION (r18756-18765): Incomplete list marker repaint when resizing list item
Product: WebKit Reporter: Matt Lilek <dev+webkit>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz
Priority: P1 Keywords: HasReduction, Regression
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Reduction
none
Use the final horizontal overflows for repainting changed lines. hyatt: review+

Description Matt Lilek 2007-03-27 14:59:35 PDT
Resizing a list item does not properly repaint it's list marker.

Seems to have regressed between r18756 and r18765 - likely http://trac.webkit.org/projects/webkit/changeset/18758.
Comment 1 Matt Lilek 2007-03-27 15:00:00 PDT
Created attachment 13834 [details]
Reduction
Comment 2 mitz 2007-03-27 16:25:42 PDT
Actually, the regression happened in r18762.
Comment 3 mitz 2007-03-27 16:55:52 PDT
Created attachment 13835 [details]
Use the final horizontal overflows for repainting changed lines.

Change log and repaint test included.
Comment 4 Dave Hyatt 2007-03-28 15:09:13 PDT
Comment on attachment 13835 [details]
Use the final horizontal overflows for repainting changed lines.

-    IntRect repaintRect(0, 0, 0, 0);
-    if (useRepaintRect) {
-        repaintRect.setX(m_overflowLeft);
-        repaintRect.setWidth(m_overflowWidth - m_overflowLeft);
-        repaintRect.setY(repaintTop);
-        repaintRect.setHeight(repaintBottom - repaintTop);
-    }
-

Why is it ok to remove this part?
Comment 5 mitz 2007-03-28 15:15:56 PDT
(In reply to comment #4)
> (From update of attachment 13835 [details] [edit])
> -    IntRect repaintRect(0, 0, 0, 0);
> -    if (useRepaintRect) {
> -        repaintRect.setX(m_overflowLeft);
> -        repaintRect.setWidth(m_overflowWidth - m_overflowLeft);
> -        repaintRect.setY(repaintTop);
> -        repaintRect.setHeight(repaintBottom - repaintTop);
> -    }
> -
> 
> Why is it ok to remove this part?
> 

Because of the addition of this part:

+    if (!didFullRepaint && repaintTop != repaintBottom) {
+        IntRect repaintRect(m_overflowLeft, repaintTop, m_overflowWidth - m_overflowLeft, repaintBottom - repaintTop);

What the patch does is it leaves layoutInlineChildren() to do the computation of the top and bottom of the repaint rect, but the left and right are determined afterwards in layoutBlock() when the final overflows are known.
Comment 6 Dave Hyatt 2007-03-28 15:18:05 PDT
Comment on attachment 13835 [details]
Use the final horizontal overflows for repainting changed lines.

r=me
Comment 7 Dave Hyatt 2007-03-28 15:18:34 PDT
Try to test editing a bit if you can with this change, since it really exercise the incremental line repaint code.
Comment 8 Alexey Proskuryakov 2007-03-31 01:43:10 PDT
Committed revision 20636.