1284868 - Intl.DateTimeFormat issue with "2-digit" option on hour, minute and second

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, defect, P5)

Description

[Peter Smith]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36

Steps to reproduce:

var options = { second: "2-digit", hour12: false };
var dateTimeFormat = new Intl.DateTimeFormat('en-US', options);
document.write(dateTimeFormat.format(new Date('2016/06/06 15:05:07')));

https://jsfiddle.net/jtdthuys/


Actual results:

output was "7"


Expected results:

should have output "07"

Comment 1

[Tooru Fujisawa [:arai]]

you can check the resolved options for the specified options, with resolvedOptions method.

  var options = {
      second: "2-digit",
      hour12: false
  };
  var dateTimeFormat = new Intl.DateTimeFormat('en-US', options);
  console.log(JSON.stringify(dateTimeFormat.resolvedOptions()));

and it shows the following for me

  {"locale":"en-US","calendar":"gregory","numberingSystem":"latn","second":"numeric"}

that means, formatting only "second" with "2-digit" is not supported.
and, iiuc, that format is not required by the spec [1].

[1] http://www.ecma-international.org/ecma-402/3.0/index.html#sec-intl.datetimeformat-internal-slots

Comment 2

[Jeff Walden [:Waldo]]

Yeah, strictly speaking our behavior is spec-compliant.  It's a reasonable quality-of-implementation request, tho -- but I'm not sure when we'll get to it, exactly, given feature work to do.  (And that the call for displaying two-digit seconds, standalone, seems likely to be fairly low compared to most other quality-of-implementation things we might want to do.)  But who knows, maybe ICU will fix this upstream and we'll just get it for free sometime.

Comment 3

[André Bargull [:anba]]

ICU provides an option (UDATPG_MATCH_ALL_FIELDS_LENGTH [1,2]) to enforce that every field in the resolved pattern has the same length as in the input skeleton. We'd need to test if it makes sense to enable this option for every input (that's probably not the case), but for simple skeletons like "ss" we should be able to use it.

[1] http://icu-project.org/apiref/icu4c/udatpg_8h.html#ab3c76ea5c08e4acb44e0ddc66c5831e8
[2] http://icu-project.org/apiref/icu4c/udatpg_8h.html#a5327a64b164f975dc0636e36b4d0f02c

Comment 5

[arman.128]

The problem is still here and it's kinda sucks.

Comment 6

[Shane Carr]

This got reported upstream in this ECMAScript issue: https://github.com/tc39/ecma402/issues/283

I filed a ticket against CLDR. Peter Edberg quickly replied with:

DateTimePatternGenerator has a version of getBestPattern that takes options:

UnicodeString icu::DateTimePatternGenerator::getBestPattern (const
UnicodeString &skeleton,
UDateTimePatternMatchOptions options, UErrorCode & status);

and one of those options is

UDATPG_MATCH_HOUR_FIELD_LENGTH

Frank Tang added that flag to Chromium and was able to fix the issue:

https://bugs.chromium.org/p/chromium/issues/detail?id=527926#c14

Perhaps the same fix can be done in SpiderMonkey.

Comment 7

[André Bargull [:anba]]

(In reply to Shane Carr from comment #6)

Perhaps the same fix can be done in SpiderMonkey.

Comment #0 is similar, but not the same as https://github.com/tc39/ecma402/issues/283 ("second" vs. "hour" field). ICU also has UDATPG_MATCH_ALL_FIELDS_LENGTH, but I don't know if just enabling it is the correct choice for all possible skeleton combinations (comment #3).

Comment 8

[Shane Carr]

Is there an example where UDATPG_MATCH_ALL_FIELDS_LENGTH doesn't do the right thing?

Does answering that question just require a careful reading of the spec?

Comment 9

[André Bargull [:anba]]

I haven't tried it yet, so I can't really tell. In the worst case, every possible field combination in Intl.DateTimeFormat for all supported locales needs to be compared when UDATPG_MATCH_ALL_FIELDS_LENGTH is used resp. not used, and then the output needs to be checked if it's still valid for each locale to avoid returning bogus results.

Comment 10

[André Bargull [:anba]]

After reading the relevant ICU source code, I come to the conclusion that always using UDATPG_MATCH_ALL_FIELDS_LENGTH is okay to do and should also match the CLDR "automatic expansion" requirements from https://unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems.

Basically the default behaviour when calling udatpg_getBestPattern resp. udatpg_getBestPatternWithOptions with UDATPG_MATCH_NO_OPTIONS is to adjust all fields except for "hour", "minute", and "second". And only when UDATPG_MATCH_ALL_FIELDS_LENGTH is used, the "hour", "minute", and "second" fields also get the "automatic expansion" feature described in CLDR.

---

Example for "automatic expansion" (basically repeating the example from CLDR):

common/main/en.xml has this entry:

<dateFormatItem id="yMMMd">MMM d, y</dateFormatItem>

There is no "yMMMMd" entry, so a request for the skeleton "yMMMMd" returns the best-match result "yMMMd" with its pattern "MMM d, y". This best-match pattern is then expanded to "MMMM d, y" and we get the expected result:

js> new Intl.DateTimeFormat("en", {year: "numeric", month: "long", day: "numeric"}).format(new Date("2019-08-06T00:00:00"))     
"August 6, 2019"

And using the shell we can also verify the expected pattern is computed:

js> getSelfHostedValue("intl_patternForSkeleton")("en", "yMMMMd")
"MMMM d, y"

Comment 11

[André Bargull [:anba]]

And to address my concerns at comment #3, comment #7, and comment #9:
I was concerned UDATPG_MATCH_ALL_FIELDS_LENGTH means invalid length adjustments are performed, similar to the example in https://unicode.org/reports/tr35/tr35-dates.html#Matching_Skeletons where it's made clear that the pattern "y年M月" should not be expanded to "y年MMMM月" when the input skeleton was "yMMMM".

Comment 12

[André Bargull [:anba]]

Hmm, I don't think we can make this change unless we also request a spec change. When UDATPG_MATCH_ALL_FIELDS_LENGTH is used, the following test case uses "numeric" representation for "minute" and "second", but I guess most would expect to see "2-digit" there.

js> new Date("2019-08-01T01:02:03").toLocaleString("en", {timeZoneName: "long"}) 
"8/1/2019, 1:2:3 AM Pacific Daylight Time"

This is caused by ToDateTimeOptions defaulting "minute" and "second" to "numeric" in step 7.

Comment 13

[André Bargull [:anba]]

Maybe that's just an ICU bug? The example in comment #12 should use the skeleton "jmszzzz", which, when replacing "j" with the preferred hour symbol "h", should result in finding the following pattern from en.xml:

<dateFormatItem id="hmsv">h:mm:ss a v</dateFormatItem>

The "hms" in "hmsv" matches the fields in the input skeleton "hmszzzz", so only "v" should get automatic expansion to "zzzz". Therefore I'd expect the final pattern to be "h:mm:ss a zzzz". My first guess is that the "v" and "z" mismatch is causing this issue:

js> getSelfHostedValue("intl_patternForSkeleton")("en", "hmsv")
"h:mm:ss a v"
js> getSelfHostedValue("intl_patternForSkeleton")("en", "hmsvvvv")
"h:mm:ss a vvvv"
js> getSelfHostedValue("intl_patternForSkeleton")("en", "hmszzzz")
"h:m:s a zzzz"

Comment 14

[André Bargull [:anba]]

Yes, it is an ICU bug resp. limitation. When DateTimePatternGenerator::adjustFieldTypes adjust the field lengths, it normally takes the original input skeleton into account, except for the case when no skeleton is present. A skeleton may not be present when the pattern was added through DateTimePatternGenerator::addICUPatterns, because that function ends up calling addPatternWithSkeleton with skeletonToUse = nullptr.

One of the ICU patterns added in addICUPatterns is "h:mm:ss a zzzz", which is the one used when requesting "hmszzzz". So we request "hmszzzz" and ICU finds "h:mm:ss a zzzz" as the best match. But "h:mm:ss a zzzz" has no skeleton attached to it, so adjustFieldTypes can't compare the input skeleton with the skeleton of the best-match pattern and instead will adjust all fields, resulting in the final pattern "h:m:s a zzzz".

Comment 15

[André Bargull [:anba]]

Attachment: Bug 1284868: Adjust field widths for hour/minute/second when the user requested 2-digit representation. r=dminor!

Change DateTimePatternGenerator::GetBestPattern() to call udatpg_getBestPatternWithOptions(),
so we can adjust the computed pattern to use 2-digit representation when it was requested by the
user.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be9084db6739
Adjust field widths for hour/minute/second when the user requested 2-digit representation. r=dminor

Comment 18

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/be9084db6739