RESOLVED FIXED Bug 97633
[Forms] Adding localization texts for multiple fields date/time input UI
https://bugs.webkit.org/show_bug.cgi?id=97633
Summary [Forms] Adding localization texts for multiple fields date/time input UI
yosin
Reported 2012-09-25 22:57:01 PDT
To implement multiple fields date/time input UI, we would like to have: 1. month format in LDML, e.g. "MM/yyyy" 2. week format in LDML, e.g. "W/yyyy" 3. date format indicates letters, e.g. "ymd" to display "dd-mm-yyyy"
Attachments
Patch 1 (6.13 KB, patch)
2012-09-25 23:59 PDT, yosin
no flags
Patch 2 (6.25 KB, patch)
2012-09-26 02:16 PDT, yosin
no flags
Patch 3 (6.25 KB, patch)
2012-09-26 02:22 PDT, yosin
no flags
Patch 4 (6.66 KB, patch)
2012-09-26 02:56 PDT, yosin
no flags
Patch 5 (6.50 KB, patch)
2012-09-26 22:01 PDT, yosin
no flags
Patch 6 (7.77 KB, patch)
2012-09-26 22:39 PDT, yosin
no flags
yosin
Comment 1 2012-09-25 23:59:41 PDT
yosin
Comment 2 2012-09-26 00:00:25 PDT
Comment on attachment 165739 [details] Patch 1 Could you review this patch? Thanks in advance.
WebKit Review Bot
Comment 3 2012-09-26 00:03:00 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Kent Tamura
Comment 4 2012-09-26 00:15:56 PDT
Comment on attachment 165739 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=165739&action=review > Source/WebCore/ChangeLog:16 > + Note: We'll rename ENABLE_INPUT_TYPE_MULTIPLE_FIELDS to generic name > + to support "date", "datetime", "month" and "week" multiple fields input > + UI. Why don't you rename it before this patch? > Source/WebCore/ChangeLog:22 > + (WebCore): Added declarations of dateFormatHelpLetters(), dateFormatHelpText(), monthFormatInLDML() and > + weekFormatInLDML(). Because we have no call sites of these functions yet, none except you can understand what the functions should return. Please explain what they are and why they are needed concretely.
yosin
Comment 5 2012-09-26 02:16:46 PDT
yosin
Comment 6 2012-09-26 02:22:41 PDT
yosin
Comment 7 2012-09-26 02:24:46 PDT
Comment on attachment 165757 [details] Patch 3 Could you review this patch? Thanks in advance. = Changes since the last review = * Update ChangeLog to refer ENABLE_INPUT_MULTIPLE_FIELDS_UI * Add more explanations for new functions. * Fix localized string identifiers in functions.
Kent Tamura
Comment 8 2012-09-26 02:30:56 PDT
Comment on attachment 165757 [details] Patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=165757&action=review > Source/WebKit/chromium/ChangeLog:15 > + (WebCore::dateFormatHelpLetters): Added for DateInputType, DateTimeInputType, and DateTimeLocalInputType to provide threeletters, e.g. "ymd" for English, for indicating order of day, month and year fields to display them intead of "-" when field has empty value. threeletters -> three letters? Is "年" one letter? Should we count a character of which code point is greater than U+FFFF as one letter? How about Unicode composition? > Source/WebKit/chromium/ChangeLog:16 > + (WebCore::dateFormatHelpText): Added for DateInputType, DateTimeInputType, and DateTimeLocalInputType to provide help text of order of day, month and year fields. still unclear. In what situation a user will see this text? What text is expected?
yosin
Comment 9 2012-09-26 02:56:38 PDT
yosin
Comment 10 2012-09-26 02:58:00 PDT
Comment on attachment 165762 [details] Patch 4 Could you review this patch? Thanks in advance. = Changes since the last review = * Add more explanations to new functions in ChangeLog.
Kent Tamura
Comment 11 2012-09-26 04:09:37 PDT
Comment on attachment 165762 [details] Patch 4 View in context: https://bugs.webkit.org/attachment.cgi?id=165762&action=review > Source/WebKit/chromium/ChangeLog:15 > + (WebCore::dateFormatHelpLetters): Added for DateInputType, DateTimeInputType, and DateTimeLocalInputType to provide three letters, e.g. "ymd" for English, for indicating order of day, month and year fields to display them intead of "-" when field has empty value. Each letter should have same width to "-" in monospace font-family. This function may return empty string if there are no such letters in locale or order of day, month and year is well-known in locale. This doesn't resolve my previous question. is a sequence of 'A' and U+0300 (COMBINING GRAVE ACCENT) one letter? Why don't you support at most two letters for one field though a day/month/year field has at least two digits? BTW, these explanation should be in LocalizedStrings.h and WebCore/ChangeLog, not WebKit/chromium/ChangeLog. > Source/WebKit/chromium/ChangeLog:16 > + (WebCore::dateFormatHelpText): Added for DateInputType, DateTimeInputType, and DateTimeLocalInputType to provide help text of order of day, month and year fields, e.g. "Day/Month/Year". This string will be displayed bubble when the input element is focused and dateFormatHelpLetters() returns empty string. This function may return empty string if order of day, month, and year is well-known in locale. We already have WebCore::localizedDateFormatText(). Also, a date format depends on a local machine setting, and we need some code to build this text. Such text is not suitable for LocalizedStrings.
yosin
Comment 12 2012-09-26 22:01:53 PDT
yosin
Comment 13 2012-09-26 22:03:45 PDT
Comment on attachment 165926 [details] Patch 5 Could you review this patch? Thanks in advance. = Changes since the last review = * Use placeholderFor{DayOfMonth,Month,Year}() instead of dateFormatHelp{Letters,Text}() for simplicity of implementation and ease of localization.
Kent Tamura
Comment 14 2012-09-26 22:18:14 PDT
Did you read my comment #11? > BTW, these explanation should be in LocalizedStrings.h and WebCore/ChangeLog, not WebKit/chromium/ChangeLog.
yosin
Comment 15 2012-09-26 22:39:27 PDT
yosin
Comment 16 2012-09-26 22:40:52 PDT
Comment on attachment 165930 [details] Patch 6 Could you review this patch? Thanks in advance. = Changes since the last review = * Move explanation of new functions into WebCore/platform/LocalizedString.h and WebCore/ChangeLog from WebKit/chromium/ChangeLog
Kent Tamura
Comment 17 2012-09-26 22:43:44 PDT
Comment on attachment 165930 [details] Patch 6 ok
yosin
Comment 18 2012-09-26 22:46:40 PDT
Comment on attachment 165930 [details] Patch 6 Clearing flags on attachment: 165930 Committed r129728: <http://trac.webkit.org/changeset/129728>
yosin
Comment 19 2012-09-26 22:46:45 PDT
All reviewed patches have been landed. Closing bug.
yosin
Comment 20 2012-09-28 03:20:49 PDT
See http://crbug.com/152632 for Chromium side changes.
Note You need to log in before you can comment on or make changes to this bug.