New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 6
Cc:
Components:
HW: All
NextAction: ----
OS: All
Priority: 2
Type: Bug



Sign in to add a comment
link

Issue 7482: [Intl] Don't add hour-cycle to resolved locale if hour12 option is present

Reported by gsat...@chromium.org, Feb 22 2018 Project Member

Issue description

Implement https://github.com/tc39/ecma402/pull/204

See failing test262 test:
'intl402/DateTimeFormat/prototype/resolvedOptions/resolved-locale-with-hc-unicode'
 

Comment 1 by js...@chromium.org, Apr 29 2018

esid: sec-Intl.DateTimeFormat.prototype.resolvedOptions
description: >
  The resolved locale doesn't include a hc Unicode extension value if the
  hour12 or hourCycle option is also present.
info: |
  12.1.1 InitializeDateTimeFormat(dateTimeFormat, locales, options)
    ...
    6. Let hour12 be ? GetOption(options, "hour12", "boolean", undefined, undefined).
    7. Let hourCycle be ? GetOption(options, "hourCycle", "string", « "h11", "h12", "h23", "h24" », undefined).
    8. If hour12 is not undefined, then
      a. Let hourCycle be null.
    9. Set opt.[[hc]] to hourCycle.
    ...

  9.2.6 ResolveLocale(availableLocales, requestedLocales, options, relevantExtensionKeys, localeData)
    ...
    8. For each element key of relevantExtensionKeys in List order, do
      ...
      i. If options has a field [[<key>]], then
        i. Let optionsValue be options.[[<key>]].
        ii. Assert: Type(optionsValue) is either String, Undefined, or Null.
        iii. If keyLocaleData contains optionsValue, then
          1. If SameValue(optionsValue, value) is false, then
            a. Let value be optionsValue.
            b. Let supportedExtensionAddition be "".
      ...
---*/

Comment 2 by ftang@chromium.org, Jul 13 2018

Comment 3 by ftang@chromium.org, Jul 13 2018

Cc: bstell@chromium.org js...@chromium.org

Comment 4 by ftang@chromium.org, Oct 12

good progress on https://chromium-review.googlesource.com/c/v8/v8/+/1253101 need to clean up the cl more before asking for review.

Comment 5 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/44771cbe0c7d82ae33e662f70129e28db31898e7

commit 44771cbe0c7d82ae33e662f70129e28db31898e7
Author: Frank Tang <ftang@chromium.org>
Date: Fri Dec 14 01:20:42 2018

[Intl] Fixes Intl.DateTimeFormat hourCycle option issues.

Add bit flags to remember hourCycle
Reorder the code in JSDateTimeFormat::Initialize
Implement the hourCycle option resolutions
Fix intl402/DateTimeFormat/prototype/resolvedOptions/hourCycle in test262

Bug:  v8:7482 
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Idc136276da89b95df6ae864161b114e34f9dcae8
Reviewed-on: https://chromium-review.googlesource.com/c/1253101
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58233}
[modify] https://crrev.com/44771cbe0c7d82ae33e662f70129e28db31898e7/src/objects-debug.cc
[modify] https://crrev.com/44771cbe0c7d82ae33e662f70129e28db31898e7/src/objects-printer.cc
[modify] https://crrev.com/44771cbe0c7d82ae33e662f70129e28db31898e7/src/objects/intl-objects.cc
[modify] https://crrev.com/44771cbe0c7d82ae33e662f70129e28db31898e7/src/objects/intl-objects.h
[modify] https://crrev.com/44771cbe0c7d82ae33e662f70129e28db31898e7/src/objects/js-date-time-format-inl.h
[modify] https://crrev.com/44771cbe0c7d82ae33e662f70129e28db31898e7/src/objects/js-date-time-format.cc
[modify] https://crrev.com/44771cbe0c7d82ae33e662f70129e28db31898e7/src/objects/js-date-time-format.h
[modify] https://crrev.com/44771cbe0c7d82ae33e662f70129e28db31898e7/test/test262/test262.status

Comment 6 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/33408e6e3dcd9c7653e82a5c032c10d581b09d90

commit 33408e6e3dcd9c7653e82a5c032c10d581b09d90
Author: Frank Tang <ftang@chromium.org>
Date: Fri Dec 14 23:39:50 2018

[Intl] Add date-format/check-hc-option.js

Bug:  v8:7482 
Change-Id: Icd3383e350555907c01ec65d2620477973117ea3
Reviewed-on: https://chromium-review.googlesource.com/c/1368864
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58258}
[add] https://crrev.com/33408e6e3dcd9c7653e82a5c032c10d581b09d90/test/intl/date-format/check-hc-option.js

Comment 7 by ftang@chromium.org, Jan 17

Cc: littledan@chromium.org
It turn out not only our impl has some problem (We DO)
The test in intl402/DateTimeFormat/prototype/resolvedOptions/resolved-locale-with-hc-unicode also is buggy.
see
https://github.com/tc39/test262/pull/2035

Comment 9 by ftang@chromium.org, Jan 22

Summary: [Intl] Don't add hour-cycle to resolved locale if hour12 option is present (was: Don't add hour-cycle to resolved locale if hour12 option is present)

Comment 10 by bugdroid, Jan 25

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/75f8f2f85e61ce63a303ef0c11ff53f2ff3a0998

commit 75f8f2f85e61ce63a303ef0c11ff53f2ff3a0998
Author: Frank Tang <ftang@chromium.org>
Date: Fri Jan 25 02:56:36 2019

[Intl] Fix resolved-locale-with-hc-unicode

Remove hc from -u- if does not agree with the resolved one.


Bug:  v8:7482 
Change-Id: I635c5357b8fd2b630ed80577a9b6a116e9a0e3f4
Reviewed-on: https://chromium-review.googlesource.com/c/1417170
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59084}
[modify] https://crrev.com/75f8f2f85e61ce63a303ef0c11ff53f2ff3a0998/src/objects/js-date-time-format.cc
[modify] https://crrev.com/75f8f2f85e61ce63a303ef0c11ff53f2ff3a0998/test/intl/date-format/check-hc-option.js
[modify] https://crrev.com/75f8f2f85e61ce63a303ef0c11ff53f2ff3a0998/test/test262/test262.status

Comment 11 by ftang@chromium.org, Feb 6

Status: Fixed (was: Assigned)

Sign in to add a comment