Date picker mixes different calendars when using with Persian (fa) locale

RESOLVED FIXED in Firefox 56

Status

()

Core
Layout: Form Controls
P1
normal
RESOLVED FIXED
5 months ago
3 months ago

People

(Reporter: Arash, Assigned: scottwu)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug)

unspecified
mozilla56
All
Unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

5 months ago
Created attachment 8874237 [details]
Explanation of issues on screenshot

When using Firefox with fa (Persian) locale, the date picker mixes the Gregorian calendar with Hijri.

What's happening:

As in the screenshot attached, the month selector chooses the month based on the Hijri calendar index, then shows the title based on the Gregorian calendar (The selected month is 5th in Hijri, but it shows the 5th month in Gregorian calendar). Then on the input field the picker inserts "05: ۰۵" for month index (local calendar), but then chooses Gregorian year (2017: ۲۰۱۷) for year (local year is 1395: ۱۳۹۵).
The same thing is true about "day". The dropdown chooses the day based on local calendar (not Gregorian), while the year is still chosen from Gregorian calendar.

What should happen:

I'm not sure what is the right behavior. Based on the documents, the datepicker should normalize the data format sent to the server, regardless of the user's locale. So I guess the date picker should show the local calendar but send the Gregorian date. But right now it's mixed and all wrong.
I'm setting blocking to the defect meta so that we can triage it.
Blocks: 1323674
No longer blocks: 888320
yeah, we should either consistently use local calendar in the picker or gregorian.
(Assignee)

Comment 3

5 months ago
It looks like we should have used Gregorian rather than Hijri strings. The month/year button on the top should display Gregorian month and year in Farsi, so should the months and years spinners.

Currently the month strings are from the `mozIntl.getDisplayNames` API:
http://searchfox.org/mozilla-central/rev/20963d7269b1b14d455f47bc0260d0653015bf84/toolkit/content/widgets/datetimepopup.xml#130-144

I'm playing with the API, and found that when I ask for "dates/gregorian/months/june" with locale set as "fa", it returns "شهریور", which is actually a Hijri month. But when the locale is set as "fa-u-ca-gregory", it returns "ژوئن", which appears to be the correct Gregorian month.

It looks like I should append "-u-ca-gregory" to the locale string (from `Services.locale.getAppLocaleAsBCP47()`) to make sure I get the Gregorian calendar format. Is there anything I'm missing :zibi?
Flags: needinfo?(gandalf)
oh, that's a bug. We should normalize to gregorian calendar when fetching gregorian calendar fields.

Unfortunately, I doubt there's a simple fix here :(
just adding `u-ca-gregory` will not work even as a workaround since the locale code may already contain the `ca` key and that could result in `fa-u-ca-islamic-u-ca-gregory`.

I see two solutions here:

1) We change the keys to just say "dates/months/june" and help you build a proper locale code with gregorian calendar
2) We fix the `intl_ComputeDisplayNames` in Intl.cpp to return names for gregory calendar

I'm not sure which one is better. (1) requires some API that takes a locale code and options and returns a locale code with options applied, like:

```
let locale = new Intl.Locale('fa-u-ca-islamic', {
  calendar: 'gregory'
});
locale.toString(); // `fa-u-ca-gregory'
```

That's exactly an API that we were planning for ECMA402 - see https://github.com/tc39/ecma402/issues/106

(2) would not require adding any new API, but would require changing the logic in C++ so that we generate different locale code for different keys (in case someone would ask for `keys: ['dates/gregorian/months/june', 'dates/islamic/months/july']`).

I think I'm leaning toward (1).

:littledan - do you have any thoughts on this?
Flags: needinfo?(littledan)
[triage]
P1 for serious user impact.
This takes efforts from l10n , not only front-end.
Priority: -- → P1
(Assignee)

Comment 6

5 months ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> I see two solutions here:
> 
> 1) We change the keys to just say "dates/months/june" and help you build a
> proper locale code with gregorian calendar
> 2) We fix the `intl_ComputeDisplayNames` in Intl.cpp to return names for
> gregory calendar

Option 1 would make things very convenient, but I see that the API is still in discussion, and might not be ready for implementation yet.

One thing I'm not sure if option 2 could do is to generate the correct year strings. In this case, the years I got from DateTimeFormat are Hijri years, which is very neat but not what we want if the picker is to be all Gregorian. Since I'm not getting the year string (and the month-year string on the top) one by one from the `mozIntl.getDisplayNames`, improving it would not solve this issue I think.

Comment 7

4 months ago
Constructing the BCP 47 locale with the Gregorian calendar sounds like a good way to handle this to me. (I imagine a calendar picker for more non-Gregorian calendars will be a bit more work...). Nice to see a use case for the Intl.Locale API here which motivates its addition.

For the short term, you could use a JS library to manipulate BCP 47 tags, such as https://www.npmjs.com/package/bcp-47 , or implement the manipulation yourself, with some simple code like this:

function setCalendar(locale) {
  if (locale.match(/u-ca-/)) {
    return locale.replace(/u-ca-[^-]+/, "u-ca-gregory");
  }
  return  locale + "-u-ca-gregory";
}
Flags: needinfo?(littledan)
:scottwu, for now, I think Daniel's proposal from comment 7 should work. Feel free to use it to override the calendar.

I filed bug 1376616 to implement mozIntl.Locale which would give you:

function setGregoryCalendar(locale) {
  return new mozIntl.Locale(locale, { calendar: 'gregory' }).toString();
}
Depends on: 1376616
Flags: needinfo?(gandalf) → needinfo?(scwwu)
(Assignee)

Comment 9

4 months ago
Thanks :gandalf and :littledan, I'll use the suggestion from comment 7 as a fix for now.
Assignee: nobody → scwwu
Status: NEW → ASSIGNED
Flags: needinfo?(scwwu)
I filed bug 1377218 for the issue with mozIntl.getDisplayNames.
Depends on: 1377218
Comment hidden (mozreview-request)
Comment on attachment 8886942 [details]
Bug 1370086 - [DateTimePicker] Ensure calendar type is Gregorian for all locales.

https://reviewboard.mozilla.org/r/157702/#review162970

Seems okay, thanks!

::: toolkit/content/widgets/datetimepopup.xml:290
(Diff revision 1)
> +        <parameter name="locale"/>
> +        <body><![CDATA[
> +          if (locale.match(/u-ca-/)) {
> +            return locale.replace(/u-ca-[^-]+/, "u-ca-gregory");
> +          }
> +          return  locale + "-u-ca-gregory";

Nit - spare space after return
Attachment #8886942 - Flags: review?(mconley) → review+

Comment 13

3 months ago
mozreview-review
Comment on attachment 8886942 [details]
Bug 1370086 - [DateTimePicker] Ensure calendar type is Gregorian for all locales.

https://reviewboard.mozilla.org/r/157702/#review163000
Attachment #8886942 - Flags: review?(gandalf) → review+
(Assignee)

Comment 14

3 months ago
mozreview-review-reply
Comment on attachment 8886942 [details]
Bug 1370086 - [DateTimePicker] Ensure calendar type is Gregorian for all locales.

https://reviewboard.mozilla.org/r/157702/#review162970

> Nit - spare space after return

Good catch. Thanks!
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Keywords: checkin-needed

Comment 16

3 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e821f5a748d3
[DateTimePicker] Ensure calendar type is Gregorian for all locales. r=gandalf,mconley
Keywords: checkin-needed

Comment 17

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e821f5a748d3
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: mozilla55 → mozilla56
You need to log in before you can comment on or make changes to this bug.