386334 - make the format of the date line in message pane configurable

Status: NEW

(MailNews Core :: Localization, enhancement)

Description

[Christian Eyrich]

Attachment: proposed patch

The format of the date column in the thread pane is configurable via the mail.ui.display.dateformat.* prefs. I was asked to make this configurable and here's my proposal.

The patch is a twofold change. Firstly the date string now can in one of three instead of two formats (added format for same week) like in the thread pane. Secondly it's configurable using a set of new prefs mail.ui.display.dateformat_header.*. If a pref from this branch is not set, it's tried to be retrieved from the dateformat.* branch before falling back to the default.

The main ingredients of this patch have been copied over from nsMsgDBView.cpp like it has been done with the code for nsMimeHtmlDisplayEmitter::GenerateDateString() in the past.
Question is if the common code from these fuction should be sourced out to a common function. Also some portion from nsMsgGroupView::AllocHashKeyForHdr() is the same.

Comment 1

[Ludovic Hirlimann [:Usul]]

Jonathan what was the bug you fixed regarding dates ?

Comment 2

[Jonathan Watt [:jwatt]]

bug 360018

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 270330 [details] [diff] [review]
proposed patch

Patch has unfortunately bitrotted:

$ patch -p1 --dry-run < ~/Desktop/attachment.cgi 
patching file mime/emitters/src/nsMimeHtmlEmitter.cpp
Hunk #1 succeeded at 122 (offset 42 lines).
Hunk #2 FAILED at 324.
Hunk #3 FAILED at 390.
2 out of 3 hunks FAILED -- saving rejects to file mime/emitters/src/nsMimeHtmlEmitter.cpp.rej
patching file mime/emitters/src/nsMimeHtmlEmitter.h
Hunk #1 FAILED at 87.
1 out of 1 hunk FAILED -- saving rejects to file mime/emitters/src/nsMimeHtmlEmitter.h.rej

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Christian, any chance of a new patch?

Comment 5

[Christian Eyrich]

Attachment: proposed path v2

Updated patch, it now also gives consistent results when the pref mailnews.display.date_senders_timezone is true.
But please test this someone with some edge cases of dates for the today, this week and older.
And please give me some feedback on the new prefs.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #5)
> Created an attachment (id=385641) [details]
> proposed path v2
> 
> Updated patch, it now also gives consistent results when the pref
> mailnews.display.date_senders_timezone is true.
> But please test this someone with some edge cases of dates for the today, this
> week and older.
> And please give me some feedback on the new prefs.

Christian, please request a review from any of the reviewers: http://www.mozilla.org/mailnews/review.html

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 385641 [details] [diff] [review]
proposed path v2

Sorry I've not got to this. I think David knows mime and date type things a bit more, so I'm going to pass it to him and hope he's quicker than me.

Comment 8

[David :Bienvenu]

Christian, thx for the patch. We no longer need to use the LL macro's so that should get cleaned up in this patch...I'll try to look at this in a bit...

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 385641 [details] [diff] [review]
proposed path v2

Patch can still be applied! :) bienvenu, review-ping?

$ patch -p1 --dry-run < ~/Desktop/p386334.diff 
patching file mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp
Hunk #1 succeeded at 121 (offset -1 lines).
Hunk #2 succeeded at 316 (offset -1 lines).
Hunk #3 succeeded at 404 (offset -1 lines).
patching file mailnews/mime/emitters/src/nsMimeHtmlEmitter.h

Comment 10

[David :Bienvenu]

Comment on attachment 385641 [details] [diff] [review]
proposed path v2

patch is still using the LL macros per my previous comment, and shouldn't.

don't need to init rv here:
+  nsresult rv = NS_OK;

seems like this code could be shared with the view code that has to do similar things (but doesn't do the GMTCompTimeShift stuff)

Comment 11

[Ludovic Hirlimann [:Usul]]

Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.