Bug 11277 - REGRESSION: Incomplete repaint of overflow areas when deleting
Summary: REGRESSION: Incomplete repaint of overflow areas when deleting
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Adele Peterson
URL: data:text/html,<textarea>Delete%20the...
Keywords: InRadar, Regression
: 11311 (view as bug list)
Depends on:
Reported: 2006-10-13 03:47 PDT by mitz
Modified: 2006-11-01 15:36 PST (History)
3 users (show)

See Also:

patch (1.85 KB, patch)
2006-10-13 17:14 PDT, Adele Peterson
adele: review-
Details | Formatted Diff | Diff
test case (771 bytes, patch)
2006-10-13 17:52 PDT, Adele Peterson
no flags Details | Formatted Diff | Diff
Test case showing that repainting isn't offset by the scroll amount. (786 bytes, text/html)
2006-10-15 00:38 PDT, mitz
no flags Details
patch (10.89 KB, patch)
2006-11-01 13:59 PST, Adele Peterson
no flags Details | Formatted Diff | Diff
updated patch (10.74 KB, patch)
2006-11-01 14:16 PST, Adele Peterson
no flags Details | Formatted Diff | Diff
another patch (14.81 KB, patch)
2006-11-01 15:21 PST, Adele Peterson
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]

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]

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]

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.