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+

Matt Lilek
Reported 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.
Attachments
Reduction (371 bytes, text/html)
2007-03-27 15:00 PDT, Matt Lilek
no flags
Use the final horizontal overflows for repainting changed lines. (40.80 KB, patch)
2007-03-27 16:55 PDT, mitz
hyatt: review+
Matt Lilek
Comment 1 2007-03-27 15:00:00 PDT
Created attachment 13834 [details] Reduction
mitz
Comment 2 2007-03-27 16:25:42 PDT
Actually, the regression happened in r18762.
mitz
Comment 3 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.
Dave Hyatt
Comment 4 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?
mitz
Comment 5 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.
Dave Hyatt
Comment 6 2007-03-28 15:18:05 PDT
Comment on attachment 13835 [details] Use the final horizontal overflows for repainting changed lines. r=me
Dave Hyatt
Comment 7 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.
Alexey Proskuryakov
Comment 8 2007-03-31 01:43:10 PDT
Committed revision 20636.
Note You need to log in before you can comment on or make changes to this bug.