Bug 98109 – Adding Localizer::dateFormat() for multiple fields date/datetime input UI

• Component: Platform
• Status: RESOLVED FIXED

To implement multiple fields date/datetime input UI, we need to have Localizer::dateFormat().

Created attachment 166599 [details]
Patch 1

Created attachment 166601 [details]
Patch 2

Comment on attachment 166601 [details]
Patch 2

Could you review this patch?
Thanks in advance.

[Kent Tamura]

Comment on attachment 166601 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=166601&action=review

> Source/WebCore/platform/text/Localizer.cpp:193
> +    if (!m_localizedDateFormatText.isEmpty())
> +        return m_localizedDateFormatText;
> +    m_localizedDateFormatText = "dd/mm/yyyy";
> +    return m_localizedDateFormatText;

I think we had better return an empty string or make dateFormat pure virtual because this code won't be used.

> Source/WebCore/platform/text/Localizer.h:57
> +    // Returns time format in Unicode TR35 LDML[1] containing day of month,

time -> date

> Source/WebCore/platform/text/Localizer.h:120
> +    String m_localizedDateFormatText;

should be m_dateFormat if you need this member.

Created attachment 166605 [details]
Patch 3

Comment on attachment 166605 [details]
Patch 3

Could you review this patch?
Thanks in advance.

Note: This patch was compiled on Mac EWS, although failed on unrelated tests:

Unexpected flakiness: text-only failures (6)
  compositing/iframes/overlapped-nested-iframes.html [ Failure Pass ]
  fast/loader/javascript-url-in-object.html [ Failure Pass ]
  http/tests/cookies/single-quoted-value.html [ Failure Pass ]
  http/tests/security/cookies/third-party-cookie-blocking-user-action.html [ Failure Pass ]
  http/tests/security/sandboxed-iframe-origin-add.html [ Failure Pass ]
  media/media-controller-playback.html [ Failure Pass ]

Unexpected flakiness: timeouts (1)
  http/tests/webarchive/test-preload-resources.html [ Timeout Pass ]


Regressions: Unexpected text-only failures : (1)
  platform/mac/editing/spelling/removing-underline-after-accepting-autocorrection-using-punctuation.html [ Failure ]

[Kent Tamura]

Comment on attachment 166605 [details]
Patch 3

View in context: https://bugs.webkit.org/attachment.cgi?id=166605&action=review

> Source/WebCore/platform/text/Localizer.h:60
> +    // Returns date format in Unicode TR35 LDML[1] containing day of month,
> +    // month, and year, e.g. "dd/mm/yyyy"
> +    // [1] LDML http://unicode.org/reports/tr35/tr35-6.html#Date_Format_Patterns
> +    virtual String dateFormat() = 0;

Would you move this below localizedDecimalSeparator() please?
We'd like to categorize number-related functions and date/time related functions.

Also, http://unicode.org/reports/tr35/tr35-6.html is not the latest version of TR35.  Is it ok?

Created attachment 166619 [details]
Patch 4

Comment on attachment 166619 [details]
Patch 4

Clearing flags on attachment: 166619

Committed r130127: <http://trac.webkit.org/changeset/130127>

All reviewed patches have been landed.  Closing bug.

[Kent Tamura]

Comment on attachment 166619 [details]
Patch 4

View in context: https://bugs.webkit.org/attachment.cgi?id=166619&action=review

> Source/WebCore/platform/text/LocaleNone.cpp:81
> +    return ASCIILiteral("dd/mm/yyyyy");

should be dd/MM/yyyy, right?