Bug 1610514 - Part 3: Pass through a boolean flag to detect style=unit number formatters. r=jwalden
authorAndré Bargull <andre.bargull@gmail.com>
Thu, 30 Jan 2020 09:47:48 +0000
changeset 512115 236b8297fd5d66bebcb478e4dffe5ad7d2376a4b
parent 512114 177fd2d58046b31fd9d212446f0ed6252fa398d0
child 512116 784fdbec47d2351b211f1fceae1d68974ea21798
push id37072
push usercsabou@mozilla.com
push dateThu, 30 Jan 2020 15:44:43 +0000
treeherdermozilla-central@f97c48da9cee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1610514
milestone74.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1610514 - Part 3: Pass through a boolean flag to detect style=unit number formatters. r=jwalden Differential Revision: https://phabricator.services.mozilla.com/D60969
js/src/builtin/BigInt.js
js/src/builtin/Number.js
js/src/builtin/intl/NumberFormat.cpp
js/src/builtin/intl/NumberFormat.h
js/src/builtin/intl/NumberFormat.js
js/src/tests/jstests.list
js/src/tests/non262/Intl/NumberFormat/unit-formatToParts-has-unit-field.js
--- a/js/src/builtin/BigInt.js
+++ b/js/src/builtin/BigInt.js
@@ -26,11 +26,11 @@ function BigInt_toLocaleString() {
             numberFormatCache.runtimeDefaultLocale = RuntimeDefaultLocale();
         }
         numberFormat = numberFormatCache.numberFormat;
     } else {
         numberFormat = intl_NumberFormat(locales, options);
     }
 
     // Step 3.
-    return intl_FormatNumber(numberFormat, x, /* formatToParts = */ false);
+    return intl_FormatNumber(numberFormat, x, /* formatToParts = */ false, /* unitStyle = */ false);
 }
 #endif  // JS_HAS_INTL_API
--- a/js/src/builtin/Number.js
+++ b/js/src/builtin/Number.js
@@ -30,17 +30,17 @@ function Number_toLocaleString() {
             numberFormatCache.runtimeDefaultLocale = RuntimeDefaultLocale();
         }
         numberFormat = numberFormatCache.numberFormat;
     } else {
         numberFormat = intl_NumberFormat(locales, options);
     }
 
     // Step 5.
-    return intl_FormatNumber(numberFormat, x, /* formatToParts = */ false);
+    return intl_FormatNumber(numberFormat, x, /* formatToParts = */ false, /* unitStyle = */ false);
 }
 #endif  // JS_HAS_INTL_API
 
 // ES6 draft ES6 20.1.2.4
 function Number_isFinite(num) {
     if (typeof num !== "number")
         return false;
     return num - num === 0;
--- a/js/src/builtin/intl/NumberFormat.cpp
+++ b/js/src/builtin/intl/NumberFormat.cpp
@@ -1000,18 +1000,21 @@ static bool FormatNumeric(JSContext* cx,
   if (!str) {
     return false;
   }
 
   result.setString(str);
   return true;
 }
 
+enum class FormattingType { ForUnit, NotForUnit };
+
 static FieldType GetFieldTypeForNumberField(UNumberFormatFields fieldName,
-                                            HandleValue x) {
+                                            HandleValue x,
+                                            FormattingType formattingType) {
   // See intl/icu/source/i18n/unicode/unum.h for a detailed field list.  This
   // list is deliberately exhaustive: cases might have to be added/removed if
   // this code is compiled with a different ICU with more UNumberFormatFields
   // enum initializers.  Please guard such cases with appropriate ICU
   // version-testing #ifdefs, should cross-version divergence occur.
   switch (fieldName) {
     case UNUM_INTEGER_FIELD:
       if (x.isNumber()) {
@@ -1039,16 +1042,21 @@ static FieldType GetFieldTypeForNumberFi
       // positive in our implementation.
       bool isNegative = x.isNumber()
                             ? !IsNaN(x.toNumber()) && IsNegative(x.toNumber())
                             : x.toBigInt()->isNegative();
       return isNegative ? &JSAtomState::minusSign : &JSAtomState::plusSign;
     }
 
     case UNUM_PERCENT_FIELD:
+      // Percent fields are returned as "unit" elements when the number
+      // formatter's style is "unit".
+      if (formattingType == FormattingType::ForUnit) {
+        return &JSAtomState::unit;
+      }
       return &JSAtomState::percentSign;
 
     case UNUM_CURRENCY_FIELD:
       return &JSAtomState::currency;
 
     case UNUM_PERMILL_FIELD:
       MOZ_ASSERT_UNREACHABLE(
           "unexpected permill field found, even though "
@@ -1413,16 +1421,17 @@ ArrayObject* NumberFormatFields::toArray
   return partsArray;
 }
 
 #ifndef U_HIDE_DRAFT_API
 static bool FormattedNumberToParts(JSContext* cx,
                                    const UFormattedValue* formattedValue,
                                    HandleValue number,
                                    FieldType relativeTimeUnit,
+                                   FormattingType formattingType,
                                    MutableHandleValue result) {
   MOZ_ASSERT(number.isNumeric());
 
   RootedString overallResult(cx, FormattedNumberToString(cx, formattedValue));
   if (!overallResult) {
     return false;
   }
 
@@ -1463,18 +1472,18 @@ static bool FormattedNumberToParts(JSCon
 
     int32_t beginIndex, endIndex;
     ucfpos_getIndexes(fpos, &beginIndex, &endIndex, &status);
     if (U_FAILURE(status)) {
       intl::ReportInternalError(cx);
       return false;
     }
 
-    FieldType type =
-        GetFieldTypeForNumberField(UNumberFormatFields(field), number);
+    FieldType type = GetFieldTypeForNumberField(UNumberFormatFields(field),
+                                                number, formattingType);
 
     if (!fields.append(type, beginIndex, endIndex)) {
       return false;
     }
   }
 
   ArrayObject* array = fields.toArray(cx, overallResult, relativeTimeUnit);
   if (!array) {
@@ -1484,19 +1493,19 @@ static bool FormattedNumberToParts(JSCon
   result.setObject(*array);
   return true;
 }
 
 bool js::intl::FormattedRelativeTimeToParts(
     JSContext* cx, const UFormattedValue* formattedValue, double timeValue,
     FieldType relativeTimeUnit, MutableHandleValue result) {
   Value tval = DoubleValue(timeValue);
-  return FormattedNumberToParts(cx, formattedValue,
-                                HandleValue::fromMarkedLocation(&tval),
-                                relativeTimeUnit, result);
+  return FormattedNumberToParts(
+      cx, formattedValue, HandleValue::fromMarkedLocation(&tval),
+      relativeTimeUnit, FormattingType::NotForUnit, result);
 }
 #else
 static ArrayObject* LegacyFormattedNumberToParts(
     JSContext* cx, const UFormattedNumber* formatted, HandleValue x,
     MutableHandleValue result) {
   RootedString overallResult(cx, FormattedNumberToString(cx, formatted));
   if (!overallResult) {
     return false;
@@ -1537,36 +1546,39 @@ static ArrayObject* LegacyFormattedNumbe
 
   result.setObject(*array);
   return true;
 }
 #endif
 
 static bool FormatNumericToParts(JSContext* cx, const UNumberFormatter* nf,
                                  UFormattedNumber* formatted, HandleValue x,
+                                 FormattingType formattingType,
                                  MutableHandleValue result) {
   PartitionNumberPatternResult formattedValue =
       PartitionNumberPattern(cx, nf, formatted, x);
   if (!formattedValue) {
     return false;
   }
 
 #ifndef U_HIDE_DRAFT_API
-  return FormattedNumberToParts(cx, formattedValue, x, nullptr, result);
+  return FormattedNumberToParts(cx, formattedValue, x, nullptr, formattingType,
+                                result);
 #else
   return LegacyFormattedNumberToParts(cx, formattedValue, x, result);
 #endif
 }
 
 bool js::intl_FormatNumber(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
-  MOZ_ASSERT(args.length() == 3);
+  MOZ_ASSERT(args.length() == 4);
   MOZ_ASSERT(args[0].isObject());
   MOZ_ASSERT(args[1].isNumeric());
   MOZ_ASSERT(args[2].isBoolean());
+  MOZ_ASSERT(args[3].isBoolean());
 
   Rooted<NumberFormatObject*> numberFormat(
       cx, &args[0].toObject().as<NumberFormatObject>());
 
   // Obtain a cached UNumberFormatter object.
   UNumberFormatter* nf = numberFormat->getNumberFormatter();
   if (!nf) {
     nf = NewUNumberFormatter(cx, numberFormat);
@@ -1588,13 +1600,17 @@ bool js::intl_FormatNumber(JSContext* cx
     }
     numberFormat->setFormattedNumber(formatted);
 
     // UFormattedNumber memory tracked as part of UNumberFormatter.
   }
 
   // Use the UNumberFormatter to actually format the number.
   if (args[2].toBoolean()) {
-    return FormatNumericToParts(cx, nf, formatted, args[1], args.rval());
+    FormattingType formattingType = args[3].toBoolean()
+                                        ? FormattingType::ForUnit
+                                        : FormattingType::NotForUnit;
+    return FormatNumericToParts(cx, nf, formatted, args[1], formattingType,
+                                args.rval());
   }
 
   return FormatNumeric(cx, nf, formatted, args[1], args.rval());
 }
--- a/js/src/builtin/intl/NumberFormat.h
+++ b/js/src/builtin/intl/NumberFormat.h
@@ -89,17 +89,18 @@ extern MOZ_MUST_USE bool intl_numberingS
                                               Value* vp);
 
 /**
  * Returns a string representing the number x according to the effective
  * locale and the formatting options of the given NumberFormat.
  *
  * Spec: ECMAScript Internationalization API Specification, 11.3.2.
  *
- * Usage: formatted = intl_FormatNumber(numberFormat, x, formatToParts)
+ * Usage: formatted = intl_FormatNumber(numberFormat, x, formatToParts,
+ *                                      unitStyle)
  */
 extern MOZ_MUST_USE bool intl_FormatNumber(JSContext* cx, unsigned argc,
                                            Value* vp);
 
 #if DEBUG || MOZ_SYSTEM_ICU
 /**
  * Returns an object with all available measurement units.
  *
--- a/js/src/builtin/intl/NumberFormat.js
+++ b/js/src/builtin/intl/NumberFormat.js
@@ -693,17 +693,17 @@ function createNumberFormatFormat(nf) {
         // Step 2.
         assert(IsObject(nf), "InitializeNumberFormat called with non-object");
         assert(GuardToNumberFormat(nf) !== null, "InitializeNumberFormat called with non-NumberFormat");
 
         // Steps 3-4.
         var x = ToNumeric(value);
 
         // Step 5.
-        return intl_FormatNumber(nf, x, /* formatToParts = */ false);
+        return intl_FormatNumber(nf, x, /* formatToParts = */ false, /* unitStyle = */ false);
     };
 }
 
 /**
  * Returns a function bound to this NumberFormat that returns a String value
  * representing the result of calling ToNumber(value) according to the
  * effective locale and the formatting options of this NumberFormat.
  *
@@ -741,24 +741,24 @@ function Intl_NumberFormat_formatToParts
     var nf = this;
 
     // Steps 2-3.
     if (!IsObject(nf) || (nf = GuardToNumberFormat(nf)) === null) {
         return callFunction(CallNumberFormatMethodIfWrapped, this, value,
                             "Intl_NumberFormat_formatToParts");
     }
 
-    // Ensure the NumberFormat internals are resolved.
-    getNumberFormatInternals(nf);
+    var internals = getNumberFormatInternals(nf);
+    var unitStyle = internals.style === "unit";
 
     // Step 4.
     var x = ToNumeric(value);
 
     // Step 5.
-    return intl_FormatNumber(nf, x, /* formatToParts = */ true);
+    return intl_FormatNumber(nf, x, /* formatToParts = */ true, unitStyle);
 }
 
 /**
  * Returns the resolved options for a NumberFormat object.
  *
  * Spec: ECMAScript Internationalization API Specification, 11.4.5.
  */
 function Intl_NumberFormat_resolvedOptions() {
--- a/js/src/tests/jstests.list
+++ b/js/src/tests/jstests.list
@@ -459,19 +459,16 @@ skip script test262/intl402/NumberFormat
 skip script test262/intl402/NumberFormat/prototype/formatToParts/signDisplay-de-DE.js
 skip script test262/intl402/NumberFormat/prototype/format/signDisplay-en-US.js
 skip script test262/intl402/NumberFormat/prototype/format/signDisplay-zh-TW.js
 skip script test262/intl402/NumberFormat/prototype/format/signDisplay-ja-JP.js
 skip script test262/intl402/NumberFormat/prototype/format/signDisplay-ko-KR.js
 skip script test262/intl402/NumberFormat/prototype/format/signDisplay-de-DE.js
 skip script test262/intl402/NumberFormat/prototype/format/signDisplay-rounding.js
 
-# https://bugzilla.mozilla.org/show_bug.cgi?id=1610514
-skip script test262/intl402/NumberFormat/prototype/formatToParts/percent-en-US.js
-
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1608809
 skip script test262/language/expressions/class/super-evaluation-order.js
 
 
 ###########################################################
 # Tests disabled due to issues in test262 importer script #
 ###########################################################
 
new file mode 100644
--- /dev/null
+++ b/js/src/tests/non262/Intl/NumberFormat/unit-formatToParts-has-unit-field.js
@@ -0,0 +1,86 @@
+// |reftest| skip-if(!this.hasOwnProperty("Intl")||release_or_beta)
+
+const sanctionedSimpleUnitIdentifiers = [
+  "acre",
+  "bit",
+  "byte",
+  "celsius",
+  "centimeter",
+  "day",
+  "degree",
+  "fahrenheit",
+  "fluid-ounce",
+  "foot",
+  "gallon",
+  "gigabit",
+  "gigabyte",
+  "gram",
+  "hectare",
+  "hour",
+  "inch",
+  "kilobit",
+  "kilobyte",
+  "kilogram",
+  "kilometer",
+  "liter",
+  "megabit",
+  "megabyte",
+  "meter",
+  "mile",
+  "mile-scandinavian",
+  "milliliter",
+  "millimeter",
+  "millisecond",
+  "minute",
+  "month",
+  "ounce",
+  "percent",
+  "petabyte",
+  "pound",
+  "second",
+  "stone",
+  "terabit",
+  "terabyte",
+  "week",
+  "yard",
+  "year",
+];
+
+// Test only English and Chinese to keep the overall runtime reasonable.
+//
+// Chinese is included because it contains more than one "unit" element for
+// certain unit combinations.
+const locales = ["en", "zh"];
+
+// Plural rules for English only differentiate between "one" and "other". Plural
+// rules for Chinese only use "other". That means we only need to test two values
+// per unit.
+const values = [0, 1];
+
+// Ensure unit formatters contain at least one "unit" element.
+
+for (const locale of locales) {
+  for (const unit of sanctionedSimpleUnitIdentifiers) {
+    const nf = new Intl.NumberFormat(locale, {style: "unit", unit});
+
+    for (const value of values) {
+      assertEq(nf.formatToParts(value).filter(e => e.type === "unit").length > 0, true,
+               `locale=${locale}, unit=${unit}`);
+    }
+  }
+
+  for (const numerator of sanctionedSimpleUnitIdentifiers) {
+    for (const denominator of sanctionedSimpleUnitIdentifiers) {
+      const unit = `${numerator}-per-${denominator}`;
+      const nf = new Intl.NumberFormat(locale, {style: "unit", unit});
+
+      for (const value of values) {
+        assertEq(nf.formatToParts(value).filter(e => e.type === "unit").length > 0, true,
+                 `locale=${locale}, unit=${unit}`);
+      }
+    }
+  }
+}
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);