Bug 59703

Summary:

input type=number doesn't render correctly in rtl

Product:

WebKit

Reporter:

Tony Gentilcore <tonyg>

Component:

Forms

Assignee:

Kent Tamura <tkent>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

mitz, morrita, playmobil, rniwa, satish, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

Bug Blocks:

64108

Attachments:

Description	Flags
Testcase	none
Patch	rniwa: review+

Tony Gentilcore

Reported 2011-04-28 08:37:06 PDT

Created attachment 91497 [details] Testcase Begin typing the in the rtl number input in the attached testcase. Notice that the input overlaps the spinner.

Attachments
Testcase (28 bytes, text/html) 2011-04-28 08:37 PDT, Tony Gentilcore	no flags	Details
Patch (20.18 KB, patch) 2011-07-19 23:01 PDT, Kent Tamura	rniwa: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2011-04-28 15:57:26 PDT

The behavior is different between Safari 5.0.5 and ToT, but badly broken in both.

Kent Tamura

Comment 2 2011-07-19 23:01:53 PDT

Created attachment 101428 [details] Patch

WebKit Review Bot

Comment 3 2011-07-19 23:04:59 PDT

Attachment 101428 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 ERROR: FAILURES FOR <, x86, release, cpu> ERROR: Line:27 Test lacks BUG modifier. editing/selection/empty-cell-right-click.html ERROR: Line:28 Test lacks BUG modifier. editing/selection/dump-as-markup.html ERROR: Line:29 Test lacks BUG modifier. fast/js/array-sort-modifying-tostring.html ERROR: Line:30 Test lacks BUG modifier. fast/overflow/lots-of-sibling-inline-boxes.html ERROR: Line:31 Test lacks BUG modifier. inspector/cookie-resource-match.html LayoutTests/platform/qt/test_expectations.txt:27: Test lacks BUG modifier. editing/selection/empty-cell-right-click.html [test/expectations] [2] LayoutTests/platform/qt/test_expectations.txt:28: Test lacks BUG modifier. editing/selection/dump-as-markup.html [test/expectations] [2] LayoutTests/platform/qt/test_expectations.txt:29: Test lacks BUG modifier. fast/js/array-sort-modifying-tostring.html [test/expectations] [2] LayoutTests/platform/qt/test_expectations.txt:30: Test lacks BUG modifier. fast/overflow/lots-of-sibling-inline-boxes.html [test/expectations] [2] LayoutTests/platform/qt/test_expectations.txt:31: Test lacks BUG modifier. inspector/cookie-resource-match.html [test/expectations] [2] Total errors found: 5 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.

Jeremy Moskovich

Comment 4 2011-07-19 23:51:29 PDT

Thanks for posting this patch! Not a reviewer but, here's my take: I'd add a bit of text to the layout test so it's easy to tell what the desired behavior is. e.g. "in this input box the button should be on the left of the field" otherwise if one looks at the test without context it's hard to tell what the desired behavior is.

Ryosuke Niwa

Comment 5 2011-07-19 23:53:37 PDT

Comment on attachment 101428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101428&action=review > LayoutTests/fast/forms/input-appearance-number-rtl.html:3 > + > +<p dir=rtl><input type=number value=1> I'd like to see <html><body> and </p>'s since we're not really testing HTML parser here. > LayoutTests/fast/forms/input-appearance-number-rtl.html:14 > +document.getElementById('i5').dir = 'ltr'; Can we have another variant that sets dir to rtl? > Source/WebCore/css/html.css:430 > input::-webkit-textfield-decoration-container { > - direction: ltr; > display: -webkit-box; > -webkit-box-align: center; > } > > +input[type="search"]::-webkit-textfield-decoration-container { > + direction: ltr; > +} > + Does this fix input[type="speech"]? e.g. <p dir="rtl"><input type="speech" ...> If so, we should add a test for it.

Ryosuke Niwa

Comment 6 2011-07-19 23:54:23 PDT

(In reply to comment #5) > > LayoutTests/fast/forms/input-appearance-number-rtl.html:14 > > +document.getElementById('i5').dir = 'ltr'; > > Can we have another variant that sets dir to rtl? Also, it'll be nice to test style.direction = 'ltr'/'rtl';

Ryosuke Niwa

Comment 7 2011-07-19 23:56:30 PDT

Comment on attachment 101428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101428&action=review > LayoutTests/fast/forms/input-appearance-number-rtl.html:2 > + Also, as Jeremy pointed out, it'll be nice to have some description as to what this test is testing what output is expected. You can put it in a comment if you'd like to avoid random font-difference failures on the various bots.

Kent Tamura

Comment 8 2011-07-20 00:48:41 PDT

Comment on attachment 101428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101428&action=review >> LayoutTests/fast/forms/input-appearance-number-rtl.html:2 >> + > > Also, as Jeremy pointed out, it'll be nice to have some description as to what this test is testing what output is expected. You can put it in a comment if you'd like to avoid random font-difference failures on the various bots. Done >> LayoutTests/fast/forms/input-appearance-number-rtl.html:3 >> +<p dir=rtl><input type=number value=1> > > I'd like to see <html><body> and </p>'s since we're not really testing HTML parser here. Done. >> LayoutTests/fast/forms/input-appearance-number-rtl.html:14 >> +document.getElementById('i5').dir = 'ltr'; > > Can we have another variant that sets dir to rtl? Done. >> Source/WebCore/css/html.css:430 >> + > > Does this fix input[type="speech"]? e.g. <p dir="rtl"><input type="speech" ...> If so, we should add a test for it. x-webkit-speech case is covered by LayoutTests/fast/speech/speech-bidi-rendering.html.

Kent Tamura

Comment 9 2011-07-20 00:52:53 PDT

Committed r91345: <http://trac.webkit.org/changeset/91345>

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