In a printed mail Thunderbird ignores the locale settings for the date generating month and day of week

RESOLVED FIXED in Thunderbird 3.3a1

Status

MailNews Core
MIME
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Frieder Engel, Assigned: Tobias Koenig)

Tracking

({intl})

Trunk
Thunderbird 3.3a1

Thunderbird Tracking Flags

(thunderbird3.1 wontfix)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; de-DE; rv:1.7.7) Gecko/20050414 Firefox/1.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; de-DE; rv:1.7.7) Gecko/20050414 Firefox/1.0.3

In a printed mail Thunderbird ignores the locale settings for the date.
The title (Date/Datum) is correct though, but the date itself is wrong (always 
English/American format)

Example:
 
English:
Date: 15 May 2005  ...

German:
Datum: 15 May 2005 ... should be: 15. Mai 2005

Reproducible: Always

Steps to Reproduce:
1. Set computers locale to German
2. Print any message
3.



Expected Results:  
Use the computers correct locale settings.

Comment 1

12 years ago
This is probably the same issue as bug 252508.
Just like to mention that the PrintingTools-extension now "solves" this error:
https://nic-nac-project.de/~kaosmos/printingtools-en.html

So you might have a look at it and at least it provides a workaround for people who experience this problem.
QA Contact: general
Duplicate of this bug: 252508
no dupes afaict
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: intl
Summary: In a printed mail Thunderbird ignores the locale settings for the date → In a printed mail Thunderbird ignores the locale settings for the date generating month and day of week

Comment 5

10 years ago
xref bug 107854 (for forwarding). It's the raw header, not wrong date per se.
Assignee: mscott → nobody
OS: Windows 2000 → All
Hardware: PC → All

Updated

10 years ago
Duplicate of this bug: 392552

Comment 7

9 years ago
the date is still not translated in 2.0.0.14.

The header fields From:, To: are translated in the view and printout and the date is formated for the local format when you read the E-Mail so it should be the same in the printout.
(Assignee)

Comment 8

9 years ago
Created attachment 323412 [details] [diff] [review]
Use internationalized date for printing

The attached patch fixes this bug by moving the GenerateDateString from nsMimeHtmlDisplayEmitter to nsMimeBaseEmitter and making use of it there in WriteHeaderFieldHTML() when the mailnews.display.original_date config option is set.
Attachment #323412 - Flags: superreview?(mkmelin+mozilla)
Attachment #323412 - Flags: review?(mkmelin+mozilla)

Comment 9

9 years ago
Comment on attachment 323412 [details] [diff] [review]
Use internationalized date for printing

Sorry, you need a mailnews backend reviewer for this.
See http://www.mozilla.org/owners.html#mail-and-news-backend

The patch should apply to trunk (CVS HEAD), so please make sure it does - looks like this was generated against the branch.
Attachment #323412 - Flags: superreview?(mkmelin+mozilla)
Attachment #323412 - Flags: review?(mkmelin+mozilla)
(Assignee)

Comment 10

9 years ago
Created attachment 324277 [details] [diff] [review]
previous patch ported against HEAD

This patch prints the date in internationalized format by moving the GenerateDateString method up in the inheritance hierarchy, so that MimeBaseEmitter can make use of it.
Attachment #323412 - Attachment is obsolete: true
Attachment #324277 - Flags: superreview?(dmose)
Attachment #324277 - Flags: review?(dmose)
Comment on attachment 324277 [details] [diff] [review]
previous patch ported against HEAD

In general, this looks good; just need a few tweaks:

> 
>+  if ( strcmp(field, "Date") == 0 )
>+  {

The method this is in is already long enough; making it even longer starts to get hard to read, especially given that this is special-case code.  Please push this down into a new method called from here.

>+    if ( !displayOriginalDate )
>+    {
>+      nsCAutoString convertedDateString;
>+      GenerateDateString(value, convertedDateString);

Check the return value, please!

Finally, please add yourself to the Contributors section of the license boilerplate of (at least) nsMimeBaseEmitter.cpp, if not the other files too.
Attachment #324277 - Flags: superreview?(dmose)
Attachment #324277 - Flags: superreview-
Attachment #324277 - Flags: review?(dmose)
Attachment #324277 - Flags: review-
Reassigning to Tobias, as he's been doing a fine job owning this bug...
Assignee: nobody → tokoe
(Assignee)

Comment 13

9 years ago
Created attachment 324444 [details] [diff] [review]
corrected version

Moved the convertion of date header in its own method, checked for result value of GetDateString() and added my email to the contributors section
Attachment #324277 - Attachment is obsolete: true
Attachment #324444 - Flags: review?(dmose)

Comment 14

8 years ago
any news/update on this?
next step?

Comment 15

8 years ago
The patch has bitrotted :(
Apologies for not having gotten to this sooner.  After we're out from under 3.0 code pressure, I'll unbitrot this up and review it for 3.1.  Alternately, if someone else wants to unbitrot sooner, bienvenu and/or BenB could also be good reviewers, though they're still somewhat busy with the 3.0 as well.
Created attachment 473464 [details] [diff] [review]
Updated patch

I've fixed the bitrot on this. From what I can tell it works fine.

There are try server builds here with this fix for anyone that wants to test:

http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tryserver-builds/bugzilla@standard8.plus.com-dfacd3858259/
Assignee: tokoe → bugzilla
Attachment #324444 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #324444 - Flags: review?(dmose)
Attachment #473464 - Flags: review+
Comment on attachment 473464 [details] [diff] [review]
Updated patch

Given this was just a bitrot fix, I've reviewed the patch and given it r+. However, as this is mime, I'd like David to take a quick look as well.
Attachment #473464 - Flags: review?(bienvenu)

Comment 19

7 years ago
Comment on attachment 473464 [details] [diff] [review]
Updated patch

prefer NS_ENSURE_SUCCESS(rv, rv); here:

+    mDateFormatter = do_CreateInstance(NS_DATETIMEFORMAT_CONTRACTID, &rv);
+    if (NS_FAILED(rv))
+      return rv;

I know this is just being moved, but I'd lose the commented out code:

+/*
+  else if (LL_CMP(currentTime, >, dateOfMsg))
+  {
+    PRInt64 microSecondsPerSecond, secondsInDays, microSecondsInDays;
+    LL_I2L(microSecondsPerSecond, PR_USEC_PER_SEC);
+    LL_UI2L(secondsInDays, 60 * 60 * 24 * 7); // how many seconds in 7 days.....
+    LL_MUL(microSecondsInDays, secondsInDays, microSecondsPerSecond); // turn that into microseconds
+
+    PRInt64 diff;
+    LL_SUB(diff, currentTime, dateOfMsg);
+    if (LL_CMP(diff, <=, microSecondsInDays)) // within the same week
+      dateFormat = kDateFormatWeekday;
+  }
+*/

these lines look awfully long:
+        PRInt32 senderoffset = (explodedMsgTime.tm_params.tp_gmt_offset + explodedMsgTime.tm_params.tp_dst_offset) / 60;
+        // append offset to date string
+        PRUnichar *tzstring = nsTextFormatter::smprintf(NS_LITERAL_STRING(" %+05d").get(), (senderoffset / 60 * 100) + (senderoffset % 60));

prefer NS_ENSURE_SUCCESS(rv, rv); here, which gets rid of the need for the if (prefBranch) line and the braces:

+  nsCOMPtr<nsIPrefBranch> pPrefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
+  if (pPrefBranch)
+  {
+    pPrefBranch->GetBoolPref("mailnews.display.original_date", &displayOriginalDate);
+  }

lose the spaces around !displayOriginalData, and rv is already declared above:

+  if ( !displayOriginalDate )
+  {
+    nsCAutoString convertedDateString;
+    nsresult rv = GenerateDateString(dateString, convertedDateString);
+    if (NS_SUCCEEDED(rv))

please add a comment in the header file to the effect that the caller needs to free the result of GetLocalizedDateString();.

r+ with those addressed.
Attachment #473464 - Flags: review?(bienvenu) → review+
(In reply to comment #19)
> prefer NS_ENSURE_SUCCESS(rv, rv); here, which gets rid of the need for the if
> (prefBranch) line and the braces:
> 
> +  nsCOMPtr<nsIPrefBranch> pPrefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID,
> &rv));
> +  if (pPrefBranch)
> +  {
> +    pPrefBranch->GetBoolPref("mailnews.display.original_date",
> &displayOriginalDate);
> +  }

That didn't quite work because the function returns char*, so I just removed the rv and the braces.
Created attachment 475494 [details] [diff] [review]
Updated patch v2

Updated patch ready for checkin which I'll do once the tree is a bit healthier.
Attachment #473464 - Attachment is obsolete: true
Keywords: checkin-needed
Assignee: bugzilla → nobody
Component: General → MIME
Product: Thunderbird → MailNews Core
QA Contact: general → mime
Version: unspecified → Trunk
Assignee: nobody → bugzilla
Checked in: http://hg.mozilla.org/comm-central/rev/75a2d4946290

Tobias, thanks for all the work you did on this bug, sorry it took so long to get it landed.
Assignee: bugzilla → tokoe
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
status-thunderbird3.1: --- → ?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a1
Attachment #475494 - Flags: approval-thunderbird3.1.6?
Attachment #475494 - Flags: approval-thunderbird3.1.7? → approval-thunderbird3.1.7+
Comment on attachment 475494 [details] [diff] [review]
Updated patch v2

Having just granted this approval, I'm not sure I should have done so really. It isn't a fix to stability or security, and we don't normally take basic fixes for those releases. This also isn't a regression.

Therefore denying approval for the stable branch. It will make the next major release whatever version that is.
Attachment #475494 - Flags: approval-thunderbird3.2a1?
Attachment #475494 - Flags: approval-thunderbird3.1.7-
Attachment #475494 - Flags: approval-thunderbird3.1.7+
status-thunderbird3.1: ? → wontfix
Attachment #475494 - Flags: approval-thunderbird3.2a1?

Updated

6 years ago
Depends on: 669385
You need to log in before you can comment on or make changes to this bug.