544359 - Replying to an attached message (message/rfc822 part) or an opened .eml file shows incorrect date

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Simon Montagu :smontagu]

Spun off from bug 522561

Steps to reproduce:
1. Send yourself an email.
1a. Wait till you receive it back in your inbox.

2. Send yourself another mail but now with the above mail as received as
attachment.
2a. Wait till you receive it back in your inbox 

There are sample .eml files in attachment 423787 [details] or attachment 423785 [details] which can be used instead of the steps above.

3. Open the mail.
3a. Open the attachment message in the mail.
3b. In the message window that now opens, click on reply

Expected results:
The reply quotes the original message with the original date and time

Actual results:
The reply quotes the original message with "On 7/22/28164 11:59 AM" (or crashes on OS X without the fix for bug 522561)

Comment 1

[Wayne Mery (:wsmwk)]

I don't get same results on windows Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.7) Gecko/20100204 Shredder/3.0.2pre

compose window from reply has

  On 2:59 PM, Hayo Baan wrote:

Comment 2

[Simon Montagu :smontagu]

I see the bug in different forms on both Linux and OS X. On Linux it appears as 

 On 01/-10/-28163 09:59 PM, Hayo Baan wrote:

Comment 3

[WADA:World Anti-bad-Duping Agency]

(1) mail structure
> Date: Fri, 05 Feb 2010 14:14:30 +0900
> Content-Type: multipart/mixed; boundary="------------060902080903090008040203"
>
> --------------060902080903090008040203
> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
>
> --------------060902080903090008040203
> Content-Type: message/rfc822; name="Attached-Message-01"
> Date: Sat, Jan 01 00:00:00 1970 +0000
>
> --------------060902080903090008040203
> Content-Type: message/rfc822; name="Attached-Message02"
> Date: Sun, 27 Sep 2009 09:25:44 +0900
> --------------060902080903090008040203--

Reply test result at 2010/2/05 14:20 (GMT+09:00, JST) with Tb 3.0.1 on Win-XP.
(Subject, Sender is correctly set)

(2) Reply to original mail
> On 2010/02/05 14:14, b b wrote:

(3) Reply to Attached-Message-01
> On 04:59, c c wrote:

(4) Reply to Attached-Message-02
> On 04:59, d d wrote:

Quoting of date/time is done by next.
> mailnews.reply_header_ondate = On %s
%s seems to see garbage instead of Date: header in message/rfc822 part.

On MS Win, yyyy/mm/dd from garbage looks today fortunately.
xx in xx:59 looks to depend on time zone, but 59 in xx:59 looks common among OS.

Comment 4

[Sidney Markowitz]

Attachment: much simplified example in mbox format

I've noticed an additional symptom and I've attached a very simplified test mbox file that demonstrates the problem. If I look at this message and doubleclick the eml attachment to view it in its own window, it looks like an ordinary mail message with the correct date, but View | Message Source shows HTML source, not the source of the email. Reply to that attached message causes the crash on my MacOS X system that indicates that it is using a bad date.

Note that this is not an HTML format message, so I don't know where that HTML in the view message source is coming from.

Comment 5

[Sidney Markowitz]

Attachment: Test case for even more simplified version of bug with no attachment

I reproduced the problem by simply using File | Open Saved Message to read in a simple eml file with no attachments involved (see attached example), then clicking on Reply.

Unlike the example in my previous comment View | Message Source does not show anything unusual. When I click on Reply it crashes on my unpatched Mac OS X version. In Ubuntu the quoted message in the compose window shows a date "-9/01/37 07:59"

Comment 8

[Rubén Martín [:Nukeador] ❌ [away till Aug 31st]]

Nobody is working on a fix? Need more information/tests?

Comment 9

[Rubén Martín [:Nukeador] ❌ [away till Aug 31st]]

The problem is still present in Thunderbird 5.0

Comment 12

[Jim Porter (:squib)]

Attachment: Fix this

Here's a patch for this; it turns out this is actually pretty simple. I'm not sure what the best way to test this is; I have a feeling it would be complicated, though. Any ideas?

Comment 13

[Rubén Martín [:Nukeador] ❌ [away till Aug 31st]]

The way to reproduce this problem is easy:

1. Save any mail as .eml
2. Open the eml file with Thunderbird.
3. Click on reply.
4. The quoted date will be shown for example like this: -10/01/37 20:59

Comment 14

[Jim Porter (:squib)]

Checked in: http://hg.mozilla.org/comm-central/rev/0d2fa760d840

Comment 15

[Rubén Martín [:Nukeador] ❌ [away till Aug 31st]]

Great, thanks for the fix. Any reason we have to wait till Thunderbird 10? ;)

Comment 16

[Jens Hatlak (:InvisibleSmiley)]

I don't know for sure how TB works in this case and I don't have a build containing this fix at hand, but when I applied the patch to SeaMonkey (which is almost equal code-wise at the patched positions), it didn't work for me because header.headerValue was equal to the value displayed in the UI, which is German for me (which has a different date format). Also it failed for messages from today where the short format (time only) was used. I guess in both cases Date.parse returned NaN.

Did you (patch author, reviewer) check the above cases with TB?

Comment 17

[Jens Hatlak (:InvisibleSmiley)]

(Note: I was referring to the OS locale, in this case Kubuntu. I'm otherwise testing with en-US builds. Also I manually set mailnews.reply_header_type=2 to match TB's default before testing.)

Comment 18

[Jim Porter (:squib)]

Further proof that libmime is why we can't have nice things (it munges the date before we get it). Still, that part is easy to fix, so we probably don't need to back this out just yet...

Comment 19

[Jim Porter (:squib)]

My plan here is to have the MIME emitter emit 2 headers for the date: the original date ("Date"), and the localized date ("X-Mozilla-LocaleDate"). This part is easy, but I'm not sure what we should do in msgHdrViewOverlay.js. I could either make the "date" field correspond to X-Mozilla-LocaleDate, which would cause "all headers" mode to show the date field twice, or I could just add X-Mozilla-LocaleDate to the currentHeaders dictionary as "Date" instead and ignore the raw date. I don't think we actually need the raw Date header, since we could get that info from the nsIMsgDBHdr. Ideas?

Comment 20

[Jim Porter (:squib)]

Attachment: Do the latter from comment 19

This should be applied in addition to the previous attachment and inserts "X-Mozilla-LocaleDate" into currentHeaders as "date".

Comment 21

[Jim Porter (:squib)]

Comment on attachment 564123 [details] [diff] [review]
Do the latter from comment 19

Callek, hopefully you're the right person to ask for review for the Seamonkey side of this. If not, feel free to pass it on to whoever.

Comment 22

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 564123 [details] [diff] [review]
Do the latter from comment 19

The SM MailNews reviewer is Karsten, switching requestee.

You forgot the "date: 0," init for SM (the patch alone also works in my test case but it should be cleaner).

Warning: Mnenhy 0.8.4 badly breaks all of this; you need to deactivate it for testing.

My test case was:
1. Start composing a message, save it (Ctrl+S)
2. Start composing a second message, attach the first one from Drafts and save this one, too
3. Open the message attached to the second message in Drafts
4. Hit Reply (Ctrl+R)

In the above case, with HTML composition, Mnenhy not only gets the date wrong (where with the patch but without Mnenhy the date is correct) but also displays the HTML code instead of the result of its interpretation (of course that's not to be fixed here, only for reference).

>+        if (lowerCaseHeaderName == "date")
>+          continue;
>+        else if (lowerCaseHeaderName == "x-mozilla-localedate")
>+          lowerCaseHeaderName = "date";

Strictly this doesn't need an "else". ;-)

Comment 23

[David :Bienvenu]

(In reply to Jim Porter (:squib) from comment #19)
> My plan here is to have the MIME emitter emit 2 headers for the date: the
> original date ("Date"), and the localized date ("X-Mozilla-LocaleDate").
> This part is easy, but I'm not sure what we should do in
> msgHdrViewOverlay.js. I could either make the "date" field correspond to
> X-Mozilla-LocaleDate, which would cause "all headers" mode to show the date
> field twice, or I could just add X-Mozilla-LocaleDate to the currentHeaders
> dictionary as "Date" instead and ignore the raw date. I don't think we
> actually need the raw Date header, since we could get that info from the
> nsIMsgDBHdr. Ideas?

With the patch, I'm seeing x-mozilla-local-date in all headers view. Is it hard/impossible to prevent that? Other than that, the patch seems to work OK.

Comment 24

[Jim Porter (:squib)]

(In reply to David :Bienvenu from comment #23)
> (In reply to Jim Porter (:squib) from comment #19)
> With the patch, I'm seeing x-mozilla-local-date in all headers view. Is it
> hard/impossible to prevent that? Other than that, the patch seems to work OK.

This should be easy; I just forgot to update the name in one spot. However, I'm not 100% what the best way is here, now that I think about it. We could 1) eliminate the date field in All Headers, since it's right below the message header buttons, 2) print the localized date field, or 3) print the raw date field. Any preference? I'm leaning towards (3) or maybe (1), since (2) is just redundant.

Of course, this is all different for Seamonkey, so we'd just print the localized date field there.

Comment 25

[David :Bienvenu]

(In reply to Jim Porter (:squib) from comment #24)

> This should be easy; I just forgot to update the name in one spot. However,
> I'm not 100% what the best way is here, now that I think about it. We could
> 1) eliminate the date field in All Headers, since it's right below the
> message header buttons, 2) print the localized date field, or 3) print the
> raw date field. Any preference? I'm leaning towards (3) or maybe (1), since
> (2) is just redundant.

My bias is towards 3, since I think of show all headers as what you do when you want to see all headers, exactly as they appear in the message.

Comment 26

[Jim Porter (:squib)]

Attachment: Show the raw Date field in All Headers mode in Thunderbird

Ok, here we go. This patch uses method (3) from comment 24 for Thunderbird and method (2) for Seamonkey, since Seamonkey doesn't have the extra date label that Thunderbird does. I could probably come up with a way to write tests for this (at least the reading part; the replying part might be harder) if you feel it's necessary.

Comment 27

[David :Bienvenu]

Comment on attachment 564753 [details] [diff] [review]
Show the raw Date field in All Headers mode in Thunderbird

r=me for the thunderbird part. I don't see the x-mozilla-local-date header in all headers mode, and reply from a .eml puts in the right date.

Comment 28

[Karsten Düsterloh]

Comment on attachment 564753 [details] [diff] [review]
Show the raw Date field in All Headers mode in Thunderbird

>+++ b/suite/mailnews/msgHdrViewOverlay.js
>+        else if (lowerCaseHeaderName == "x-mozilla-localedate") {

r=me if you drop the unnecessary "else" token and move the "{" onto the next line.

Comment 29

[Jens Hatlak (:InvisibleSmiley)]

Jim, will you take care of the "date: 0" init for SM or is that obsolete/irrelevant?

Comment 30

[Jim Porter (:squib)]

Checked in with comments 28 and 29 fixed (I also changed X-Mozilla-LocaleDate to X-Mozilla-LocalizedDate, since that's a bit more grammatical):

http://hg.mozilla.org/comm-central/rev/90841458f49f

Also, to respond to comment 15, I'm not going to nominate this for aurora (or beta), since I managed to get this wrong the first time around. This could probably use a bit more baking time just in case something else goes wrong.