[Intl] Fix RelativeTimeFormat formatToParts
The Intl.RelativeTimeFormat.prototype.formatToParts does not correctly implement the spec. Change the implementation by refactoring the JSNumber::FormatToParts and delegate part of the JSRelativeTimeFormat::FormatToParts to call the new refactored function. Bug: v8:8382 Change-Id: Ie153aa256ca78ce71c92efcdad55262564349ca9 Reviewed-on: https://chromium-review.googlesource.com/c/1305936 Commit-Queue: Frank Tang <ftang@chromium.org> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org> Cr-Commit-Position: refs/heads/master@{#57188}
This commit is contained in:
parent
6525976b1f
commit
91a5b3a73a
@ -639,23 +639,23 @@ std::vector<NumberFormatSpan> FlattenRegionsToParts(
|
||||
return out_parts;
|
||||
}
|
||||
|
||||
MaybeHandle<JSArray> JSNumberFormat::FormatToParts(
|
||||
Isolate* isolate, Handle<JSNumberFormat> number_format, double number) {
|
||||
Factory* factory = isolate->factory();
|
||||
icu::NumberFormat* fmt = number_format->icu_number_format()->raw();
|
||||
CHECK_NOT_NULL(fmt);
|
||||
|
||||
Maybe<int> JSNumberFormat::FormatToParts(Isolate* isolate,
|
||||
Handle<JSArray> result,
|
||||
int start_index,
|
||||
const icu::NumberFormat& fmt,
|
||||
double number, Handle<String> unit) {
|
||||
icu::UnicodeString formatted;
|
||||
icu::FieldPositionIterator fp_iter;
|
||||
UErrorCode status = U_ZERO_ERROR;
|
||||
fmt->format(number, formatted, &fp_iter, status);
|
||||
fmt.format(number, formatted, &fp_iter, status);
|
||||
if (U_FAILURE(status)) {
|
||||
THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kIcuError), JSArray);
|
||||
THROW_NEW_ERROR_RETURN_VALUE(
|
||||
isolate, NewTypeError(MessageTemplate::kIcuError), Nothing<int>());
|
||||
}
|
||||
|
||||
Handle<JSArray> result = factory->NewJSArray(0);
|
||||
int32_t length = formatted.length();
|
||||
if (length == 0) return result;
|
||||
int index = start_index;
|
||||
if (length == 0) return Just(index);
|
||||
|
||||
std::vector<NumberFormatSpan> regions;
|
||||
// Add a "literal" backdrop for the entire string. This will be used if no
|
||||
@ -674,7 +674,6 @@ MaybeHandle<JSArray> JSNumberFormat::FormatToParts(
|
||||
|
||||
std::vector<NumberFormatSpan> parts = FlattenRegionsToParts(®ions);
|
||||
|
||||
int index = 0;
|
||||
for (auto it = parts.begin(); it < parts.end(); it++) {
|
||||
NumberFormatSpan part = *it;
|
||||
Handle<String> field_type_string =
|
||||
@ -682,14 +681,33 @@ MaybeHandle<JSArray> JSNumberFormat::FormatToParts(
|
||||
? isolate->factory()->literal_string()
|
||||
: IcuNumberFieldIdToNumberType(part.field_id, number, isolate);
|
||||
Handle<String> substring;
|
||||
ASSIGN_RETURN_ON_EXCEPTION(
|
||||
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
|
||||
isolate, substring,
|
||||
Intl::ToString(isolate, formatted, part.begin_pos, part.end_pos),
|
||||
JSArray);
|
||||
Intl::AddElement(isolate, result, index, field_type_string, substring);
|
||||
Nothing<int>());
|
||||
if (unit.is_null()) {
|
||||
Intl::AddElement(isolate, result, index, field_type_string, substring);
|
||||
} else {
|
||||
Intl::AddElement(isolate, result, index, field_type_string, substring,
|
||||
isolate->factory()->unit_string(), unit);
|
||||
}
|
||||
++index;
|
||||
}
|
||||
JSObject::ValidateElements(*result);
|
||||
return Just(index);
|
||||
}
|
||||
|
||||
MaybeHandle<JSArray> JSNumberFormat::FormatToParts(
|
||||
Isolate* isolate, Handle<JSNumberFormat> number_format, double number) {
|
||||
Factory* factory = isolate->factory();
|
||||
icu::NumberFormat* fmt = number_format->icu_number_format()->raw();
|
||||
CHECK_NOT_NULL(fmt);
|
||||
|
||||
Handle<JSArray> result = factory->NewJSArray(0);
|
||||
|
||||
Maybe<int> maybe_format_to_parts = JSNumberFormat::FormatToParts(
|
||||
isolate, result, 0, *fmt, number, Handle<String>());
|
||||
MAYBE_RETURN(maybe_format_to_parts, Handle<JSArray>());
|
||||
|
||||
return result;
|
||||
}
|
||||
|
@ -46,6 +46,18 @@ class JSNumberFormat : public JSObject {
|
||||
V8_WARN_UNUSED_RESULT static MaybeHandle<JSArray> FormatToParts(
|
||||
Isolate* isolate, Handle<JSNumberFormat> number_format, double number);
|
||||
|
||||
// A utility function used by the above JSNumberFormat::FormatToParts()
|
||||
// and JSRelativeTimeFormat::FormatToParts().
|
||||
// Format the number by using the icu::NumberFormat to get the field
|
||||
// information. It add an object into the result array, starting from the
|
||||
// start_index and return the total number of elements in the result array.
|
||||
// For each object added as element, it set the substring of the field as
|
||||
// "value", the field type as "type". If the unit is not null, it also set
|
||||
// unit as "unit" to each added object.
|
||||
V8_WARN_UNUSED_RESULT static Maybe<int> FormatToParts(
|
||||
Isolate* isolate, Handle<JSArray> result, int start_index,
|
||||
const icu::NumberFormat& fmt, double number, Handle<String> unit);
|
||||
|
||||
V8_WARN_UNUSED_RESULT static MaybeHandle<String> FormatNumber(
|
||||
Isolate* isolate, Handle<JSNumberFormat> number_format, double number);
|
||||
|
||||
|
@ -16,6 +16,7 @@
|
||||
#include "src/isolate.h"
|
||||
#include "src/objects-inl.h"
|
||||
#include "src/objects/intl-objects.h"
|
||||
#include "src/objects/js-number-format.h"
|
||||
#include "src/objects/js-relative-time-format-inl.h"
|
||||
#include "unicode/datefmt.h"
|
||||
#include "unicode/numfmt.h"
|
||||
@ -244,7 +245,8 @@ Handle<String> UnitAsString(Isolate* isolate, URelativeDateTimeUnit unit_enum) {
|
||||
|
||||
MaybeHandle<JSArray> GenerateRelativeTimeFormatParts(
|
||||
Isolate* isolate, const icu::UnicodeString& formatted,
|
||||
const icu::UnicodeString& integer_part, URelativeDateTimeUnit unit_enum) {
|
||||
const icu::UnicodeString& integer_part, URelativeDateTimeUnit unit_enum,
|
||||
double number, const icu::NumberFormat& nf) {
|
||||
Factory* factory = isolate->factory();
|
||||
Handle<JSArray> array = factory->NewJSArray(0);
|
||||
int32_t found = formatted.indexOf(integer_part);
|
||||
@ -275,18 +277,12 @@ MaybeHandle<JSArray> GenerateRelativeTimeFormatParts(
|
||||
substring);
|
||||
}
|
||||
|
||||
// array.push({
|
||||
// 'type': 'integer',
|
||||
// 'value': formatted.substring(found, found + integer_part.length),
|
||||
// 'unit': unit})
|
||||
ASSIGN_RETURN_ON_EXCEPTION(isolate, substring,
|
||||
Intl::ToString(isolate, formatted, found,
|
||||
found + integer_part.length()),
|
||||
JSArray);
|
||||
Handle<String> unit = UnitAsString(isolate, unit_enum);
|
||||
Intl::AddElement(isolate, array, index++,
|
||||
factory->integer_string(), // field_type_string
|
||||
substring, factory->unit_string(), unit);
|
||||
|
||||
Maybe<int> maybe_format_to_parts =
|
||||
JSNumberFormat::FormatToParts(isolate, array, index, nf, number, unit);
|
||||
MAYBE_RETURN(maybe_format_to_parts, Handle<JSArray>());
|
||||
index = maybe_format_to_parts.FromJust();
|
||||
|
||||
// array.push({
|
||||
// 'type': 'literal',
|
||||
@ -402,19 +398,21 @@ MaybeHandle<Object> JSRelativeTimeFormat::Format(
|
||||
}
|
||||
|
||||
if (to_parts) {
|
||||
icu::UnicodeString integer;
|
||||
icu::UnicodeString number_str;
|
||||
icu::FieldPosition pos;
|
||||
formatter->getNumberFormat().format(std::abs(number), integer, pos, status);
|
||||
double abs_number = std::abs(number);
|
||||
formatter->getNumberFormat().format(abs_number, number_str, pos, status);
|
||||
if (U_FAILURE(status)) {
|
||||
THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kIcuError),
|
||||
Object);
|
||||
}
|
||||
|
||||
Handle<JSArray> elements;
|
||||
ASSIGN_RETURN_ON_EXCEPTION(
|
||||
isolate, elements,
|
||||
GenerateRelativeTimeFormatParts(isolate, formatted, integer, unit_enum),
|
||||
Object);
|
||||
ASSIGN_RETURN_ON_EXCEPTION(isolate, elements,
|
||||
GenerateRelativeTimeFormatParts(
|
||||
isolate, formatted, number_str, unit_enum,
|
||||
abs_number, formatter->getNumberFormat()),
|
||||
Object);
|
||||
return elements;
|
||||
}
|
||||
|
||||
|
@ -66,3 +66,44 @@ assertEquals('day', parts[0].unit);
|
||||
assertEquals(2, Object.getOwnPropertyNames(parts[1]).length);
|
||||
assertEquals('literal', parts[1].type);
|
||||
assertEquals(' days ago', parts[1].value);
|
||||
|
||||
// Test with non integer
|
||||
// Part Idx: 0 1 23 45 6
|
||||
assertEquals('in 123,456.78 seconds', longAlways.format(123456.78, 'seconds'));
|
||||
parts = longAlways.formatToParts(123456.78, 'seconds');
|
||||
assertEquals(7, parts.length);
|
||||
// 0: "in "
|
||||
assertEquals(2, Object.getOwnPropertyNames(parts[0]).length);
|
||||
assertEquals('literal', parts[0].type);
|
||||
assertEquals('in ', parts[0].value);
|
||||
assertEquals(undefined, parts[0].unit);
|
||||
// 1: "123"
|
||||
assertEquals(3, Object.getOwnPropertyNames(parts[1]).length);
|
||||
assertEquals('integer', parts[1].type);
|
||||
assertEquals('123', parts[1].value);
|
||||
assertEquals('second', parts[1].unit);
|
||||
// 2: ","
|
||||
assertEquals(3, Object.getOwnPropertyNames(parts[2]).length);
|
||||
assertEquals('group', parts[2].type);
|
||||
assertEquals(',', parts[2].value);
|
||||
assertEquals('second', parts[2].unit);
|
||||
// 3: "456"
|
||||
assertEquals(3, Object.getOwnPropertyNames(parts[3]).length);
|
||||
assertEquals('integer', parts[3].type);
|
||||
assertEquals('456', parts[3].value);
|
||||
assertEquals('second', parts[3].unit);
|
||||
// 4: "."
|
||||
assertEquals(3, Object.getOwnPropertyNames(parts[4]).length);
|
||||
assertEquals('decimal', parts[4].type);
|
||||
assertEquals('.', parts[4].value);
|
||||
assertEquals('second', parts[4].unit);
|
||||
// 5: "78"
|
||||
assertEquals(3, Object.getOwnPropertyNames(parts[4]).length);
|
||||
assertEquals('fraction', parts[5].type);
|
||||
assertEquals('78', parts[5].value);
|
||||
assertEquals('second', parts[5].unit);
|
||||
// 6: " seconds"
|
||||
assertEquals(2, Object.getOwnPropertyNames(parts[6]).length);
|
||||
assertEquals('literal', parts[6].type);
|
||||
assertEquals(' seconds', parts[6].value);
|
||||
assertEquals(undefined, parts[6].unit);
|
||||
|
@ -615,6 +615,14 @@
|
||||
# https://bugs.chromium.org/p/v8/issues/detail?id=7871
|
||||
'intl402/ListFormat/prototype/formatToParts/en-us-disjunction': [FAIL],
|
||||
|
||||
# https://bugs.chromium.org/p/v8/issues/detail?id=8382
|
||||
'intl402/RelativeTimeFormat/prototype/formatToParts/en-us-numeric-auto': [FAIL],
|
||||
'intl402/RelativeTimeFormat/prototype/formatToParts/en-us-numeric-always': [FAIL],
|
||||
'intl402/RelativeTimeFormat/prototype/formatToParts/en-us-style-short': [FAIL],
|
||||
'intl402/RelativeTimeFormat/prototype/formatToParts/pl-pl-style-short': [FAIL],
|
||||
'intl402/RelativeTimeFormat/prototype/formatToParts/pl-pl-style-long': [FAIL],
|
||||
'intl402/RelativeTimeFormat/prototype/formatToParts/pl-pl-style-narrow': [FAIL],
|
||||
|
||||
######################## NEEDS INVESTIGATION ###########################
|
||||
|
||||
# These test failures are specific to the intl402 suite and need investigation
|
||||
|
Loading…
Reference in New Issue
Block a user