Bug 1728180 - Part 2: Call mozilla::intl::Calendar methods in intl_GetCalendarInfo. r=platform-i18n-reviewers,dminor
authorAndré Bargull <andre.bargull@gmail.com>
Thu, 23 Sep 2021 07:43:51 +0000
changeset 593018 79f2d028ab5120b149d7e591b53eb71cb640f970
parent 593017 a604874baa24dee6891cde46ca1a1ee681d0fae1
child 593019 48edce87cade4fb02493e3c25eff9e63e1ec79b8
push id38820
push usersmolnar@mozilla.com
push dateThu, 23 Sep 2021 21:45:25 +0000
treeherdermozilla-central@4eda9eb8926b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersplatform-i18n-reviewers, dminor
bugs1728180
milestone94.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 1728180 - Part 2: Call mozilla::intl::Calendar methods in intl_GetCalendarInfo. r=platform-i18n-reviewers,dminor Use `mozilla::intl::Calendar` instead of directly calling ICU in `intl_GetCalendarInfo`. This implies that the days of the week are now ordered the same way as in the "Intl Locale Info API" and the "Temporal" proposal, i.e. starting from Monday=1 to Sunday=7. Differential Revision: https://phabricator.services.mozilla.com/D126228
intl/docs/dataintl.rst
js/src/builtin/intl/IntlObject.cpp
js/src/builtin/intl/IntlObject.h
js/src/builtin/intl/IntlObject.js
js/src/tests/non262/Intl/getCalendarInfo.js
toolkit/modules/DateTimePickerPanel.jsm
--- a/intl/docs/dataintl.rst
+++ b/intl/docs/dataintl.rst
@@ -136,40 +136,40 @@ produce the correct pattern for short da
 
 
 mozIntl.getCalendarInfo(locale)
 -------------------------------
 
 The API will return the following calendar information for a given locale code:
 
 * firstDayOfWeek
-    an integer in the range 1=Sunday to 7=Saturday indicating the day
-    considered the first day of the week in calendars, e.g. 1 for en-US,
-    2 for en-GB, 1 for bn-IN
+    an integer in the range 1=Monday to 7=Sunday indicating the day
+    considered the first day of the week in calendars, e.g. 7 for en-US,
+    1 for en-GB, 7 for bn-IN
 * minDays
     an integer in the range of 1 to 7 indicating the minimum number
     of days required in the first week of the year, e.g. 1 for en-US, 4 for de
 * weekend
-    an array with values in the range 1=Sunday to 7=Saturday indicating the days
-    of the week considered as part of the weekend, e.g. [1, 7] for en-US and en-GB,
-    [1] for bn-IN (note that "weekend" is *not* necessarily two days)
+    an array with values in the range 1=Monday to 7=Sunday indicating the days
+    of the week considered as part of the weekend, e.g. [6, 7] for en-US and en-GB,
+    [7] for bn-IN (note that "weekend" is *not* necessarily two days)
 
 Those bits of information should be especially useful for any UI that works
 with calendar data.
 
 Example:
 
 .. code-block:: javascript
 
     // omitting the `locale` argument will make the API return data for the
     // current Gecko application UI locale.
     let {
-      firstDayOfWeek,  // 2
+      firstDayOfWeek,  // 1
       minDays,         // 4
-      weekend,         // [1, 7]
+      weekend,         // [6, 7]
       calendar,        // "gregory"
       locale,          // "pl"
     } = Services.intl.getCalendarInfo();
 
 
 mozIntl.DisplayNames(locales, options)
 -----------------------------------------
 
--- a/js/src/builtin/intl/IntlObject.cpp
+++ b/js/src/builtin/intl/IntlObject.cpp
@@ -26,29 +26,26 @@
 #include "builtin/intl/DateTimeFormat.h"
 #include "builtin/intl/FormatBuffer.h"
 #include "builtin/intl/LanguageTag.h"
 #include "builtin/intl/MeasureUnitGenerated.h"
 #include "builtin/intl/NumberFormat.h"
 #include "builtin/intl/NumberingSystemsGenerated.h"
 #include "builtin/intl/PluralRules.h"
 #include "builtin/intl/RelativeTimeFormat.h"
-#include "builtin/intl/ScopedICUObject.h"
 #include "builtin/intl/SharedIntlData.h"
 #include "ds/Sort.h"
 #include "js/CharacterEncoding.h"
 #include "js/Class.h"
 #include "js/friend/ErrorMessages.h"  // js::GetErrorMessage, JSMSG_*
 #include "js/GCAPI.h"
 #include "js/GCVector.h"
 #include "js/PropertySpec.h"
 #include "js/Result.h"
 #include "js/StableStringChars.h"
-#include "unicode/ucal.h"
-#include "unicode/utypes.h"
 #include "vm/GlobalObject.h"
 #include "vm/JSAtom.h"
 #include "vm/JSContext.h"
 #include "vm/JSObject.h"
 #include "vm/PlainObject.h"  // js::PlainObject
 #include "vm/StringType.h"
 #include "vm/WellKnownAtom.h"  // js_*_str
 
@@ -63,76 +60,55 @@ bool js::intl_GetCalendarInfo(JSContext*
   CallArgs args = CallArgsFromVp(argc, vp);
   MOZ_ASSERT(args.length() == 1);
 
   UniqueChars locale = intl::EncodeLocale(cx, args[0].toString());
   if (!locale) {
     return false;
   }
 
-  UErrorCode status = U_ZERO_ERROR;
-  const UChar* uTimeZone = nullptr;
-  int32_t uTimeZoneLength = 0;
-  UCalendar* cal = ucal_open(uTimeZone, uTimeZoneLength, locale.get(),
-                             UCAL_DEFAULT, &status);
-  if (U_FAILURE(status)) {
-    intl::ReportInternalError(cx);
+  auto result = mozilla::intl::Calendar::TryCreate(locale.get());
+  if (result.isErr()) {
+    intl::ReportInternalError(cx, result.unwrapErr());
     return false;
   }
-  ScopedICUObject<UCalendar, ucal_close> toClose(cal);
+  auto calendar = result.unwrap();
 
   RootedObject info(cx, NewPlainObject(cx));
   if (!info) {
     return false;
   }
 
   RootedValue v(cx);
-  int32_t firstDayOfWeek = ucal_getAttribute(cal, UCAL_FIRST_DAY_OF_WEEK);
-  v.setInt32(firstDayOfWeek);
 
+  v.setInt32(static_cast<int32_t>(calendar->GetFirstDayOfWeek()));
   if (!DefineDataProperty(cx, info, cx->names().firstDayOfWeek, v)) {
     return false;
   }
 
-  int32_t minDays = ucal_getAttribute(cal, UCAL_MINIMAL_DAYS_IN_FIRST_WEEK);
-  v.setInt32(minDays);
+  v.setInt32(calendar->GetMinimalDaysInFirstWeek());
   if (!DefineDataProperty(cx, info, cx->names().minDays, v)) {
     return false;
   }
 
   RootedArrayObject weekendArray(cx, NewDenseEmptyArray(cx));
   if (!weekendArray) {
     return false;
   }
 
-  for (int i = UCAL_SUNDAY; i <= UCAL_SATURDAY; i++) {
-    UCalendarDaysOfWeek dayOfWeek = static_cast<UCalendarDaysOfWeek>(i);
-    UCalendarWeekdayType type = ucal_getDayOfWeekType(cal, dayOfWeek, &status);
-    if (U_FAILURE(status)) {
-      intl::ReportInternalError(cx);
-      return false;
-    }
+  auto weekend = calendar->GetWeekend();
+  if (weekend.isErr()) {
+    intl::ReportInternalError(cx, weekend.unwrapErr());
+    return false;
+  }
 
-    switch (type) {
-      case UCAL_WEEKDAY:
-        break;
-      case UCAL_WEEKEND:
-        if (!NewbornArrayPush(cx, weekendArray, Int32Value(i))) {
-          return false;
-        }
-        break;
-      case UCAL_WEEKEND_ONSET:
-      case UCAL_WEEKEND_CEASE:
-        // At the time this code was added, ICU apparently never behaves this
-        // way, so just throw, so that users will report a bug and we can
-        // decide what to do.
-        intl::ReportInternalError(cx);
-        return false;
-      default:
-        break;
+  for (auto day : weekend.unwrap()) {
+    if (!NewbornArrayPush(cx, weekendArray,
+                          Int32Value(static_cast<int32_t>(day)))) {
+      return false;
     }
   }
 
   v.setObject(*weekendArray);
   if (!DefineDataProperty(cx, info, cx->names().weekend, v)) {
     return false;
   }
 
--- a/js/src/builtin/intl/IntlObject.h
+++ b/js/src/builtin/intl/IntlObject.h
@@ -15,27 +15,27 @@ namespace js {
 extern const JSClass IntlClass;
 
 /**
  * Returns a plain object with calendar information for a single valid locale
  * (callers must perform this validation).  The object will have these
  * properties:
  *
  *   firstDayOfWeek
- *     an integer in the range 1=Sunday to 7=Saturday indicating the day
- *     considered the first day of the week in calendars, e.g. 1 for en-US,
- *     2 for en-GB, 1 for bn-IN
+ *     an integer in the range 1=Monday to 7=Sunday indicating the day
+ *     considered the first day of the week in calendars, e.g. 7 for en-US,
+ *     1 for en-GB, 7 for bn-IN
  *   minDays
  *     an integer in the range of 1 to 7 indicating the minimum number
  *     of days required in the first week of the year, e.g. 1 for en-US,
  *     4 for de
  *   weekend
- *     an array with values in the range 1=Sunday to 7=Saturday indicating the
- *     days of the week considered as part of the weekend, e.g. [1, 7] for en-US
- *     and en-GB, [1] for bn-IN (note that "weekend" is *not* necessarily two
+ *     an array with values in the range 1=Monday to 7=Sunday indicating the
+ *     days of the week considered as part of the weekend, e.g. [6, 7] for en-US
+ *     and en-GB, [7] for bn-IN (note that "weekend" is *not* necessarily two
  *     days)
  *
  * NOTE: "calendar" and "locale" properties are *not* added to the object.
  */
 [[nodiscard]] extern bool intl_GetCalendarInfo(JSContext* cx, unsigned argc,
                                                JS::Value* vp);
 
 /**
--- a/js/src/builtin/intl/IntlObject.js
+++ b/js/src/builtin/intl/IntlObject.js
@@ -38,17 +38,17 @@ function Intl_supportedValuesOf(key) {
  *     The first day of the week for the resolved locale.
  *
  *   minDays:
  *     The minimum number of days in a week for the resolved locale.
  *
  *   weekend:
  *     The days of the week considered as the weekend for the resolved locale.
  *
- * Days are encoded as integers in the range 1=Sunday to 7=Saturday.
+ * Days are encoded as integers in the range 1=Monday to 7=Sunday.
  */
 function Intl_getCalendarInfo(locales) {
     // 1. Let requestLocales be ? CanonicalizeLocaleList(locales).
     const requestedLocales = CanonicalizeLocaleList(locales);
 
     const DateTimeFormat = dateTimeFormatInternalProperties;
 
     // 2. Let localeData be %DateTimeFormat%.[[localeData]].
--- a/js/src/tests/non262/Intl/getCalendarInfo.js
+++ b/js/src/tests/non262/Intl/getCalendarInfo.js
@@ -18,59 +18,59 @@ function checkCalendarInfo(info, expecte
 
 addIntlExtras(Intl);
 
 let gCI = Intl.getCalendarInfo;
 
 assertEq(gCI.length, 1);
 
 checkCalendarInfo(gCI('en-US'), {
-  firstDayOfWeek: 1,
+  firstDayOfWeek: 7,
   minDays: 1,
-  weekend: [1, 7],
+  weekend: [6, 7],
   calendar: "gregory",
   locale: "en-US"
 });
 
 checkCalendarInfo(gCI('en-IL'), {
-  firstDayOfWeek: 1,
+  firstDayOfWeek: 7,
   minDays: 1,
-  weekend: [6, 7],
+  weekend: [5, 6],
   calendar: "gregory",
   locale: "en-IL"
 });
 
 
 checkCalendarInfo(gCI('en-GB'), {
-  firstDayOfWeek: 2,
+  firstDayOfWeek: 1,
   minDays: 4,
-  weekend: [1, 7],
+  weekend: [6, 7],
   calendar: "gregory",
   locale: "en-GB"
 });
 
 
 checkCalendarInfo(gCI('pl'), {
-  firstDayOfWeek: 2,
+  firstDayOfWeek: 1,
   minDays: 4,
-  weekend: [1, 7],
+  weekend: [6, 7],
   calendar: "gregory",
   locale: "pl"
 });
 
 checkCalendarInfo(gCI('ar-IQ'), {
-  firstDayOfWeek: 7,
+  firstDayOfWeek: 6,
   minDays: 1,
-  weekend: [6, 7],
+  weekend: [5, 6],
   calendar: "gregory",
   locale: "ar-IQ"
 });
 
 checkCalendarInfo(gCI('fa-IR'), {
-  firstDayOfWeek: 7,
+  firstDayOfWeek: 6,
   minDays: 1,
-  weekend: [6],
+  weekend: [5],
   calendar: "persian",
   locale: "fa-IR"
 });
 
 if (typeof reportCompare === 'function')
     reportCompare(0, 0);
--- a/toolkit/modules/DateTimePickerPanel.jsm
+++ b/toolkit/modules/DateTimePickerPanel.jsm
@@ -266,23 +266,27 @@ var DateTimePickerPanel = class {
         break;
       }
     }
   }
 
   getCalendarInfo(locale) {
     const calendarInfo = Services.intl.getCalendarInfo(locale);
 
-    // Day of week from calendarInfo starts from 1 as Sunday to 7 as Saturday,
+    // Day of week from calendarInfo starts from 1 as Monday to 7 as Sunday,
     // so they need to be mapped to JavaScript convention with 0 as Sunday
     // and 6 as Saturday
-    let firstDayOfWeek = calendarInfo.firstDayOfWeek - 1,
+    function toDateWeekday(day) {
+      return day === 7 ? 0 : day;
+    }
+
+    let firstDayOfWeek = toDateWeekday(calendarInfo.firstDayOfWeek),
       weekend = calendarInfo.weekend;
 
-    let weekends = weekend.map(day => day - 1);
+    let weekends = weekend.map(toDateWeekday);
 
     return {
       firstDayOfWeek,
       weekends,
     };
   }
 
   handleEvent(aEvent) {