Bug 108283

Summary:

REGRESSION (r139282): Caret repainting is broken for text-align: center'd <input>

Product:

WebKit

Reporter:

Tim Horton <thorton>

Component:

Forms

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aelias, ap, eae, leviw, mifenton, simon.fraser, trchen, webkit.review.bot

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
test	none
off caret	none
on caret	none
Patch	none
Patch	none
Patch	none

Tim Horton

Reported 2013-01-29 18:54:13 PST

See attached testcase; the caret is not fully erased when blinking. Reproduces in Safari on Mountain Lion. I have a hunch that this is due to a combination of r139282 (confirmed by bisection) and subpixel layout (entirely speculative). <rdar://problem/13062683>

Attachments
test (265 bytes, text/html) 2013-01-29 18:54 PST, Tim Horton	no flags	Details
off caret (7.14 KB, image/png) 2013-01-29 18:54 PST, Tim Horton	no flags	Details
on caret (6.83 KB, image/png) 2013-01-29 18:54 PST, Tim Horton	no flags	Details
Patch (3.97 KB, patch) 2013-01-30 00:24 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (4.98 KB, patch) 2013-01-30 02:32 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (4.90 KB, patch) 2013-01-30 02:38 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Tim Horton

Comment 1 2013-01-29 18:54:28 PST

Created attachment 185375 [details] test

Tim Horton

Comment 2 2013-01-29 18:54:40 PST

Created attachment 185376 [details] off caret

Tim Horton

Comment 3 2013-01-29 18:54:52 PST

Created attachment 185377 [details] on caret

Alexandre Elias

Comment 4 2013-01-29 20:32:39 PST

Also reproducing on Mac Chromium 25.0.1364.45, looks like cross-platform issue.

Tien-Ren Chen

Comment 5 2013-01-29 23:54:50 PST

Confirmed on chromium-linux 25.0.1364.45 too. Looks like we still need the 1-pixel padding workaround. I'm uploading a patch now.

Tien-Ren Chen

Comment 6 2013-01-30 00:24:53 PST

Created attachment 185414 [details] Patch

Tien-Ren Chen

Comment 7 2013-01-30 00:26:32 PST

Don't have access to a Mac right now. Waiting for Mac EWS bot result to update TestExpectations.

Tien-Ren Chen

Comment 8 2013-01-30 02:17:31 PST

Comment on attachment 185414 [details] Patch I'm surprised Mac EWS passed all the tests. Do they generate pixel results properly for repaint regions?

Tim Horton

Comment 9 2013-01-30 02:18:03 PST

(In reply to comment #8) > (From update of attachment 185414 [details]) > I'm surprised Mac EWS passed all the tests. Do they generate pixel results properly for repaint regions? Mac EWS doesn't run pixel tests.

Tien-Ren Chen

Comment 10 2013-01-30 02:19:13 PST

Got it. I'll run it on my Macbook then.

Tien-Ren Chen

Comment 11 2013-01-30 02:32:26 PST

Created attachment 185442 [details] Patch

Tim Horton

Comment 12 2013-01-30 02:35:14 PST

Comment on attachment 185442 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185442&action=review > Source/WebCore/editing/FrameSelection.cpp:1330 > + // FIXME: Need to over-paint 1 pixel to workaround some rounding problem. s/some/a/ or s/problem/problems/. > Source/WebCore/editing/FrameSelection.cpp:1332 > + // Can't reproduce http://webkit.org/b/19086 but probably the same root cause. This line doesn't belong here.

Tien-Ren Chen

Comment 13 2013-01-30 02:38:40 PST

Created attachment 185445 [details] Patch

WebKit Review Bot

Comment 14 2013-01-30 03:22:03 PST

Comment on attachment 185445 [details] Patch Clearing flags on attachment: 185445 Committed r141243: <http://trac.webkit.org/changeset/141243>

WebKit Review Bot

Comment 15 2013-01-30 03:22:07 PST

All reviewed patches have been landed. Closing bug.

Emil A Eklund

Comment 16 2013-01-30 09:18:03 PST

Thanks for adding back that workaround, I'll try to figure out why we get the repaint rect wrong for the caret when I get a chance. We had similar problems in chromium when we first turned on subpixel layout.

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