Bug 86784

Summary:

WebKit erroneously add 1px padding in input elements

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

Forms

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

adele, ap, darin, dglazkov, d-r, eric, hyatt, jussi.kukkonen, macpherson, menard, naginenis, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

86824, 86825

Bug Blocks:

71323

Attachments:

Description	Flags
screenshot demonstrating the bug	none
ie9 (standard mode)	none
ie9 (quirks mode)	none
Chrome 18 (for comparison)	none
Fixes the bug	tkent: review+, webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-01	none

Ryosuke Niwa

Reported 2012-05-17 16:01:52 PDT

Created attachment 142575 [details] screenshot demonstrating the bug Open the following HTML in WebKit. WebKit renders the caret inside the input element with extra 1px padding whereas the caret inside the textarea doesn't have this 1px padding. <html> <head> <style type="text/css"> .common { font-family: arial,sans-serif; padding-left: 10px; border: 1px solid #D9D9D9; } </style> </head> <body> <input type="text" class="common" value="|"> <br> <textarea class="common">|</textarea> </body> </html> tkent identified this is caused by: http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp#L481 which was added in http://trac.webkit.org/changeset/13567. http://crbug.com/128086

Attachments
screenshot demonstrating the bug (3.95 KB, image/png) 2012-05-17 16:01 PDT, Ryosuke Niwa	no flags	Details
ie9 (standard mode) (3.50 KB, image/png) 2012-05-17 16:02 PDT, Ryosuke Niwa	no flags	Details
ie9 (quirks mode) (3.51 KB, image/png) 2012-05-17 16:02 PDT, Ryosuke Niwa	no flags	Details
Chrome 18 (for comparison) (3.35 KB, image/png) 2012-05-17 16:03 PDT, Ryosuke Niwa	no flags	Details
Fixes the bug (deleted) 2012-05-17 22:50 PDT, Ryosuke Niwa	tkent: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ec2-cr-linux-01 (887.45 KB, application/zip) 2012-05-17 23:33 PDT, WebKit Review Bot	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2012-05-17 16:02:29 PDT

Created attachment 142576 [details] ie9 (standard mode)

Ryosuke Niwa

Comment 2 2012-05-17 16:02:53 PDT

Created attachment 142577 [details] ie9 (quirks mode)

Ryosuke Niwa

Comment 3 2012-05-17 16:03:20 PDT

Created attachment 142579 [details] Chrome 18 (for comparison)

Ryosuke Niwa

Comment 4 2012-05-17 16:04:08 PDT

It appears that IE doesn't exhibit this behavior anymore.

Ryosuke Niwa

Comment 5 2012-05-17 18:20:23 PDT

Just removing the said code doesn't seem to work. fast/forms/input-placeholder-text-indent.html fails with off-by-one error :\

Kent Tamura

Comment 6 2012-05-17 18:24:17 PDT

(In reply to comment #5) > Just removing the said code doesn't seem to work. > fast/forms/input-placeholder-text-indent.html fails with off-by-one error :\ We need to remove paddings of ::-webkit-input-placeholder in html.css.

Ryosuke Niwa

Comment 7 2012-05-17 20:00:19 PDT

(In reply to comment #6) > (In reply to comment #5) > > Just removing the said code doesn't seem to work. > > fast/forms/input-placeholder-text-indent.html fails with off-by-one error :\ > > We need to remove paddings of ::-webkit-input-placeholder in html.css. That makes sense. I was looking for that in C++ code :( Thanks!

Ryosuke Niwa

Comment 8 2012-05-17 21:15:43 PDT

This is a very simple fix but requires a lot of rebaselines (~150 in each platform). Will upload a patch in half an hour.

Ryosuke Niwa

Comment 9 2012-05-17 22:50:59 PDT

Created attachment 142634 [details] Fixes the bug

Kent Tamura

Comment 10 2012-05-17 23:18:35 PDT

Comment on attachment 142634 [details] Fixes the bug If we need to update a lot of test results, I prefer just marking failures in test_expectations.txt in the patch ;-)

Ryosuke Niwa

Comment 11 2012-05-17 23:20:44 PDT

(In reply to comment #10) > (From update of attachment 142634 [details]) > If we need to update a lot of test results, I prefer just marking failures in test_expectations.txt in the patch ;-) Yeah. I probably need to do that instead (or just land and rebaseline them in non-PST time). It's not feasible to land this patch as is :( unless I want to clash the svn server.

WebKit Review Bot

Comment 12 2012-05-17 23:32:59 PDT

Comment on attachment 142634 [details] Fixes the bug Attachment 142634 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12726365 New failing tests: editing/selection/select-across-readonly-input-5.html css3/selectors3/xml/css3-modsel-24.xml editing/selection/drag-select-1.html css3/selectors3/xhtml/css3-modsel-24.xml editing/pasteboard/4806874.html editing/selection/3690719.html css3/selectors3/xhtml/css3-modsel-68.xml editing/selection/4895428-3.html css3/selectors3/html/css3-modsel-24.html editing/input/caret-at-the-edge-of-input.html editing/inserting/before-after-input-element.html editing/selection/select-across-readonly-input-1.html css3/selectors3/xml/css3-modsel-23.xml editing/selection/3690703-2.html editing/selection/4975120.html editing/selection/select-across-readonly-input-2.html editing/selection/select-across-readonly-input-4.html editing/selection/select-across-readonly-input-3.html css3/selectors3/xml/css3-modsel-69.xml editing/pasteboard/drop-text-without-selection.html editing/selection/3690703.html css3/selectors3/html/css3-modsel-23.html css3/selectors3/xml/css3-modsel-68.xml css3/selectors3/html/css3-modsel-69.html css3/selectors3/xhtml/css3-modsel-69.xml editing/pasteboard/input-field-1.html css3/selectors3/xhtml/css3-modsel-23.xml css3/selectors3/html/css3-modsel-68.html

WebKit Review Bot

Comment 13 2012-05-17 23:33:04 PDT

Created attachment 142645 [details] Archive of layout-test-results from ec2-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Ryosuke Niwa

Comment 14 2012-05-18 01:45:49 PDT

Oops, I just realized that my previous analysis was wrong. It's not that IE doesn't add 1px padding to input element anymore. It's that IE also adds that 1px padding to textarea :(

Ryosuke Niwa

Comment 15 2012-05-18 02:48:39 PDT

This patch was landed in r117556. And rebaselines were done in r117557, r117558, and r117559. But got reverted in r117565, r117566, r117567, and r117570.

Alexey Proskuryakov

Comment 16 2012-05-18 12:56:58 PDT

Is this the same as bug 71323? > This patch was landed in r117556. Should this bug be RESOLVED/FIXED now?

Ryosuke Niwa

Comment 17 2012-05-18 12:59:21 PDT

(In reply to comment #16) > Is this the same as bug 71323? > > > This patch was landed in r117556. > > Should this bug be RESOLVED/FIXED now? No, I reverted it after realizing that my original analysis suffered from off-by-one error.

Ryosuke Niwa

Comment 18 2012-05-18 14:15:36 PDT

Okay.. my second analysis was also wrong. The test case for the bug 71323 works fine on IE9: https://bugs.webkit.org/show_bug.cgi?id=71323

Ryosuke Niwa

Comment 19 2012-05-18 14:45:00 PDT

I'm sorry, I don't know what's wrong with me. Maybe because it was too late in the evening but it appears that my first analysis was correct (or perhaps that I was fooled by screenshots seen through Screen Share), and my comment #14 was bogus. I'm going to re-land the patch later today while my brain is functioning properly.

Ryosuke Niwa

Comment 20 2012-05-19 02:34:09 PDT

Committed in http://trac.webkit.org/changeset/117672. Rebaselines: http://trac.webkit.org/changeset/117673 http://trac.webkit.org/changeset/117674 http://trac.webkit.org/changeset/117676 (philn) http://trac.webkit.org/changeset/117677 http://trac.webkit.org/changeset/117679 http://trac.webkit.org/changeset/117681 (ossy) http://trac.webkit.org/changeset/117682 http://trac.webkit.org/changeset/117686

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