Created attachment 91497 [details] Testcase Begin typing the in the rtl number input in the attached testcase. Notice that the input overlaps the spinner.
The behavior is different between Safari 5.0.5 and ToT, but badly broken in both.
Created attachment 101428 [details] Patch
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.
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.
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.
(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';
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.
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.
Committed r91345: <http://trac.webkit.org/changeset/91345>