954568 - Log date is not translated

Status: RESOLVED FIXED

(Instantbird Graveyard :: Localization, defect)

Description

[Instantbot]

*** Original post on bio 1135 by Michal Stanke <michal.stanke AT mikk.cz> at 2011-11-02 16:14:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***

Comment 1

[Instantbot]

Attachment: screenshot

*** Original post on bio 1135 as attmnt 969 by michal.stanke AT mikk.cz at 2011-11-02 16:14:00 UTC ***

Days and months are shown in English.

Comment 2

[Instantbot]

*** Original post on bio 1135 by Tomáš Komárek <tomaskom.cz AT seznam.cz> at 2011-11-02 17:00:29 UTC ***

May I add, this is obviously Czech translation, and I have it in English too. Though I visited history several times, I just didn't notice it is in english :-D

Comment 3

[Patrick Cloke [:clokep]]

Attachment: Fix v1

*** Original post on bio 1135 as attmnt 1124 at 2012-01-17 00:34:00 UTC ***

Easy fix, just use toLocaleString, which we use in other bits of the UI as well too.

Comment 4

[Benedikt Pfeifer]

Attachment: Fix v2

*** Original post on bio 1135 as attmnt 1300 at 2012-04-05 17:08:00 UTC ***

toLocaleTimeString only shows the time part, not the full date. I updated it to what I think you initially meant to write and also r+ed it, if flo doesn't mind.

I tried with de-DE as locale of the application and it showed the german format for the time in the log list, so I think that counts as 'works'.

Comment 5

[Benedikt Pfeifer]

Comment on attachment 8352867 [details] [diff] [review]
Fix v1

*** Original change on bio 1135 attmnt 1124 at 2012-04-05 17:08:08 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 6

[Patrick Cloke [:clokep]]

*** Original post on bio 1135 at 2012-04-05 17:09:55 UTC ***

(In reply to comment #3)
> Created attachment 8353053 [details] [diff] [review] (bio-attmnt 1300) [details]
> Fix v2
> 
> toLocaleTimeString only shows the time part, not the full date. I updated it to
> what I think you initially meant to write and also r+ed it, if flo doesn't
> mind.
> 
> I tried with de-DE as locale of the application and it showed the german format
> for the time in the log list, so I think that counts as 'works'.

Mic that's probably what I meant, yes. And you can certainly test it better than I can! :) Thanks.

Comment 7

[Benedikt Pfeifer]

*** Original post on bio 1135 at 2012-04-05 17:18:33 UTC ***

(In reply to comment #3)
> Created attachment 8353053 [details] [diff] [review] (bio-attmnt 1300) [details]
> Fix v2
> 
> toLocaleTimeString only shows the time part, not the full date. I updated it to
> what I think you initially meant to write and also r+ed it, if flo doesn't
> mind.
> 
> I tried with de-DE as locale of the application and it showed the german format
> for the time in the log list, so I think that counts as 'works'.

Turns out that was the wrong thing to do (since my OS is set to german): flo reminded me on IRC that "toLocale..." converts the date into the format that the _OS_ shows for its currently selected locale, not the applications locale. That means you would get czech dates in an english Instantbird just by running it on an OS that has czech set as its locale. Changing the locale of Ib doesn't have any effect then.

Maybe it's still better to show the date in the OS locale than in english in general?

Comment 8

[Benedikt Pfeifer]

*** Original post on bio 1135 at 2012-04-05 17:23:08 UTC ***

19:10	flo	Mic|web: well, the patch is obviously wrong. But I've nothing better to propose, and I think it's less wrong than not changing anything
19:10	flo	JS Date .toLocale* familly of strings puts the string in the OS's locale, rather than the application locale.
19:11		clokep_work wonders how lightning deals with this...
19:11	flo	so the proposed patch improves the situation for users who have a non-en-US locale and have the OS and Instantbird setup in the same locale



I'm keeping the patch as it currently is then and leave it up to you whether it's going to be checked in or not.

Comment 9

[Benedikt Pfeifer]

Attachment: Fix v3

*** Original post on bio 1135 as attmnt 1356 at 2012-04-14 17:28:00 UTC ***

This time it's getting the locale from the pref "general.useragent.locale" (I haven't found an API to read out the current locale of the app) and uses a service to get the date-time format for this locale. Will still depend on the OS knowing how to format dates for that locale, though.

Worked fine (afaict) when setting the locale to de-DE, en-US, fr-FR, sv-SE on a german Windows XP SP3.

Comment 10

[Benedikt Pfeifer]

Comment on attachment 8353053 [details] [diff] [review]
Fix v2

*** Original change on bio 1135 attmnt 1300 at 2012-04-14 17:28:55 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 11

[Benedikt Pfeifer]

*** Original post on bio 1135 at 2012-04-14 17:30:42 UTC ***

Maybe we should export the "format date in application locale" code into a utility module? It could be useful elsewhere. I remember a bug about needing localized time strings in message themes?

Comment 12

[Florian Quèze [:florian]]

Comment on attachment 8353109 [details] [diff] [review]
Fix v3

*** Original change on bio 1135 attmnt 1356 at 2012-05-04 20:56:57 UTC ***

>+      let localizedDateTimeString = dts.FormatDateTime(appLocaleCode,
>+                                                       dts.dateFormatLong,
>+                                                       dts.timeFormatNoSeconds,
>+                                                       logDate.getFullYear(),
>+                                                       logDate.getMonth(),

logDate.getMonth() + 1

>+                                                       logDate.getDate(),
>+                                                       logDate.getHours(),
>+                                                       logDate.getMinutes(),
>+                                                       logDate.getSeconds());

You don't need to provide the value for seconds as you request that they aren't displayed.

Although I'm not completely happy with this patch (at least on Mac the day of the week is no longer displayed), I think it's still a significant improvement for all users of localized builds so I'm going to check it in (with the 2 details mentioned above fixed, and some minor style change).

I think at some point we will want to further improve this date display to show the time for today, "yesterday" + time for yesterday, the day of the week for the last 7 days, and the date otherwise.
There's some example code at http://mxr.mozilla.org/comm-central/source/mailnews/base/util/templateUtils.js#49 and http://mxr.mozilla.org/comm-central/source/mozilla/mobile/xul/chrome/content/downloads.js#344 but unfortunately both use .toLocaleFormat :(

Comment 13

[Florian Quèze [:florian]]

*** Original post on bio 1135 at 2012-05-04 23:34:19 UTC ***

https://hg.instantbird.org/instantbird/rev/0e3104136068

Comment 14

[Patrick Cloke [:clokep]]

*** Original post on bio 1135 at 2012-05-05 13:29:43 UTC ***

(In reply to comment #9)
> I think at some point we will want to further improve this date display to show
> the time for today, "yesterday" + time for yesterday, the day of the week for
> the last 7 days, and the date otherwise.
> There's some example code at
> http://mxr.mozilla.org/comm-central/source/mailnews/base/util/templateUtils.js#49
> and
> http://mxr.mozilla.org/comm-central/source/mozilla/mobile/xul/chrome/content/downloads.js#344
> but unfortunately both use .toLocaleFormat :(

Filed bug 954860 (bio 1425) for this. :)