Encoding not retained in GatherMimeAttachments

RESOLVED FIXED in Thunderbird 12.0

Status

MailNews Core
Composition
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: standard8, Assigned: aceman)

Tracking

({regression})

Trunk
Thunderbird 12.0
regression

Thunderbird Tracking Flags

(thunderbird10-, thunderbird11-)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Created attachment 584949 [details] [diff] [review]
The fix

David, can you assess the likely impact of not having this fix?
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #584949 - Flags: review?(dbienvenu)

Updated

6 years ago
Attachment #584949 - Flags: review?(dbienvenu) → review+

Comment 2

6 years ago
(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.
Not tracking as we won't actually hit this code.
tracking-thunderbird10: ? → -
tracking-thunderbird11: ? → -
(Assignee)

Comment 4

6 years ago
Should the variable be removed?
(Assignee)

Comment 5

6 years ago
Created attachment 588992 [details] [diff] [review]
removal of the code
Attachment #588992 - Flags: review?(dbienvenu)
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.
(Assignee)

Comment 7

6 years ago
Created attachment 589008 [details] [diff] [review]
removal v2
Attachment #588992 - Attachment is obsolete: true
Attachment #588992 - Flags: review?(dbienvenu)
Attachment #589008 - Flags: review?(dbienvenu)
(Assignee)

Updated

6 years ago
Assignee: mbanner → acelists

Comment 8

6 years ago
Comment on attachment 589008 [details] [diff] [review]
removal v2

looks good, thx.
Attachment #589008 - Flags: review?(dbienvenu) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Attachment #584949 - Attachment is obsolete: true
Checked in: http://hg.mozilla.org/comm-central/rev/7cf8ce4693a4
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 12.0
You need to log in before you can comment on or make changes to this bug.