Bug 11277

Summary: REGRESSION: Incomplete repaint of overflow areas when deleting
Product: WebKit Reporter: mitz
Component: FormsAssignee: Adele Peterson <adele>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, dev+webkit, tyler
Priority: P1 Keywords: InRadar, Regression
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: data:text/html,<textarea>Delete%20the%20following%20word:%20character</textarea>
Attachments:
Description Flags
patch
adele: review-
test case
none
Test case showing that repainting isn't offset by the scroll amount.
none
patch
none
updated patch
none
another patch mitz: review+

Description mitz 2006-10-13 03:47:13 PDT
When the number of lines in a textarea decreases, the last (old) line isn't cleared.
To reproduce: open the URL, move the insertion point after the word "character" and delete it character by character. The "ch" will be left behind as the word starts fitting on the first line. Forcing a repaint will erase it.
Comment 1 mitz 2006-10-13 03:58:11 PDT
Looks like result of r17018 (fix for <rdar://problem/4650813> REGRESSION: typing in a textarea in Safari is extremely slow).
Comment 2 mitz 2006-10-13 07:21:13 PDT
I think this is what's killing the repaint, from RenderBlock::layoutInlineChildren():

        if (hasOverflowClip())
            // Don't allow this rect to spill out of our overflow box.
            repaintRect.intersect(IntRect(0, 0, m_width, m_height));

When the above code executes, the inner div still hasn't reached its final height, so part of the needed repaint rect is clipped. I think that code can be moved to the end of layoutBlock(), where the repaint rect is used (and when the final height has been calculated).

Before the subtree optimization, some ancestor's repaint was hiding this bug.
Comment 3 Adele Peterson 2006-10-13 17:14:58 PDT
Created attachment 11081 [details]
patch

I was unable to reproduce this bug in an automated test case.
Comment 4 Adele Peterson 2006-10-13 17:39:23 PDT
Comment on attachment 11081 [details]
patch

after talking w/ Mitz, I don't think this is the right solution
Comment 5 Adele Peterson 2006-10-13 17:52:15 PDT
Created attachment 11082 [details]
test case

attaching test case
Comment 6 mitz 2006-10-15 00:23:04 PDT
Another thing I noticed about the current code is that it neglects to account for the inner div's scroll offset.
Comment 7 mitz 2006-10-15 00:38:32 PDT
Created attachment 11093 [details]
Test case showing that repainting isn't offset by the scroll amount.
Comment 8 mitz 2006-10-15 23:29:44 PDT
*** Bug 11311 has been marked as a duplicate of this bug. ***
Comment 9 David Harrison 2006-10-17 15:12:27 PDT
<rdar://problem/4788524> REGRESSION: Incomplete repaint of text area when deleting (11277)
Comment 10 Adele Peterson 2006-11-01 13:43:32 PST
Changing title to something more generic
Comment 11 Adele Peterson 2006-11-01 13:59:15 PST
Created attachment 11338 [details]
patch

This patch fixes a basic overflow case, and the scroll offset problem, but doesn't fix the flexbox problem.  We'll need to fix that separately.
Comment 12 Adele Peterson 2006-11-01 14:16:40 PST
Created attachment 11340 [details]
updated patch

updated patch to include a simpler layout test for the scrolling case
Comment 13 Adele Peterson 2006-11-01 15:21:58 PST
Created attachment 11342 [details]
another patch

one more try...

this adds a test case to make sure outlines aren't getting clipped, and moves the code around about to fix that case.
Comment 14 mitz 2006-11-01 15:32:51 PST
Comment on attachment 11342 [details]
another patch

r=me with a change we discussed on IRC: adding a !repaintRect.isEmpty() to the check before the call to addRepaintInfo
Comment 15 Adele Peterson 2006-11-01 15:36:37 PST
Committed revision 17524.