1755961 - mozilla::intl::AppDateTimeFormat::Format() don't switch locale format with intl.multilingual.liveReload

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

[Richard Marti (:Paenglab)]

From bug 1755181 comment 4:

TB uses `mozilla::intl::AppDateTimeFormat::Format()` also used for certificate display in FF:
https://searchfox.org/mozilla-central/rev/41614c2fb53602e9a4c9793afa76f2422c0bf9a2/security/manager/ssl/nsCertTree.cpp#544
When you look at this bug, please check that certificate display follows the locale the user chose.

Let's not lump too much into one bug, it'll only cause things to be missed or confused. The fact that AppDataTimeFormat is still returning the wrong thing after a switch feels like it warrants a bug in itself - it is a specific issue that some of these others may depend on. The certificate display is probably best filed as a separate bug as well - then we have an explicit reminder to check it.

Comment 1

[Greg Tatum [:gregtatum]]

Marking as P2 enhancement until the feature goes live, then we'll switch to defects. Thanks for filing!

Comment 2

[Greg Tatum [:gregtatum]]

The AppDateTimeFormat cached components need to be invalidated on "intl:app-locales-changed".

Comment 3

[Greg Tatum [:gregtatum]]

Attachment: Bug 1755961 - Invalidate locale cache in AppDateTimeFormat; r?#platform-i18n-reviewers!

Depends on D140665

Comment 4

[Phabricator Automation]

Comment on attachment 9267061 [details]
Bug 1755961 - Invalidate locale cache in AppDateTimeFormat; r?#platform-i18n-reviewers!

Revision D140666 was moved to bug 1755181. Setting attachment 9267061 [details] to obsolete.

Comment 5

[Rachel Martin]

This can be closed now that bug 1755181 is done?

Comment 6

[Greg Tatum [:gregtatum]]

Attachment: image.png

The dialog on the left is the correct text.
The dialog on the right is the text with an incorrect text.

The dialog on the left is what happens after opening the dialog after a live language switch.
The dialog on the right is what happens when opening a dialog first, then doing a live language switch. Closing and reopening the dialog gets it working again.

I think this is good enough to close this issue, there is still an issue with the date, but it is quickly resolved by reopening it. This seems like a much lower priority compared to the original issue.

Comment 7

[Greg Tatum [:gregtatum]]

The issue in Bug 1755181 did fix the underlying issue, but the nsCertTree.cpp would also need to listen to the "intl:app-locales-changed" and rebuild itself.

Comment 8

[Rachel Martin]

Greg, are you sure this bug is fixed as reported?
mozilla::intl::AppDateTimeFormat::Format() don't switch locale format with intl.multilingual.liveReload

In Thunderbird, switching of the locale is observed and the message list (also called thread pane) is redrawn:
https://searchfox.org/comm-central/rev/8d258916e1367d6c1a08d7225781cde9fc769136/mail/components/MailGlue.jsm#358-369

Drawing of the date happens via mozilla::intl::AppDateTimeFormat::Format() here:
https://searchfox.org/comm-central/search?q=mozilla%3A%3Aintl%3A%3AAppDateTimeFormat%3A%3AFormat%28&path=nsMsgDBView.cpp&case=false&regexp=false

Observed behaviour is that after the local is switched, the dates are still drawn the same, no change from, for example, English dd/mm/yyyy to German dd.mm.yyyy format. Other parts of the tree are redrawn in the new language, especially the Status column which changes from "Read" to "Gelesen" (in German). Even if a new window with the same folder is launched where even the tree column headings update, the dates are still drawn in the same format. Without any debugging, that indicates that mozilla::intl::AppDateTimeFormat::Format() is still returning the same value as before the language switch.

At a cursory look, resetting sLocale might not be enough. Shouldn't sFormatCache also be deleted?
https://hg.mozilla.org/mozilla-central/rev/b61343e0db5a

Comment 9

[Greg Tatum [:gregtatum]]

Seems worth reopening. I'll leave the NI open on it as I don't have time to look at it right this second.

Comment 10

[Greg Tatum [:gregtatum]]

I'm marking it as P3 since it's blocking Bug 62174, and I doubt I'll get to this before Fx 100 merges.

Comment 11

[Rachel Martin]

This fixes the issue in TB:

diff --git a/intl/locale/AppDateTimeFormat.cpp b/intl/locale/AppDateTimeFormat.cpp
--- a/intl/locale/AppDateTimeFormat.cpp
+++ b/intl/locale/AppDateTimeFormat.cpp
@@ -254,8 +254,9 @@ void AppDateTimeFormat::Shutdown() {

 /*static*/
 void AppDateTimeFormat::ClearLocaleCache() {
   MOZ_ASSERT(NS_IsMainThread());
+  DeleteCache();
   delete sLocale;
   sLocale = nullptr;
 }

Comment 12

[Greg Tatum [:gregtatum]]

Attachment: Bug 1755961 - Delete the format cache in AppDateTimeFormat; r?#platform-i18n-reviewers!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2694f26a3025
Delete the format cache in AppDateTimeFormat; r=platform-i18n-reviewers,dminor

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/2694f26a3025