Skip to content

Commit b7e108d

Browse files
FrankYFTangCommit Bot
FrankYFTang
authored and
Commit Bot
committed
[Intl] Use correct fallback values for options in Locale constructor
Fixes intl402/Locale/constructor-options-{casefirst,hourcycle,numeric}-invalid Bug: v8:7684 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: I43317f4bb1bb8422940faab1e5afa4162ed9ea11 Reviewed-on: https://chromium-review.googlesource.com/1137476 Reviewed-by: Sathya Gunasekaran <[email protected]> Commit-Queue: Frank Tang <[email protected]> Cr-Commit-Position: refs/heads/master@{#54504}
1 parent b102970 commit b7e108d

File tree

3 files changed

+39
-24
lines changed

3 files changed

+39
-24
lines changed

‎src/objects/js-locale.cc‎

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <map>
1212
#include <memory>
1313
#include <string>
14+
#include <vector>
1415

1516
#include "src/api.h"
1617
#include "src/global-handles.h"
@@ -32,39 +33,62 @@ namespace v8 {
3233
namespace internal {
3334

3435
namespace {
36+
37+
struct OptionData {
38+
const char* name;
39+
const char* key;
40+
const std::vector<const char*>* possible_values;
41+
bool is_bool_value;
42+
};
43+
3544
// Inserts tags from options into locale string.
3645
Maybe<bool> InsertOptionsIntoLocale(Isolate* isolate,
3746
Handle<JSReceiver> options,
3847
char* icu_locale) {
3948
CHECK(isolate);
4049
CHECK(icu_locale);
4150

42-
static const std::array<std::pair<const char*, const char*>, 6>
43-
kOptionToUnicodeTagMap = {{{"calendar", "ca"},
44-
{"collation", "co"},
45-
{"hourCycle", "hc"},
46-
{"caseFirst", "kf"},
47-
{"numeric", "kn"},
48-
{"numberingSystem", "nu"}}};
51+
static std::vector<const char*> hour_cycle_values = {"h11", "h12", "h23",
52+
"h24"};
53+
static std::vector<const char*> case_first_values = {"upper", "lower",
54+
"false"};
55+
static std::vector<const char*> empty_values = {};
56+
static const std::array<OptionData, 6> kOptionToUnicodeTagMap = {
57+
{{"calendar", "ca", &empty_values, false},
58+
{"collation", "co", &empty_values, false},
59+
{"hourCycle", "hc", &hour_cycle_values, false},
60+
{"caseFirst", "kf", &case_first_values, false},
61+
{"numeric", "kn", &empty_values, true},
62+
{"numberingSystem", "nu", &empty_values, false}}};
4963

5064
// TODO(cira): Pass in values as per the spec to make this to be
5165
// spec compliant.
52-
std::vector<const char*> values = {};
5366

5467
for (const auto& option_to_bcp47 : kOptionToUnicodeTagMap) {
5568
std::unique_ptr<char[]> value_str = nullptr;
56-
Maybe<bool> maybe_found = Intl::GetStringOption(
57-
isolate, options, option_to_bcp47.first, values, "locale", &value_str);
69+
bool value_bool = false;
70+
Maybe<bool> maybe_found =
71+
option_to_bcp47.is_bool_value
72+
? Intl::GetBoolOption(isolate, options, option_to_bcp47.name,
73+
"locale", &value_bool)
74+
: Intl::GetStringOption(isolate, options, option_to_bcp47.name,
75+
*(option_to_bcp47.possible_values),
76+
"locale", &value_str);
5877
if (maybe_found.IsNothing()) return maybe_found;
5978

6079
// TODO(cira): Use fallback value if value is not found to make
6180
// this spec compliant.
6281
if (!maybe_found.FromJust()) continue;
82+
83+
if (option_to_bcp47.is_bool_value) {
84+
value_str = value_bool ? isolate->factory()->true_string()->ToCString()
85+
: isolate->factory()->false_string()->ToCString();
86+
}
6387
DCHECK_NOT_NULL(value_str.get());
6488

6589
// Convert bcp47 key and value into legacy ICU format so we can use
6690
// uloc_setKeywordValue.
67-
const char* key = uloc_toLegacyKey(option_to_bcp47.second);
91+
const char* key = uloc_toLegacyKey(option_to_bcp47.key);
6892
DCHECK_NOT_NULL(key);
6993

7094
// Overwrite existing, or insert new key-value to the locale string.
@@ -113,17 +137,11 @@ bool PopulateLocaleWithUnicodeTags(Isolate* isolate, const char* icu_locale,
113137
if (bcp47_key) {
114138
const char* bcp47_value = uloc_toUnicodeLocaleType(bcp47_key, value);
115139
if (bcp47_value) {
116-
// It's either Boolean value.
117-
if (strncmp(bcp47_key, "kn", 2) == 0) {
118-
bool numeric = strcmp(bcp47_value, "true") == 0 ? true : false;
119-
Handle<Object> numeric_handle = factory->ToBoolean(numeric);
120-
locale_holder->set_numeric(*numeric_handle);
121-
continue;
122-
}
123-
// Or a string.
124140
Handle<String> bcp47_handle =
125141
factory->NewStringFromAsciiChecked(bcp47_value);
126-
if (strncmp(bcp47_key, "ca", 2) == 0) {
142+
if (strncmp(bcp47_key, "kn", 2) == 0) {
143+
locale_holder->set_numeric(*bcp47_handle);
144+
} else if (strncmp(bcp47_key, "ca", 2) == 0) {
127145
locale_holder->set_calendar(*bcp47_handle);
128146
} else if (strncmp(bcp47_key, "kf", 2) == 0) {
129147
locale_holder->set_case_first(*bcp47_handle);

‎test/intl/locale/locale-properties.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ assertEquals('buddhist', locale.calendar);
2424
assertEquals('phonebk', locale.collation);
2525
assertEquals('h23', locale.hourCycle);
2626
assertEquals('upper', locale.caseFirst);
27-
assertEquals(true, locale.numeric);
27+
assertEquals('true', locale.numeric);
2828
assertEquals('roman', locale.numberingSystem);
2929
// Not defined, expected to undefined.
3030
assertEquals(undefined, locale.currency);

‎test/test262/test262.status‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -448,12 +448,9 @@
448448
'intl402/Locale/constructor-getter-order': [FAIL],
449449
'intl402/Locale/constructor-locale-object': [FAIL],
450450
'intl402/Locale/constructor-non-iana-canon': [FAIL],
451-
'intl402/Locale/constructor-options-casefirst-invalid': [FAIL],
452-
'intl402/Locale/constructor-options-hourcycle-invalid': [FAIL],
453451
'intl402/Locale/constructor-options-language-grandfathered': [FAIL],
454452
'intl402/Locale/constructor-options-language-invalid': [FAIL],
455453
'intl402/Locale/constructor-options-language-valid': [FAIL],
456-
'intl402/Locale/constructor-options-numeric-valid': [FAIL],
457454
'intl402/Locale/constructor-options-region-invalid': [FAIL],
458455
'intl402/Locale/constructor-options-region-valid': [FAIL],
459456
'intl402/Locale/constructor-options-script-invalid': [FAIL],

0 commit comments

Comments
 (0)