Long date uses OS date so it appears in OS language instead of Sorbian because there is no Sorbian OS

RESOLVED FIXED in 4.4

Status

Calendar
Internal Components
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Michael Wolf, Assigned: MakeMyDay)

Tracking

Lightning 4.0.0.1
Unspecified
Windows

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

2 years ago
Current situation:

Upper Sorbian and Lower Sorbian will be new Lightning locales from TB 38.0. Both Sorbian languages are minority languages and there is no OS in Sorbian. Lightning uses the long date form the OS and thus the OS language (here German) is used for the long date. For me as translator of a minority language this is a form of hardcoding.

Expected situation:

Long date (and perhaps other hardcode dates I do not know about yet) should be fully localizable that the long date appears in the locale language of Lightning.
(Assignee)

Comment 1

2 years ago
Created attachment 8609896 [details] [diff] [review]
LongDateFormatReliesOnOSLocalization-V1.diff

Confirming this at least on Windows. This get's visible in the calendar pref UI, today pane, the day view and maybe elsewhere.

We have a fallback for Linux already, where we compose the long date string on our own. This patch removes the code where we're trying to get this from the OS and use the existing fallback instead for all OS.

NB: on Windows, this changes displaying day names from long to short names (Monday to Mon) for me, but I think that is okay and maybe related to my OS settings.
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Attachment #8609896 - Flags: review?(philipp)
(Assignee)

Updated

2 years ago
Component: Calendar Views → Internal Components
OS: Windows XP → Windows
Hardware: x86 → Unspecified
Check out the discussion in bug 399605. It seems there are some reasons why we use the OS date in some places. Maybe ssitter can comment on this?
Flags: needinfo?(ssitter)

Comment 3

2 years ago
If I recall correctly Sunbird/Lightning used/mixed some hard coded localization specific date and time formats in the past. This was changed with e.g. Bug 460988 to use the date and time format specified by the user in the OS. Probably main reason was to show dates and times that are known to the user and matches what is shown in Thunderbird. Currently we are using nsIScriptableDateFormat, see e.g. Bug 441167 for the request to change it from OS language to application language.
Flags: needinfo?(ssitter)
(Assignee)

Comment 4

2 years ago
Based on comment #3 I would call this one a duplicate of bug 441167, unless we want to work around thisin Calendar by the proposed solution, which may have downsides on different platforms. So, what do you think how to deal with that issue now?
(Reporter)

Comment 5

2 years ago
My focus is on the language. I think in bug 441167 the focus is more on the date format. There is no OS in Sorbian so the longe date is displayed in German. The date format is approximately the same:
%d. %B %Y. There is a small diference only: The Sorbian month name is in genitive singular. In fact, both things must be correct: date format and language. And as language should be used the Lightning language.

Example:

German: 2. Juni 2015
Sorbian: 2. junija 2015

The month names is "junij" - "junija" is the genitive singular.
(Assignee)

Comment 6

2 years ago
Created attachment 8615931 [details] [diff] [review]
LongDateFormatReliesOnOSLocalization-V2.diff

Ok, this one leaves it in general to use OS long date, but checks additionally, whether localized weekdays appear therein. If not, the fall back approach is applied like for Linux already.
Attachment #8609896 - Attachment is obsolete: true
Attachment #8609896 - Flags: review?(philipp)
Attachment #8615931 - Flags: review?(philipp)
(Reporter)

Comment 7

2 years ago
A question: Is there already a TB milestone for that? TB 38.0.1 uses system long date yet.
(Assignee)

Comment 8

2 years ago
That depends whether we go with the proposed approach within Lightning or a fix in toolkit is required instead. In case of the latter, there will be no ETA at the moment. If we proceed with this patch, the fix will be at least in next ESR, but eventually we can backport it to 4.0.x as well.

Please note, that the current patch will result not in "Juni" resp. "junija" but in the three letter abbreviation "Jun" as this is today aleady the case for Linux.
Comment on attachment 8615931 [details] [diff] [review]
LongDateFormatReliesOnOSLocalization-V2.diff

Review of attachment 8615931 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp with comments considered. I'd suggest testing this patch on trunk for a while before we backport it to 4.0.x

::: calendar/base/src/calDateTimeFormatter.js
@@ +102,5 @@
> +            // check whether weekday name appears as in Lightning localization. if not, this is
> +            // probably a minority language without OS support, so we should fall back to compose
> +            // longDate on our own. May be not needed anymore once bug 441167 is fixed.
> +            if (longDate.indexOf(this.dayName(aDate.weekday) == -1) &&
> +                longDate.indexOf(this.longDayName(aDate.weekday) == -1)) {

You can use longDate.includes() here. Also, since this is really a one time thing, can you set this.mUseLongDateService = false here to cache the value? Maybe it makes sense to check a dummy value once when initializing mUseLongDateService, which would simplify this method a bit.
Attachment #8615931 - Flags: review?(philipp) → review+
(Assignee)

Comment 10

2 years ago
Created attachment 8629593 [details] [diff] [review]
LongDateFormatReliesOnOSLocalization-V3.diff

I have changed to includes() and cached this.mUseLongDateService. To move the check to the initializing code wouldn't bring any benefit from my pow.
Attachment #8615931 - Attachment is obsolete: true
Attachment #8629593 - Flags: review+
(Assignee)

Updated

2 years ago
Blocks: 1180416
Keywords: checkin-needed
Version: Lightning 4.2 → Lightning 4.0
Pushed to comm-central changeset 40897c80a0e6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.4
(Assignee)

Updated

2 years ago
Blocks: 1194997
No longer blocks: 1180416
(Assignee)

Comment 12

2 years ago
Comment on attachment 8629593 [details] [diff] [review]
LongDateFormatReliesOnOSLocalization-V3.diff

Let's track this one for 4.0.3.
Attachment #8629593 - Flags: approval-calendar-release?(philipp)
Attachment #8629593 - Flags: approval-calendar-beta?(philipp)
Comment on attachment 8629593 [details] [diff] [review]
LongDateFormatReliesOnOSLocalization-V3.diff

Given the recent failures in 4.0.x releases and due to the fact that changing the date format isn't a light thing to do, I am not yet convinced we should take this for 4.0.3.

I'm happy to reconsider if you can convince me. Maybe it would help to add a unit test for this change.
Attachment #8629593 - Flags: approval-calendar-release?(philipp)
Attachment #8629593 - Flags: approval-calendar-release-
Attachment #8629593 - Flags: approval-calendar-beta?(philipp)
Attachment #8629593 - Flags: approval-calendar-beta-
(Assignee)

Comment 14

2 years ago
This patch provides only an additional fallback solution which is already applied today for Linux. That said, if that fails with the patch, it would have done without as well. A test on a not localized environment (in a minority language) would make no real sense.
(Assignee)

Updated

2 years ago
No longer blocks: 1194997

Comment 15

2 years ago
This change seems to have regressed Bug 1234223.
Depends on: 1234223
You need to log in before you can comment on or make changes to this bug.