391810 - .eml files should always be saved with CRLF line endings

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Kaspar]

Attachment: Proposed patch

When saving a message in .eml format - which is supposed to reflect the message/rfc822 MIME type, I assume -, MailNews doesn't currently use canonical line endings except on platforms where MSG_LINEBREAK is defined as CRLF (i.e. Windows and OS/2).

The attached patch fixes this by setting the appropriate boolean parameter when calling SaveMessageToDisk(). For Mailbox URLs, the patch is already sufficient, while for IMAP and NNTP URLs it depends on bug 320102 attachment 272053 [details] [diff] [review], additionally. Since that one is proposed for Tb 2.0.0.7, it would be nice to see this fix here applied at the same time, too (didn't dare to set the approval1.8.1.7 flag yet, though...). Thanks!

Comment 1

[David :Bienvenu]

I'm confused - the subject of the bug says that .eml files should always end with CRLF line endings, but this patch would make us use canonical endings, i.e., LF on mac and linux, iirc.

Comment 2

[Kaspar]

(In reply to comment #1)
> but this patch would make us use canonical endings, i.e., LF on mac and linux,

Errm, canonical ending refers CRLF (not to the platform specific endings), see e.g. http://lxr.mozilla.org/mozilla/source/mailnews/local/src/nsMailboxProtocol.cpp#641:

641           if (canonicalLineEnding)
642             rv = m_msgFileOutputStream->Write(CRLF, 2, &count);
643           else
644             rv = m_msgFileOutputStream->Write(MSG_LINEBREAK,
645             MSG_LINEBREAK_LEN, &count);

or http://lxr.mozilla.org/mozilla/source/mailnews/imap/src/nsImapProtocol.cpp#3400:

3400     else  // enforce canonical CRLF linebreaks
3401     {
3402       if (lineLength==0 || lineLength == 1 && cEndOfLine[-1] == '\n')
3403       {
3404         messageLine = CRLF;
3405         lineLength = 2;
3406       }

(etc.)

Comment 3

[David :Bienvenu]

Comment on attachment 276254 [details] [diff] [review]
Proposed patch

duh, you're right, sorry.

Comment 4

[Kaspar]

I guess this doesn't require approval1.9+ since it doesn't affect Firefox, is that correct?

Comment 5

[David :Bienvenu]

Comment on attachment 276254 [details] [diff] [review]
Proposed patch

adding sr.

Comment 6

[Reed Loden [:reed]]

Checking in mailnews/base/src/nsMessenger.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMessenger.cpp,v  <--  nsMessenger.cpp
new revision: 1.379; previous revision: 1.378
done