Review options handling in InitializeDateTimeFormat

[Ms2ger]

The spec gets the timeZoneName property off options in the loop at step 33.b of InitializeDateTimeFormat(). In particular, it does not get the property if at least one of timeStyle and dateStyle is provided.

This also means that the [[TimeZoneName]] slot is never initialized, so resolvedOptions().timeZoneName will be undefined in this case.

However.

Both SpiderMonkey and V8 return "long", based on the script below. SpiderMonkey also gets the timeZoneName property and validates the result, but then discards it.

var dtf = new Intl.DateTimeFormat("en", {
  get timeStyle() {
    console.log("get timeStyle");
    return "full";
  },
  get timeZoneName() {
    console.log("get timeZoneName");
    return "short";
  },
});

console.log(dtf.resolvedOptions().timeStyle);
console.log(dtf.resolvedOptions().timeZoneName);

V8:

get timeStyle
full
long

SM:

get timeStyle
get timeZoneName
full
long

Expected:

get timeStyle
full
undefined

[littledan]

When writing the specification, my understanding was that timeZoneName should not be used. I'd be interested to hear why implementers did make use of it. cc @FrankYFTang @zbraniecki @anba

[FrankYFTang]

@Ms2ger I think your expectation is correct. I file a bug in https://bugs.chromium.org/p/v8/issues/detail?id=9107 and fixed that in https://chromium-review.googlesource.com/c/v8/v8/+/1559213 already If you try with v8, you do need pass with --harmony-intl-datetime-style flag. Here is the output from the v8 trunk

$ out/x64.release/d8 --harmony-intl-datetime-style
V8 version 7.5.0 (candidate)
d8> var dtf = new Intl.DateTimeFormat("en", { get timeStyle() { console.log("get timeStyle"); return "full"; }, get timeZoneName() { console.log("get timeZoneName"); return "short"; }, });
get timeStyle
undefined
d8> console.log(dtf.resolvedOptions().timeStyle);
full
undefined
d8> console.log(dtf.resolvedOptions().timeZoneName);
undefined
undefined

[littledan]

From the bug fix, it sounds like at least V8 is happy with the current specification. It seems like there's nothing to in this repository.

@Ms2ger Could you ensure there are good tests at the test262 level, and that there's a bug filed against SpiderMonkey for its issues here? cc @anba @zbraniecki @jswalden

[zbraniecki]

@Ms2ger have you ever filed the bug against SM and/or test262?

[zbraniecki]

In the current Chromium I see:

get timeStyle
full
undefined
undefined

In SM I see:

get timeZoneName
undefined
short
undefined

I'm not sure if this is actionable.

[zbraniecki]

@anba, @jswalden - do you have an opinion based on your work in https://bugzilla.mozilla.org/show_bug.cgi?id=1557718 ?

[Ms2ger]

CC @caiolima

[FrankYFTang]

@Ms2ger - any action I should take for v8? Is there a test262 test to test this issue?

[Ms2ger]

I don't know off hand. We'll get back to you if something needs to happen in V8.

[anba]

With https://bugzilla.mozilla.org/show_bug.cgi?id=1557718 applied, SM reports:

get timeStyle
get timeZoneName
get timeStyle
typein:1:11 TypeError: can't set option timeZoneName when timeStyle is used

(RangeError or TypeError is #46.)

[zbraniecki]

On the ECMA402 call @FrankYFTang states that this is indeed the intended behavior and he'll align V8 with it. We also should extend test coverage

[FrankYFTang]

I just land the v8 change to sync with the (Jun 10, 2020) spec in https://chromium-review.googlesource.com/c/v8/v8/+/2242258 . Not sure it is align with the diff between Jun 10 -12 changes yet.