unwanted localization on Library and Sidebar(en-US Nightly on Japanese edition of Windows10)

RESOLVED FIXED in Firefox 54

Status

()

Toolkit
Places
P2
normal
RESOLVED FIXED
8 months ago
25 days ago

People

(Reporter: Alice0775 White, Assigned: gandalf)

Tracking

(Depends on: 1 bug, {regression})

54 Branch
mozilla54
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54+ fixed)

Details

MozReview Requests

()

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

Attachments

(4 attachments)

(Reporter)

Description

8 months ago
Created attachment 8841316 [details]
screenshot

[Tracking Requested - why for this release]: regression

Steps to reproduce:
1. install en-US Nightly build
2. Open History sidebar and/or Library

Actual Results:
Stupid localization string of date time range.
See screenshot.

Expected Results:
Respect the language of build.
(Reporter)

Updated

8 months ago
Summary: Stupid localization on Library and Sidebar → Stupid localization on Library and Sidebar(en-US Nightly on Japanese edition of Windows10)
(Reporter)

Updated

8 months ago
Summary: Stupid localization on Library and Sidebar(en-US Nightly on Japanese edition of Windows10) → unwanted localization on Library and Sidebar(en-US Nightly on Japanese edition of Windows10)

Comment 1

8 months ago
Sorry, I can't read japanese, could you please explain the problem with more details?
Do I understand correctly that you would like these to be build-locale-based rather than OS-locale based? Why do you think that may be a problem for users? (I'd expect anyone with a japanese OS to be able to read those strings)
Flags: needinfo?(alice0775)
(Reporter)

Comment 2

8 months ago
(In reply to Marco Bonardo [::mak] from comment #1)
> Sorry, I can't read japanese, could you please explain the problem with more
> details?
> Do I understand correctly that you would like these to be build-locale-based
> rather than OS-locale based?

No.

>  Why do you think that may be a problem for
> users? (I'd expect anyone with a japanese OS to be able to read those
> strings)

See screenshot. The ui is mixed up System locale and build-locale. i.e. Japanese and English.
There is no consistency.

Before landing Bug 1225696, All UI was respected build-locale (and also general.useragent.locale).
Flags: needinfo?(alice0775)
(Reporter)

Comment 3

8 months ago
Created attachment 8841465 [details]
Expected results
(Assignee)

Comment 4

8 months ago
The intention should be that we follow the App locale in the UI. In particular we should use the mozilla::intl::Locale service::getAppLocales API for localization of those dates.

Updated

8 months ago
status-firefox51: --- → unaffected
status-firefox52: --- → unaffected

Comment 5

8 months ago
Is this a bug in the DateTimeFormat::XXX utils? They seem to be initialized to the App locale, but then out of FormatUDateTime we get OS localized strings?

Updated

8 months ago
Priority: -- → P2
Tracking 54+ for this visible regression.
tracking-firefox54: ? → +
(Assignee)

Comment 7

8 months ago
:m_kato, is it possible that it's a regression from bug 1225696?
Flags: needinfo?(m_kato)
(Assignee)

Comment 8

8 months ago
Yeah, it seems to me that it is:

In http://searchfox.org/mozilla-central/source/intl/locale/DateTimeFormat.cpp#33 we're using nsILocaleService::getApplicationLocale.

Instead, we should use LocaleService::getAppLocales.

p.s. I know it's confusing - we're pretty close to be able to migrate all callsites and then maybe deprecate the nsILocaleService.
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Comment 10

8 months ago
Created attachment 8841864 [details]
bt.txt

Ugh, it's crashing left and right on try, so the patch will need to be fixed and I don't understand the crash.

Attaching gdb backtrace from the crash. I don't see anything related to this code there, but the crash on try is clearly in DateTimeFormat::Initialize so it has to be related, I just don't understnand how.

Hope :m_kato can help me :)

Comment 11

8 months ago
mozreview-review
Comment on attachment 8841624 [details]
Bug 1342753 - Use app locale in DateTimeFormat.cpp, instead of OS locale.

https://reviewboard.mozilla.org/r/115778/#review117712

It is OK for me.  But UNIX platform will change all date time behaviour by this.  But as UI locale, I think that we shouldn't use LC_TIME.

Example, Chrome (https://bugs.chromium.org/p/chromium/issues/detail?id=226704) is won't fix.
Attachment #8841624 - Flags: review?(m_kato) → review+
(Assignee)

Comment 12

8 months ago
:jfkthame, do you know why this patch crashes?
Flags: needinfo?(jfkthame)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #10)
> Created attachment 8841864 [details]
> bt.txt
> 
> Ugh, it's crashing left and right on try, so the patch will need to be fixed
> and I don't understand the crash.
> 
> Attaching gdb backtrace from the crash. I don't see anything related to this
> code there, but the crash on try is clearly in DateTimeFormat::Initialize so
> it has to be related, I just don't understnand how.
> 
> Hope :m_kato can help me :)

On Linux only?
This stack is child process access cookie string on sync.  So we need investigate parent process's stack and state.

Comment 15

8 months ago
mozreview-review
Comment on attachment 8841624 [details]
Bug 1342753 - Use app locale in DateTimeFormat.cpp, instead of OS locale.

https://reviewboard.mozilla.org/r/115778/#review118630

::: intl/locale/DateTimeFormat.cpp:20
(Diff revision 1)
> -  nsAutoString localeStr;
> +  if (!mLocale->IsEmpty()) {
> -  nsresult rv = NS_OK;
> -
> -  if (!mLocale) {
> -    mLocale = new nsCString();
> -  } else if (!mLocale->IsEmpty()) {
>      return NS_OK;
>    }

You can't dereference `mLocale` here (to call `IsEmpty()`) if it may not have been created yet (and I see the `new` that creates it just below).

I think you want something more like

    if (mLocale) {
      return NS_OK;
    }

to return if it has already been initialized.
Attachment #8841624 - Flags: review-
Comment hidden (mozreview-request)
Looks better, does it run OK now?
Flags: needinfo?(jfkthame)
(Assignee)

Comment 18

8 months ago
yep, thank you! Waiting for try to finish to make sure I don't regress anything but so far so good!
Flags: needinfo?(m_kato)

Comment 19

8 months ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/439f5e4ed073
Use app locale in DateTimeFormat.cpp, instead of OS locale. r=m_kato

Comment 20

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/439f5e4ed073
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54

Comment 21

8 months ago
What's happened here now? Before, On Windows I'd get the date/time format as set in "Region and Language", Formats. I call that the "Format Locale". There I have "English (United Kingdom)", so I used to get DD/MM/YYYY and 24h. With the latest Thunderbird Daily I'm back at US formats: MM/DD/YY and 12 AM/PM.

Switching general.useragent.locale to en-GB made no difference. How can I say it nicely, this merry-go-around of date/time format changes is a little tiring, for myself and other users who are complaining, see bug 1325745 comment #45.

So how do I set the format I want now? Do I really have to install an en-GB language pack?

Comment 22

8 months ago
OK, installed en-GB language pack from:
http://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central-l10n/win32/xpi/thunderbird-54.0a1.en-GB.langpack.xpi

Result: en-US format.

So kindly let me know how I can achieve the format I need or back this out.

Have you tried in Firefox? The only known C++ formatted date that behaves like TB is in the details of a certificate, see bug 1301640 comment #140.
(Assignee)

Comment 23

8 months ago
> What's happened here now?

We changed where we get the locale for date/time formatting in C++ from.

Before, we would get your OS locale. (so it worked for your setup I believe). Now we're getting your app locale.

The reason behind it is that if you have an OS in one locale, but the app in a different locale, we have to match the internationalization data to the localization data.

Otherwise, you can end up in a scenario where you have a polish OS, and en-US Firefox and a string in Firefox that says "Today is Poniedzialek" instead of "Today is Monday" (day of the week coming from Intl).

There are two pieces of the solution for you here:

1) If you want en-GB date/times, you can install en-GB language pack. That will give you en-GB localization and internationalization in Firefox (and spellchecking, line breaking, hypenathions etc.)
2) We should look into extending the C++ DateTimeFormat code to do what I'll do for JS in mozIntl.DateTimeFormat (bug 1339650) which is to look if the language between OS and App matches and if it does, take the regional preferences from the OS and apply them.

But the (2) is more designed for users who modify regional setting manually, and I'm not sure if I should always match cross region (en-GB OS vs en-US Firefox sounds reasonable, but es-MX and es-ES not so sure). 

So yeah, I'd say if you want en-GB intl, you should install en-GB langpack.
(Assignee)

Comment 24

8 months ago
> So kindly let me know how I can achieve the format I need or back this out.

I don't think that you should communicate this way. Sorry, to call this out, but I'm becoming pretty strict about keeping the communication in the bugzilla in line with Mozilla Contribution Guidelines.

No one has an obligation to explain things to you, and you have no power to demand backouts.

I'll look into this, because I want it to work, not because you order me to. Please, keep the communication respectful and friendly.

Comment 25

8 months ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #23)
> So yeah, I'd say if you want en-GB intl, you should install en-GB langpack.
As I said in comment #22, I did install the en-GB language pack which had no effect.

FF l10n stuff isn't available from
http://ftp.mozilla.org/pub/firefox/nightly/2017/03/2017-03-06-11-03-39-mozilla-central-l10n/
yet at the time of writing, otherwise I'd download a localised version (with the language pack pre-installed?) and check out the certificate dates.

Sorry about the tone, but as I said, the constant changes in this area have been a little tiring.
(Assignee)

Comment 26

8 months ago
> Sorry about the tone, but as I said, the constant changes in this area have been a little tiring.

I understand that. I have the best intentions and want to make it work for everyone.

Rewriting 20 years old code across a huge codebase is tiring as well :)
I hope to end up with a code that handles l10n/intl cleanly, but it does take time and require taking stepping stones (like, we need to unify all intl code to use app locale, see what regresses, fix it etc.).
Also, in the process, we're discovering a lot of dirty-hacks and other shenanigans going on in the code that is stitched together with chewing gum and somehow worked. It takes time to get it all moved to clean new APIs.

Sorry for the troubles. I hope to have all of that work finished by the end of march.

Wrt. your problem, it may be related to bug 1344355. Basically, what is happening is:

1) You install your app in en-US
2) You install en-GB langpack
3) You launch you app, and because of bug 1344355 we pick up en-US first
4) LocaleService caches the `en-US` locale
5) AddonsManager registers your langpack as en-GB
6) But LocaleService still keeps the cached en-US locale because, as pointed out in bug 1344355 AddonsManager is not informing anyone about the change
7) You get a date/time displayed using en-US instead of en-GB.
8) You're sad. You report. I'm sad.

So, the fastest solution for you is to download en-GB build once it's available. As much as my work in total is intending to get langpacks to work better, they're not really very stable (our l10n tech lead recommended me to not even bother with bug 1344355 since ChromeRegistry+Locale+Addons are just inherently broken).

You can also CC on bug 1344355. I'm waiting for a couple people there but I may end up just removing cache from LocaleService which will also fix the bug for langpacks or maybe we'll fix bug 1344355 properly and it'll also fix it for langpacks.

Once again, sorry for the trouble and I appreciate your reports. I'm trying to test all configurations before I land patches, but I can't cove everything myself.

Comment 27

8 months ago
OK, installed a FF daily with the matching de-DE language pack, set general.useragent.locale to de-DE. Restarted FF. Looks pretty German now.

Looked at the C++ produced dates in the certificates and I see:

Gültigkeitsdauer:
Beginnt mit: May 30, 2000
Gültig bis:  May 30, 2020.

Or under Details:
Validität, Nicht Vor:
May 30, 2000 at 12:44:50 PM
(May 30, 2000 at 10:44:50 AM GMT)

As becomes immediately visible in Thunderbird since Date/Time display is in a prime UI position, the C++ generated dates are broken again. And this after all the discussion in bug 1301640 which should have made everybody very aware of the issues here.

I'm not demanding any backout, but the quickest way to fix an obvious problem introduced due to lack of testing is a backout.
(Assignee)

Comment 28

8 months ago
Yep, as I said in comment 26, the de build will show de date/times. Langpacks, not so much and it's bug 1344355. I hope to have it fixed this week.

Comment 29

8 months ago
Gotta step out, I got the stuff from here:
http://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central-l10n/win64/xpi/firefox-54.0a1.de.langpack.xpi
http://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/firefox-54.0a1.en-US.win64.zip

I used German since that's more obvious.

Comment 30

8 months ago
OK, I'll see whether I can get hold of a DE build.
(Assignee)

Comment 31

8 months ago
Thanks!

If somehow this will not work, please, let me know and I'd appreciate STR to get to the certificates view and I'll debug it further. But if I'm correct, it should work.
status-firefox-esr52: --- → unaffected

Comment 32

8 months ago
OK, well, sorry about the noise and the harsh words. I finally downloaded a DE build of FF from here:
http://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central-l10n/firefox-54.0a1.de.win64.zip
Lo and behold, that does show the C++ generated certificate dates in German.

Next I downloaded a en-GB build of Thunderbird from here:
http://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central-l10n/thunderbird-54.0a1.en-GB.win32.zip
It a real shame that we don't build x64 l10n versions for TB.
With that version I do get the en-GB dates as predicted.

So it would be great to get bug 1344355 fixed so I can achieve the same effect with a language pack.

What about bug 1308329? Does that re-introduce the ability to set the date/time format in the OS? Would that have an effect on dates/times formatted via the C++ interface?
(Assignee)

Comment 33

8 months ago
Glad that it works!

It'll take some time to get langpacks back in a prime shape but I'd say that we should get there this year.

> What about bug 1308329? Does that re-introduce the ability to set the date/time format in the OS? Would that have an effect on dates/times formatted via the C++ interface?

Not automatically. I'm working on bug 1339650 that will introduce a JS API that merges DateTimeFormat and OSPreferences, but someone would have to improve the intl/locale/DateTimeFormat.cpp to also tap into OSPreferences to achieve the same on the C++ side.

I'd really love us to move all date and time formatting to JS tho ;)

Comment 34

8 months ago
I'm glad that you're glad, but somehow we're now in a less than fortunate situation for Thunderbird users where C++ generated date/time have a prime UI position.

Before bug 1301640 the system would honour the individual OS settings. After bug 1301640 it would at least honour the "format locale" (on Windows: Region and Language, Formats).

Now is ignores all OS settings and you have to install a localised version xx-YY if you want the date/time format corresponding to xx-YY. That makes life real hard, since many developers just use en-US, but given that they're mostly not from the US of course prefer their local formats. Now these people have to switch to the localised version, which as least for TB is not available as x64 build on Windows. That's also a annoying since we need to talk about elements of the user interface, and seeing that localised doesn't allow international communication. So we're really in a spot where we don't want to be.

I suspect you will also get opposition from FF users. Many use en-US FF with specially configured dates in the OS. And after the bug here, FF now completely ignores OS settings and just shows en-US dates everywhere, be it in the C++ generated dates or elsewhere, say in the "View Page Info".

I (semi-)understand where you want to take the whole thing and that obeying OS setting will be possible one day. I hope this day comes soon, because right now we're a the lowest point of the road ;-(
(Assignee)

Comment 35

8 months ago
> Before bug 1301640 the system would honour the individual OS settings. After bug 1301640 it would at least honour the "format locale" (on Windows: Region and Language, Formats).

That's not correct. Before bug 1301640 we would take date/time from the OS for this particular API, while we would format date/time differently when using all other APIs (which rely on ICU).
That meant that we had an inconsistency - two dates in the same app formatted differently.

After bug 1301640, we would have a different inconsistency. Some date/times were formatted using app locale, others using system locale.

That's of course noticable when your os an app have different locale only.

Jorg, what I'm trying to say, for 10 comments now, is that the interplotting between languages, locales, regional settings and internationalization is more complex then your use case.

I understand that the changes we're making impact your use case and I'm sure some other peoples as well, but I'd like you to recognize that it's not a "bug" and it's not a black and white scenario.
Each setup will not work for someone, in some configuration of options and locales.

My goal is to maximize the number of people who experience proper internationalization, and that requires us to:
a) Unify how we internationalize date and time
b) Unify how we select locale which we use for this
c) Unify how we interact between internationalization and os regional preferences

I'm working on (c) as we speak.

> I hope this day comes soon, because right now we're a the lowest point of the road ;-(

You don't know what you don't see. The fact that your setup "is at the lowest point" doesn't mean that others people setup didn't improve. As I listed above, we did fix a number of bugs with those changes - it's just that you personally didn't experience them.

Comment 36

8 months ago
Hi, is there a bug about bringing Date.toLocaleDateFormat(undefined, {}) to also work in this way? Currently 'undefined' locale picks the system locale. Now that this function is the recommended formatter (https://www.fxsitecompat.com/en-CA/docs/2015/date-prototype-tolocaleformat-will-be-removed/), this start to show up. 
It's ugly to have to fetch Components.classes["@mozilla.org/chrome/chrome-registry;1"].getService(Components.interfaces.nsIXULChromeRegistry).getSelectedLocale("global", true); to pass it to Date.toLocaleDateFormat at every callsite. I'd guess Date.toLocaleDateFormat(undefined, {}) is the common usage (you rarely want to format for another locale than the app or the OS is).
Or are we doing something incorrectly?
(Assignee)

Comment 37

8 months ago
So, sort of.

There are two efforts:

1) We're adding mozIntl.DateTimeFormat which will use applocale by defualt (bug 1339650)

2) We're talking about using app locale as "last resort" locale for the whole Gecko. If we get there, we'll initialize SpiderMonkey with this locale as "default".


Also, do not use ChromeRegistry for retrieving locale anymore. We now have a dedicated service for locale management: mozILocaleService.

It's plugged into Services, so if you have them just do `Services.locale.getAppLocales()` and you'll get the current locale fallback chain.

Comment 38

8 months ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #37)
> 1) We're adding mozIntl.DateTimeFormat which will use applocale by defualt
> (bug 1339650)

Is this diffent from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat ? How do we call it?

So is there no attempt to make Date.toLocaleDateFormat(undefined, {}) use the app locale instead of OS locale? Should we use mozIntl.DateTimeFormat to get app locale by default?
 
> 2) We're talking about using app locale as "last resort" locale for the
> whole Gecko. If we get there, we'll initialize SpiderMonkey with this locale
> as "default".

Why last resort? I thought the whole effort is to use app locale everywhere as first choice. So what is the first option you want to get the locale from?

> Also, do not use ChromeRegistry for retrieving locale anymore. We now have a
> dedicated service for locale management: mozILocaleService.
> 
> It's plugged into Services, so if you have them just do
> `Services.locale.getAppLocales()` and you'll get the current locale fallback
> chain.

OK, thanks.

Comment 39

8 months ago
Maybe let me ask this more directly.

In bug 1332351 we need to remove the use of Date.prototype.toLocaleFormat() and use Date.toLocaleDateFormat() instead. That takes the desired locale as first parameter. In the currently proposed patch we pass 'undefined' and get the system locale which causes inconsistencies. So we need to use the app/build locale.

So the best way to get this is |Services.locale.getAppLocale()|. But doing this at every call site isn't so elegant.

I understand that in the future 'undefined' may map to the app/build locale one day. But in the meantime we need to get it and pass it in, right?
(Assignee)

Comment 40

8 months ago
> Is this diffent from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat ? How do we call it?

It's a toolkit component. You can get it from Components.

> So is there no attempt to make Date.toLocaleDateFormat(undefined, {}) use the app locale instead of OS locale? Should we use mozIntl.DateTimeFormat to get app locale by default?

Actually, it seems that there is. One of the callsites I'll be moving as part of the bug 1325870 is this one: 
http://searchfox.org/mozilla-central/rev/36c8dd04de2f82576f62baeb14b135a2dd02c2dc/js/xpconnect/src/XPCLocale.cpp#270

which should mean that we'll end up with JS API using app locale. This will happen over this week.

> Why last resort? I thought the whole effort is to use app locale everywhere as first choice. So what is the first option you want to get the locale from?

Sorry, I used wrong wording.

The way it works is that we take requested locales (the ones you passed to the constructor), and negotiate them against the available ones. The result is called supported locales.

Now, we need a locale to select when none of the requested matches any of the available and we end up with an empty list.
This locale is called "last ditch locale".

In the scenario you described, which is the most common one, when you don't pass any requested locales, we end up with the empty union of requested*available and we reach out for the last ditch.

To sum it up. I believe that over the next couple weeks we'll get to the point where JS API will end up with an app locale when no locale is selected.

> So the best way to get this is |Services.locale.getAppLocale()|. But doing this at every call site isn't so elegant.

Agree. As I said before. I'm moving a huge pile of code from 20 years ago around. It's not pretty.
But we're getting close to the point where we'll unify all callsites to use the right locales.

> I understand that in the future 'undefined' may map to the app/build locale one day. But in the meantime we need to get it and pass it in, right?

Today, yes.
In two weeks, hopefully, no.

Keep an eye on bug 1325870. Once all of it's dependencies are resolved, I believe we should be left with all callsites in the engine using the app locales by default.

Sorry for the hassle, it's a lot of work and I have only two hands :)

Comment 41

8 months ago
Thanks for the explanation. So if you estimate the timeframe to ~2 weeks, that will be great.
We just want all the formatting functions functions be consistent and have the default be app locale (if that is the direction m-c has chosen). Thanks.
(Assignee)

Comment 42

7 months ago
> In the currently proposed patch we pass 'undefined' and get the system locale which causes inconsistencies. So we need to use the app/build locale.

Bug 1346674 just landed which should give app locale for default locale in Intl API.

Comment 43

7 months ago
Any news on the C++ interface? Is my comment in bug 1344594 comment #10 correct? I know for a fact that certificate dates use the C++ interface, but what about the history (sidebar/library)? You fixed this issue here with a C++ change, so does the history also use C++ generated dates or what is the connection?

And re. comment #33:
> someone would have to improve the intl/locale/DateTimeFormat.cpp
> to also tap into OSPreferences to achieve the same on the C++ side
Who will that someone be? You need that change in FF to align C++ generated dates, right?
(Assignee)

Comment 44

7 months ago
> You fixed this issue here with a C++ change, so does the history also use C++ generated dates or what is the connection?

According to:
http://searchfox.org/mozilla-central/source/intl/locale/DateTimeFormat.cpp#26

the C++ DateTimeFormat uses app locale. So yes.

> Who will that someone be? You need that change in FF to align C++ generated dates, right?

No. The amount of work required to hook OSPrefreences into C++ DateTimeFormat is non-trivial, and i can't commit to that.
But this is just about respecting user regional settings modifications. That code is about to land for JS in bug 1339650 and if anyone wants to port that (and half of the Intl.DateTimeFormat pattern resolving code) to C++, I'll be happy to give feedback.
Otherwise my recommendation is to port date/time formatting to JS where it belongs. Starting from dates that are likely affected by OS regional preferences like time (hour12/24 cycle).

Comment 45

7 months ago
Hmm, I think you misunderstood my question. I know that since landing this bug here, the C++ code uses the app locale. Let me rephrase the questions:

There was a complaint here that the history (in sidebar/library) showed Japanese formats in an en-US FF. You fixed that by making the C++ code use the app locale (en-US) instead of the system locale (Japanese).

Fist question: So at least indirectly the history uses the C++ interface. Does it use it directly or what is the connection?

Second question: Since at least the certificate dates use the C++ formatted date/times, they will be inconsistent with the rest of FF unless someone retrofits the new JS functionality onto the C++ interface or changes their display to use JS. Maybe also the history will be inconsistent, but that depends on how that uses the C++ interface, hence the first question.
(Assignee)

Comment 46

7 months ago
> Hmm, I think you misunderstood my question.

Sorry for that!

> So at least indirectly the history uses the C++ interface. Does it use it directly or what is the connection?

No idea.

> Since at least the certificate dates use the C++ formatted date/times, they will be inconsistent with the rest of FF unless someone retrofits the new JS functionality onto the C++ interface or changes their display to use JS. Maybe also the history will be inconsistent, but that depends on how that uses the C++ interface, hence the first question.

If we have C++ backed dates (certificate and maybe history) they will not be adapted to OS regional preferences.
They will for the time being stay inconsistent (due to this being a minor inconsistency and deep in the UI), and once we get to it we'll transition it to JS.

As :emk and others said, we'd like to get rid of C++ DateFormat, both Scriptable and not.

For that reason I don't plan to invest in it, and I recommend Thunderbird to move away from it.

Comment 47

7 months ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #46)
> > So at least indirectly the history uses the C++ interface. Does it use it directly or what is the connection?
> No idea.
OK, I can answer it. The C++ interface is used in a few spots in M-C:
https://dxr.mozilla.org/mozilla-central/search?q=FormatPRExplodedTime&redirect=false

Visible in the UI are the call sites in
toolkit/components/places/nsNavHistory.cpp
You're lucky since there no time is requested. I don't know whether you will get inconsistencies in the month+year display when the JS interface takes some OS settings into account and the C++ interface doesn't.

security/manager/ssl/nsNSSCertValidity.cpp
There you will get inconsistencies since certificate validity displays a full date and time.

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #46)
> As :emk and others said, we'd like to get rid of C++ DateFormat, both
> Scriptable and not.
> For that reason I don't plan to invest in it, and I recommend Thunderbird to
> move away from it.
That's easier said than done, but we'll follow if you remove the mentioned usage in M-C.
(Assignee)

Comment 48

7 months ago
Yup, it'll take some time and I don't plan to rush it.

Generally speaking, my hither focus is now on getting rid of nsILocaleService and ripping localization out of ChromeRegistry.

So, no rush, but yeah, there will be inconsistency in how date is displayed in those two places.

Comment 49

7 months ago
Sorry, but I'll have to REOPEN this. The fix was wrong.

First off, this is filed as a bug in Places. Both the title and the component say so. But the fix was global for all of Firefox and Thunderbird. As such, this is the wrong fix.

Second, the whole idea of the fix is wrong. The UI language and the date/time formats are 2 completely different settings.

Windows, for example, has an explicit setting for region, which is different from the UI language, and even allows the user to customize how dates and numbers appear. This Windows setting has existed since Windows 3.1, if my memory serves me right. Users rightfully expect that this value is used.

Why is this so important? Because many people around the world speak English, and download software in English, meaning "en-US". Sometimes, e.g. for nightlies, it's the only language available. However, the US date format is totally braindead. Nobody in his right mind would write month/day/year, apart from Americans.

The difference is particularly serious between British and American. Brits can use en-US apps in en-US without problem - color vs. colour doesn't matter. But brits write 1/4/17 for April 1st, while Americans write 4/1/17. If you show April 1st as 1/4/17 to a Brit or German, he will read Jan 4. British is just the most obvious case, but the en-US locale is used world-wide. I'm German and I often prefer installing en-US, even if German is available. But I absolutely, positively cannot stand US date, time and number formatting.

There's a reason why Windows (and most other OS) have a specific date/time setting, separate from the UI language. You HAVE to use it. Everything else is just wrong. Apr 1 is 01.04.2017 for me, never 4/1/17, even if the app is in English.

I know that we usually don't re-open bugs, but this is a serious regression, and the bug (if there was one at all) was simply fixed in the wrong code place.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ben, this has already been uplifted to Aurora. Please can you file a new bug, so that getting a resolution on the regression can be tracked properly.
Status: REOPENED → RESOLVED
Last Resolved: 8 months ago7 months ago
Resolution: --- → FIXED

Comment 51

7 months ago
We need to appreciate Zibi's work here to rework this whole area, see:
https://mail.mozilla.org/pipermail/firefox-dev/2017-March/005240.html

I hate the fact that TB is ignoring the OS settings as much as anyone else does, we have bug 1344594 on file, but the proper way to fix this is to find a resource to implement bug 1351427 which is the missing piece:
https://groups.google.com/d/msg/mozilla.dev.platform/-on0pSkG1pE/nPv3LSCiCgAJ
See Also: → bug 1354339
See Also: bug 1354339
Depends on: 1404166
You need to log in before you can comment on or make changes to this bug.