Bug 99291

Summary: Input elements with multiple fields UI should set appropriate direction for browser locale automatically
Product: WebKit Reporter: Kent Tamura <tkent>
Component: FormsAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, haraken, mifenton, morrita, webkit.review.bot, yutak
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 98992    
Bug Blocks: 98226    
Attachments:
Description Flags
Patch morrita: review+, webkit.review.bot: commit-queue-

Kent Tamura
Reported 2012-10-14 21:33:31 PDT
If the page is written in English (dir=ltr) but the browser locale is Arabic (RTL), input[type=date] should be laid out as dir=rtl because its content is for Arabic.
Attachments
Patch (228.14 KB, patch)
2012-10-14 22:53 PDT, Kent Tamura 		morrita: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Kent Tamura
Comment 1 2012-10-14 22:53:03 PDT
Created attachment 168627 [details] Patch
Kent Tamura
Comment 2 2012-10-14 22:57:09 PDT
Comment on attachment 168627 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168627&action=review > Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp:276 > + AtomicString direction = element()->localizer().isRTL() ? AtomicString("rtl", AtomicString::ConstructFromLiteral) : AtomicString("ltr", AtomicString::ConstructFromLiteral); Note: If we wrote AtomicString(...isRTL() ? "rtl" : "ltr", AtomicString::ConstructFromLiteral), VC++ couldn't compile it.
Kentaro Hara
Comment 3 2012-10-14 23:40:15 PDT
Comment on attachment 168627 [details] Patch LGTM, but I'd like to delegate the review to morrita-san.
Hajime Morrita
Comment 4 2012-10-15 00:31:13 PDT
Comment on attachment 168627 [details] Patch What about other @types?
Kent Tamura
Comment 5 2012-10-15 00:34:00 PDT
(In reply to comment #4) > (From update of attachment 168627 [details]) > What about other @types? Good point. This change affects date, datetime, datetime-local, month, time, and week types with multiple fields UI. However some types have no layout tests yet.
WebKit Review Bot
Comment 6 2012-10-15 13:59:13 PDT
Comment on attachment 168627 [details] Patch Rejecting attachment 168627 [details] from commit-queue. New failing tests: platform/chromium/fast/forms/date/date-suggestion-picker-appearance-rtl.html platform/chromium/fast/forms/time/time-suggestion-picker-appearance-rtl.html Full output: http://queues.webkit.org/results/14293803
WebKit Review Bot
Comment 7 2012-10-15 16:38:05 PDT
Comment on attachment 168627 [details] Patch Attachment 168627 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14290816 New failing tests: platform/chromium/fast/forms/date/date-suggestion-picker-appearance-rtl.html platform/chromium/fast/forms/time/time-suggestion-picker-appearance-rtl.html
Kent Tamura
Comment 8 2012-10-15 17:30:51 PDT
(In reply to comment #6) > (From update of attachment 168627 [details]) > Rejecting attachment 168627 [details] from commit-queue. > > New failing tests: > platform/chromium/fast/forms/date/date-suggestion-picker-appearance-rtl.html > platform/chromium/fast/forms/time/time-suggestion-picker-appearance-rtl.html I should have added them to TestExpectations as expected failures.
Kent Tamura
Comment 9 2012-10-15 17:39:30 PDT
Committed r131389: <http://trac.webkit.org/changeset/131389>
Note You need to log in before you can comment on or make changes to this bug.