Bug 99291 - Input elements with multiple fields UI should set appropriate direction for browser locale automatically
Summary: Input elements with multiple fields UI should set appropriate direction for b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords: WebExposed
Depends on: 98992
Blocks: 98226
  Show dependency tree / graph
 
Reported: 2012-10-14 21:33 PDT by Kent Tamura
Modified: 2012-10-15 17:39 PDT (History)
6 users (show)

See Also:

Attachments
Patch (228.14 KB, patch)
2012-10-14 22:53 PDT, Kent Tamura 		morrita: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 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.
Comment 1 Kent Tamura 2012-10-14 22:53:03 PDT 
Created attachment 168627 [details]
Patch
Comment 2 Kent Tamura 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.
Comment 3 Kentaro Hara 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.
Comment 4 Hajime Morrita 2012-10-15 00:31:13 PDT 
Comment on attachment 168627 [details]
Patch

What about other @types?
Comment 5 Kent Tamura 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.
Comment 6 WebKit Review Bot 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
Comment 7 WebKit Review Bot 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
Comment 8 Kent Tamura 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.
Comment 9 Kent Tamura 2012-10-15 17:39:30 PDT 
Committed r131389: <http://trac.webkit.org/changeset/131389>