diff --git a/src/objects/js-number-format.cc b/src/objects/js-number-format.cc index 6f2e081af0..3dc195cedd 100644 --- a/src/objects/js-number-format.cc +++ b/src/objects/js-number-format.cc @@ -639,23 +639,23 @@ std::vector FlattenRegionsToParts( return out_parts; } -MaybeHandle JSNumberFormat::FormatToParts( - Isolate* isolate, Handle number_format, double number) { - Factory* factory = isolate->factory(); - icu::NumberFormat* fmt = number_format->icu_number_format()->raw(); - CHECK_NOT_NULL(fmt); - +Maybe JSNumberFormat::FormatToParts(Isolate* isolate, + Handle result, + int start_index, + const icu::NumberFormat& fmt, + double number, Handle 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()); } - Handle 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 regions; // Add a "literal" backdrop for the entire string. This will be used if no @@ -674,7 +674,6 @@ MaybeHandle JSNumberFormat::FormatToParts( std::vector parts = FlattenRegionsToParts(®ions); - int index = 0; for (auto it = parts.begin(); it < parts.end(); it++) { NumberFormatSpan part = *it; Handle field_type_string = @@ -682,14 +681,33 @@ MaybeHandle JSNumberFormat::FormatToParts( ? isolate->factory()->literal_string() : IcuNumberFieldIdToNumberType(part.field_id, number, isolate); Handle 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()); + 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 JSNumberFormat::FormatToParts( + Isolate* isolate, Handle number_format, double number) { + Factory* factory = isolate->factory(); + icu::NumberFormat* fmt = number_format->icu_number_format()->raw(); + CHECK_NOT_NULL(fmt); + + Handle result = factory->NewJSArray(0); + + Maybe maybe_format_to_parts = JSNumberFormat::FormatToParts( + isolate, result, 0, *fmt, number, Handle()); + MAYBE_RETURN(maybe_format_to_parts, Handle()); return result; } diff --git a/src/objects/js-number-format.h b/src/objects/js-number-format.h index f0f7a8dca1..997bd3da85 100644 --- a/src/objects/js-number-format.h +++ b/src/objects/js-number-format.h @@ -46,6 +46,18 @@ class JSNumberFormat : public JSObject { V8_WARN_UNUSED_RESULT static MaybeHandle FormatToParts( Isolate* isolate, Handle 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 FormatToParts( + Isolate* isolate, Handle result, int start_index, + const icu::NumberFormat& fmt, double number, Handle unit); + V8_WARN_UNUSED_RESULT static MaybeHandle FormatNumber( Isolate* isolate, Handle number_format, double number); diff --git a/src/objects/js-relative-time-format.cc b/src/objects/js-relative-time-format.cc index fd2a4dbe8b..d6dc591ef8 100644 --- a/src/objects/js-relative-time-format.cc +++ b/src/objects/js-relative-time-format.cc @@ -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 UnitAsString(Isolate* isolate, URelativeDateTimeUnit unit_enum) { MaybeHandle 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 array = factory->NewJSArray(0); int32_t found = formatted.indexOf(integer_part); @@ -275,18 +277,12 @@ MaybeHandle 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 unit = UnitAsString(isolate, unit_enum); - Intl::AddElement(isolate, array, index++, - factory->integer_string(), // field_type_string - substring, factory->unit_string(), unit); + + Maybe maybe_format_to_parts = + JSNumberFormat::FormatToParts(isolate, array, index, nf, number, unit); + MAYBE_RETURN(maybe_format_to_parts, Handle()); + index = maybe_format_to_parts.FromJust(); // array.push({ // 'type': 'literal', @@ -402,19 +398,21 @@ MaybeHandle 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 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; } diff --git a/test/intl/relative-time-format/format-to-parts-en.js b/test/intl/relative-time-format/format-to-parts-en.js index 52a0b885d7..689059f4cd 100644 --- a/test/intl/relative-time-format/format-to-parts-en.js +++ b/test/intl/relative-time-format/format-to-parts-en.js @@ -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); diff --git a/test/test262/test262.status b/test/test262/test262.status index c1a270360c..3717047a29 100644 --- a/test/test262/test262.status +++ b/test/test262/test262.status @@ -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