1355307 - [regression] Use OS date/time format setting again instead of UI language in DateTimeFormat.cpp

Status: NEW

(Core :: Internationalization, defect, P3)

Description

[Ben Bucksch (:BenB)]

Bug 1351427 states:

> Expected Results:
> Respect the language of build.

This is wrong. This is never correct. The OS date/time format setting is more specific and always supersedes the UI language of Firefox.

The reporter was confused. Probably his OS settings were misconfigured, and he didn't understand that.

This caused serious regressions in a lot of places, see bug 1354339.

As such, bug 1351427 must be backed out to restore correct behavior.

Comment 1

[Ben Bucksch (:BenB)]

Bug 1351427 was landed for FF54. This caused widespread regressions everywhere. It should be backed out.

Comment 2

[Ben Bucksch (:BenB)]

Not only did it cause regressions, but it was also inherently incorrect. It removed a long-standing feature that many users around the world depend on.

This affects everybody who uses English (en-US) builds and who is not US american. It shows wrong dates and times for these users, in many places in the UI.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Bug 1351427 did not land anywhere yet.

> The OS date/time format setting is more specific and always supersedes the UI language of Firefox.

Bug 1351427 adds the ability for DateTimeFormat.cpp to alter the patterns using OS date/time format settings.

If your goal is to get en-AU Windows regional settings to be displayed in en-US Firefox, bug 1351427 is going to do that.


Also, we should not use OS locale for DateTimeFormat, because it leads to bugs like German month names in English sentences in the product.

Comment 4

[Ben Bucksch (:BenB)]

Sorry, I picked the wrong bug number above. Instead of bug 1351427, I meant bug 1342753.

----

> Bug 1351427 adds the ability...

What do you mean with "ability"? The default - if nothing explicit is specified by the caller - should be to use the OS setting. That was the old behavior, and it was correct. The new behavior of using the UI language is wrong. Why is it not sufficient to just revert? Please explain.

> it leads to bugs like German month names in English sentences in the product.

That's not what I am seeing in FF52 (before bug 1342753).

> we should not use OS locale for DateTimeFormat

We should not use OS locale per se, but we should use the OS date/time format settings. We've always done that, and that's what users expect.

If I configure Windows/Linux/Mac to use "1.4.2017" for April 1, I expect Firefox to use that format. Likewise with 01.04.2017, or I might even configure it to use 2017-04-01. But I most definitely never ever want 4/1/2017, even if I use English language builds.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

There's some history to that story and I'd prefer not to dive too deep into this.

The tl;dr is - historically we didn't have a way to localize date/time in Firefox. We had to rely on OS.

The cost was that in the scenario where OS and product locale differed, the date/time would end up in a wrong locale. The benefit was that the date would be customized/formatted using regional preferences.

Around 51 we started unifying how we format date/time using Intl.DateTimeFormat API based on ICU/CLDR. That allows us to format date/time according to product UI locale.

The cost is that we lost ability to adjust the formatting to regional preferences.
I spent a good part of the last year, designing a potential solution to that. For reference see:

 - https://github.com/tc39/ecma402/issues/109
 - https://bugzilla.mozilla.org/show_bug.cgi?id=1308329
 - https://bugzilla.mozilla.org/show_bug.cgi?id=1339650
 - https://bugzilla.mozilla.org/show_bug.cgi?id=1329904

It's a fairly sophisticated solution that allows us to achieve trifecta:
 - unify date/time formatting around a standardized Intl API (ECMA402)
 - align date/time format localization to product localization
 - allow us to adjust the date/time format to the OS regional settings


:BenB, I said it before, and I'll repeat it again. Your tone of communication is no acceptable to me. Please, stop demanding, ordering and claiming that what you don't understand is wrong.
I'm glad you started asking questions, but introducing you to the scope of complexity of the problem is not something I have to do, so forgive me if I will further prioritize work over countering your assumptions.

Comment 6

[Ben Bucksch (:BenB)]

> We had to rely on OS.

Using the OS date/time formatting is an important requirement that must not regress.

> Your tone of communication is no acceptable to me.

Which tone? I am strictly objective. Bug 1342753 apparently caused massive regressions. And you're arguing that's not a bug. I am faced with a broken product that is unusable. I count at least 5 developers who tried to tell you so.

This is software that 400 million people are using. Breaking 1% of them = 4 million people is unacceptable. This bug here is breaking a lot more than that.

You discount them as "few users" and "edge cases". I have not once attacked you personally. Please stop making this a personal matter.

If you really have fixes that will honor the OS date/time setting everywhere by default, without the caller having to specify anything, and it all works, that's nice. I haven't seen you state that yet. If that's the case, then all is fine. However, you're saying that it will be ready for Gecko 55 at best, and you're saying it's complex, so it may not all be ready even for Gecko 55. However, the regression is already in Gecko 54. That's why I am asking for a backout, because this cannot be released as-is. If you later have your complicated and perfect solution ready that uses OS formatting, but UI language month strings, then fine, land it when it's ready and all working perfectly in all cases. Until then, please preserve the status quo. Because this half-fix is damaging. Can we agree on this?

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

> Using the OS date/time formatting is an important requirement that must not regress.

The decision on what can and cannot regress belongs to the module and product owners and peers in this project. Please, stop making statements like this, as they make it really tempting to not respond to you.

> Bug 1342753 apparently caused massive regressions.

Please, stop trying to escalate the severity of the problem. "massive" is a relative term that I'd apply to severe startup crash bugs, not date formatting bugs.

> I have not once attacked you personally. Please stop making this a personal matter.

I am not taking it personally. I'm repeatedly informing you that it is not acceptable for you to use phrases like "this must not happen", "this cannot be released" "this must be reverted", "you must do this or I'll do that" and so on.

Unless I missed that and you're a product owner for Firefox, module owner for Gecko or Intl, you're not in a position to decide what can and cannot happen in the project.

> I haven't seen you state that yet.

I repeated it multiple times.

> Can we agree on this?

I'm afraid not. As I also stated multiple times, the net of interdependencies is complicated. The bug you believe should be backed out to fix one regression will reintroduce another regression - are you going to fix that regression yourself when you back out the patch?

I'm sorry, but I really believe that you're spending my time that would be better spent on writing patches on discussing with your claim wrong claim about the solution and architectural decisions around the area which I'm afraid I have every reason to believe you do not fully understand.

Please, stop. If you want to help, help :Jorg test his patch in bug 1351427 which if I'm correct will fix your particular problem.

Comment 8

[Ben Bucksch (:BenB)]

You keep attacking me personally. I would like to point out that neither me nor Mark have attacked you personally, at all. I merely disagree with your patches. That is totally ontopic in bugzilla. In fact, bugzilla is all about reporting bugs. That's what I am doing.

> I'm informing you that it is not acceptable for you to use phrases like "this cannot be released"

Excuse me? Why? Because you disagree? I am not allowed to disagree with you? At least 6 different developers told you that this state is broken. Why should I not be allowed to say that? These are my products, too. Am I to accept that my products are broken?

You keep saying that it's not an issue. This is bad enough that I had to stop using these 55 builds and go back to 52. This affects everybody who uses the lead (en-US) locale builds outside USA. That's a lot of people.

Could you please shed some light into the situation:
* Are you eventually going to follow OS date/time formatting settings?
* For which Gecko version is this going to land?
* What state will we have in-between?
* Which APIs are affected, which are not?

> the net of interdependencies is complicated

OK, but then you can only commit once everything is done. You apparently knew about the complexity from the start. You cannot leave the application in a half-done and broken state.

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

> Excuse me? Why? Because you disagree? I am not allowed to disagree with you?

You're allowed to disagree with me all you want. You're not allowed to speak to me as if you were in a position of power. In particular phrases such as "you must do X" or "this cannot be shipped" are reserved for decision makers.

> You keep saying that it's not an issue.

I'm not saying it's not an issue. I'm working on patches to fix it. I believe there's no better way to fix it than to write a patch.

> * Are you eventually going to follow OS date/time formatting settings?

Yes, hopefully the patches will land this week but that depends on reviewers.

> * For which Gecko version is this going to land?

55

> * What state will we have in-between?

I don't understand the question

> * Which APIs are affected, which are not?

DateTimeFormat.cpp and mozIntl.DateTimeFormat.

Comment 10

[Ben Bucksch (:BenB)]

> > * Are you eventually going to follow OS date/time formatting settings?

> Yes

OK, good, thanks. The "expected result" in bug 1342753 led me to believe that you wanted to abolish this concept.
This is good.

That leaves only the question about Gecko 54. The state is broken there. Can bug 1342753 be reverted for 54 only? Or another fix be found? Or the 55 patches be uplifted?

I still think that Gecko 54 is in a state that's not releasable. (I hope you find that formulation less objectionable.) So, I wonder what could be done about that. Please note that your fix in bug 1354339 fixes only US vs. AU, but not German vs. US. In Germany, we use "1.4.2017, 19:00", not "Apr 1, 2017, 7 PM".

Comment 11

[Ben Bucksch (:BenB)]

It is a relieve to see that you do want to follow OS settings. That wasn't clear from earlier statements.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

I'm not sure whether release management needs to track this bug or not, or if it is useful as it is described here.

Are there follow up bugs we should be filing (for example, from comment 10)?  If so, let's file specific bugs for new issues and close this one.

And, should this change have a release note (if not for this bug, for something that landed in 54?)

Comment 13

[Zibi Braniecki [:zbraniecki][:gandalf]]

> Are there follow up bugs we should be filing (for example, from comment 10)?

I don't think so.

I'm still investigating further fine-tuning of cross-locale intl matching on Windows across languages, but that's 55 material.

I don't believe there's any straight way to backport the work in 55 to 54, and since most of the affected reports are on Thunderbird/Lightning and that one will be released with 55, I don't think it's worth investing time to write new patches for 54.

> And, should this change have a release note (if not for this bug, for something that landed in 54?)

The changes landed in 53, so I think it would make sense for 53 rather than 54.

Comment 14

[Ben Bucksch (:BenB)]

Liz,
this changes how dates and times are displayed for international users, specifically non-English people who use English Firefox and Thunderbird builds. For them, this will now show the wrong date.

Zibi thinks the implementation is correct as-is. For him, the date format should match the Firefox build language.
However, it goes against almost every application on almost every operating system, all the way back to Windows 2.0, GeoWorks and Mac, up to most current Windows 10 and macOS Sierra. All these have *separate* settings for region and date/time formats that can be changed independently from the UI language. And almost all native applications on all OS follow that OS region settings and not the app language. 

That's also how Firefox and Thunderbird behaved so far. With the recent changes, Firefox ignores the OS date/time format settings and uses the one that matches the Firefox language pack (with the exception of UK/US English variations). That's apparently an intentional by Zibi, but breaks people in German, France, Scandinavia and almost every other non-US/UK country, if you use English Firefox builds. I think there are many people around the world who are affected by this.

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

:BenB - I opened up a discussion about the current state and direction in bug 1366134.

I hope I represented your request (follow the Windows regional settings), but if you think I did not, please, contribute to the conversation. I focused on Firefox UI, since it's much easier to get people's attention in the context of the product they work on :)

If we end up deciding not to follow the Microsoft products model in Firefox and stick to what we're doing now, or even close further, we can still do this in Thunderbird - it will be easy to make it a pref that you can switch and I believe once we decide on the Firefox UX, we can talk about Thunderbird following or diverging from it.

Comment 16

[Ben Bucksch (:BenB)]

Zibi, thanks for making room for TB to diverge. The problem is a 10 times more prominent in TB, where it's right in the main window, than in Firefox, where dates are only in some accessory windows.

Comment 17

[Zibi Braniecki [:zbraniecki][:gandalf]]

Yeah, I realize that.

Technically, whichever decision we'll make (and Thunderbird will make), the implementation is all within a single function - http://searchfox.org/mozilla-central/source/intl/locale/windows/OSPreferences_win.cpp#92

The challenge will come from SpiderMonkey. At the moment we assign "DefaultLocale" for each JS Context and it's the same locale as the App Locale.

Then, when we look into OS prefs, we check if the language portion matches and if it does, we use OS prefs, if it doesn't we use CLDR data.

If we'd want to follow Windows regional settings, we would need to solve the problem of - what is the default JS context locale?

Because if Firefox is in en-US, and default locale in JS context is en-US, then it would be awkward that

Intl.DateTimeFormat(undefined).resolvedOptions().locale == "en-US"; // we didn't look into OS prefs
Intl.UnitFormat(undefined).resolvedOptions().locale == "en-US"; // we took the locale from JS context locale
Intl.Collator(undefined).resolvedOptions().locale == "en-US"; // we took the locale from JS context locale
Intl.NumberFormat(undefined).resolvedOptions().locale == "en-US"; // we took the locale from JS context locale
Intl.PluralRules(undefined).resolvedOptions().locale == "en-US"; // we took the locale from JS context locale

but:

Intl.DateTimeFormat(undefined, { dateStyle: "short"}).resolvedOptions().locale == "de"; // we looked into OS prefs and took Windows user locale

Not sure how to solve this inconsistency.