714265 - Encoding not retained in GatherMimeAttachments

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Mark Banner (:standard8)]

Regression from bug 679476:

http://hg.mozilla.org/comm-central/diff/9b1d6d96451a/mailnews/compose/src/nsMsgSend.cpp#l1.80

-        SNARF(attachments[i].type, ma->m_type);
-        SNARF(attachments[i].encoding, ma->m_encoding);
...
+        attachments[i].m_type = ma->m_type;
+        attachments[i].m_encoding, ma->m_encoding;

The comma should have been replaced.

I'm not quite sure of the visible affects of this bug, but I suspect this could affect us getting the correct encoding for attachments.

Comment 1

[Mark Banner (:standard8)]

Attachment: The fix

David, can you assess the likely impact of not having this fix?

Comment 2

[David :Bienvenu]

(In reply to Mark Banner (:standard8) (afk until 3rd Jan) from comment #1)
> Created attachment 584949 [details] [diff] [review]
> The fix
> 
> David, can you assess the likely impact of not having this fix?

I don't think this code is ever hit - we only hit it if m_attachments_only_p is true, and it's never set to true, from what I can tell. So we should clean up the code around that var.

Comment 3

[Mark Banner (:standard8)]

Not tracking as we won't actually hit this code.

Comment 4

[:aceman]

Should the variable be removed?

Comment 5

[:aceman]

Attachment: removal of the code

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 588992 [details] [diff] [review]
removal of the code

I think you should also be able to remove m_attachments_done_callback - that's pretty unused apart from in this section, and I think it never actually gets set by anyone.

Comment 7

[:aceman]

Attachment: removal v2

Comment 8

[David :Bienvenu]

Comment on attachment 589008 [details] [diff] [review]
removal v2

looks good, thx.

Comment 9

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/7cf8ce4693a4