Bug 13209 - REGRESSION (r18756-18765): Incomplete list marker repaint when resizing list item
Summary: REGRESSION (r18756-18765): Incomplete list marker repaint when resizing list ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Nobody
URL:
Keywords: HasReduction, Regression
Depends on:
Blocks:
 
Reported: 2007-03-27 14:59 PDT by Matt Lilek
Modified: 2007-03-31 01:43 PDT (History)
1 user (show)

See Also:


Attachments
Reduction (371 bytes, text/html)
2007-03-27 15:00 PDT, Matt Lilek
no flags Details
Use the final horizontal overflows for repainting changed lines. (40.80 KB, patch)
2007-03-27 16:55 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 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.