Closed Bug 714265 Opened 12 years ago Closed 12 years ago

Encoding not retained in GatherMimeAttachments

Categories

(MailNews Core :: Composition, defect)

defect
Not set
normal

Tracking

(thunderbird10-, thunderbird11-)

RESOLVED FIXED
Thunderbird 12.0
Tracking Status
thunderbird10 - ---
thunderbird11 - ---

People

(Reporter: standard8, Assigned: aceman)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

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.
Attached patch The fix (obsolete) — Splinter Review
David, can you assess the likely impact of not having this fix?
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #584949 - Flags: review?(dbienvenu)
Attachment #584949 - Flags: review?(dbienvenu) → review+
(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.
Should the variable be removed?
Attached patch removal of the code (obsolete) — Splinter Review
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.
Attached patch removal v2Splinter Review
Attachment #588992 - Attachment is obsolete: true
Attachment #588992 - Flags: review?(dbienvenu)
Attachment #589008 - Flags: review?(dbienvenu)
Assignee: mbanner → acelists
Comment on attachment 589008 [details] [diff] [review]
removal v2

looks good, thx.
Attachment #589008 - Flags: review?(dbienvenu) → review+
Keywords: checkin-needed
Attachment #584949 - Attachment is obsolete: true
Checked in: http://hg.mozilla.org/comm-central/rev/7cf8ce4693a4
Status: ASSIGNED → RESOLVED
Closed: 12 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.

Attachment

General

Created:
Updated:
Size: