Bug 8032

Summary:

REGRESSION: Focus ring not completely redrawn after a Delete changes its size

Product:

WebKit

Reporter:

Justin Garcia <justin.garcia>

Component:

HTML Editing

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, Graham.Dennis

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
testcase	none
patch	none
modified (kinda automatic) testcase	none
patch 2	darin: review+

Justin Garcia

Reported 2006-03-28 07:21:35 PST

This was caused by a change made between r12899 and r12934. I think that points to the fix for: <http://bugzilla.opendarwin.org/show_bug.cgi?id=6831> contentEditable outline darkens as caret moves See the attached test case. The farthest I could reduce this an unstyled div inside a contenteditable div with left padding. I can only produce the redraw problem by manually deleting the line break, I can't produce it with document.execCommand("Delete") or by removing the line break with JS.

Attachments
testcase (382 bytes, text/html) 2006-03-28 07:22 PST, Justin Garcia	no flags	Details
patch (2.36 KB, patch) 2006-04-01 05:16 PST, Graham Dennis	no flags	Details Formatted Diff Diff
modified (kinda automatic) testcase (460 bytes, text/html) 2006-04-01 05:18 PST, Graham Dennis	no flags	Details
patch 2 (4.69 KB, patch) 2006-04-01 21:52 PST, Graham Dennis	darin: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Justin Garcia

Comment 1 2006-03-28 07:22:31 PST

Created attachment 7360 [details] testcase

Graham Dennis

Comment 2 2006-03-28 15:42:05 PST

Before the patch to 6831, outlines did not obey clip rects. After the patch, they do. So the problem is that the clip rect generated when deleting doesn't go far enough left to cover the rest of the contentEditable div. Before the patch, the outline would ignore the clip rect and would be drawn in its entirety (but over part of the outline that wasn't cleared), and after, it's not drawn at all outside the clip rect.

Graham Dennis

Comment 3 2006-03-28 15:52:20 PST

Oops. It looks like the repaint rect does extend over the entire width of the div, it just doesn't seem to go down far enough to cover the outline.

Graham Dennis

Comment 4 2006-04-01 05:16:33 PST

Created attachment 7438 [details] patch When an element changes size, only part of the element is redrawn. The part that is redrawn is the difference between the old and the new rectangles taking into account borders that need to be redrawn. However, this code was not also taking into account outlines that may need to be redrawn. This patch fixes this. The reason that the original testcase attached with this patch could not be reproduced by adding document.execCommand("Delete"); to the end of the <script> tag is that the document has not been rendered by this point, and so the document is rendered only once, and hence correctly. I modified the testcase (I will attach) uses setTimeout("document.execCommand('Delete')", 0); to do the work, however, DumpRenderTree does a pixel dump before the timeout has been reached, and the test doesn't work in the LayoutTests. I don't know how else to cover this bug with a layout test.

Graham Dennis

Comment 5 2006-04-01 05:18:13 PST

Created attachment 7439 [details] modified (kinda automatic) testcase Testcase that doesn't work with DumpRenderTree (the png output displays the same as the first test case before pressing delete)

Darin Adler

Comment 6 2006-04-01 13:33:05 PST

Comment on attachment 7438 [details] patch Looks great! Needs a test. Perhaps ask Mitz for advice on how to make one since he's made tests for similar changes recently.

Graham Dennis

Comment 7 2006-04-01 21:52:42 PST

Created attachment 7454 [details] patch 2 This is the same patch as before, but it includes a manual testcase. Mitz mentioned that this bug is the outline version of the border repaint bug 6301. The manual testcase for this bug is pretty much a copy of the testcase for 6301 with 'border' replaced with outline.

Darin Adler

Comment 8 2006-04-02 12:50:23 PDT

Comment on attachment 7454 [details] patch 2 r=me

Note You need to log in before you can comment on or make changes to this bug.