Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polyfill: Hour cycle not correctly set when defaults are used #3065

Open
anba opened this issue Dec 13, 2024 · 2 comments
Open

Polyfill: Hour cycle not correctly set when defaults are used #3065

anba opened this issue Dec 13, 2024 · 2 comments
Labels
non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! test262

Comments

@anba
Copy link
Contributor

anba commented Dec 13, 2024

Test case:

console.log(new Temporal.PlainDateTime(2020, 1, 1).toLocaleString("en", {hour12: false}));
console.log(new Temporal.PlainDateTime(2020, 1, 1).toLocaleString("en", {hour12: true}));

console.log(new Temporal.PlainDateTime(2020, 1, 1).toLocaleString("en", {hourCycle: "h23"}));
console.log(new Temporal.PlainDateTime(2020, 1, 1).toLocaleString("en", {hourCycle: "h24"}));
console.log(new Temporal.PlainDateTime(2020, 1, 1).toLocaleString("en", {hourCycle: "h11"}));
console.log(new Temporal.PlainDateTime(2020, 1, 1).toLocaleString("en", {hourCycle: "h12"}));

Actual:

1/1/2020, 12:00:00 AM
1/1/2020, 12:00:00 AM
1/1/2020, 12:00:00 AM
1/1/2020, 12:00:00 AM
1/1/2020, 12:00:00 AM
1/1/2020, 12:00:00 AM

Expected:

1/1/2020, 00:00:00
1/1/2020, 12:00:00 AM
1/1/2020, 00:00:00
1/1/2020, 24:00:00
1/1/2020, 0:00:00 AM
1/1/2020, 12:00:00 AM
@ptomato ptomato added non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! test262 labels Feb 13, 2025
@ptomato
Copy link
Collaborator

ptomato commented Feb 14, 2025

Thanks for pointing this out. I guess it's locale-dependent whether the first line of the expected results should have 00:00:00 or 24:00:00?

I'll submit a PR to make the reference code conform to the spec. However, clearly this was a gap in test262 coverage since it wasn't caught before, so I've added the test262 label to this issue and will keep it open. We should have coverage for PlainTime, PlainDateTime, Instant, and ZonedDateTime formatting.

@anba
Copy link
Contributor Author

anba commented Feb 14, 2025

I guess it's locale-dependent whether the first line of the expected results should have 00:00:00 or 24:00:00?

24:00:00 only if a locale had hc24 as its default hour cycle, which no locale has → https://unicode-org.atlassian.net/browse/CLDR-18303.

The second line, for hour12: true, can have either 12:00:00 (all locales except Japanese) or 0:00:00 (Japanese).

ptomato added a commit that referenced this issue Feb 15, 2025
The strategy of caching the output of DTF resolvedOptions() failed in the
case of hour12/hourCycle, because those options are silently dropped if
the chosen format does not include an hour component. Manually add them to
the cached resolvedOptions object if the user included them.

See: #3065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! test262
Projects
None yet
Development

No branches or pull requests

2 participants