Last Comment Bug 714265 - Encoding not retained in GatherMimeAttachments
: Encoding not retained in GatherMimeAttachments
Status: RESOLVED FIXED
: regression
Product: MailNews Core
Classification: Components
Component: Composition (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 12.0
Assigned To: :aceman
:
Mentors:
Depends on:
Blocks: 679476
  Show dependency treegraph
 
Reported: 2011-12-30 05:45 PST by Mark Banner (:standard8)
Modified: 2012-01-28 14:57 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
-
-


Attachments
The fix (721 bytes, patch)
2011-12-30 05:46 PST, Mark Banner (:standard8)
mozilla: review+
Details | Diff | Splinter Review
removal of the code (4.86 KB, patch)
2012-01-16 13:56 PST, :aceman
no flags Details | Diff | Splinter Review
removal v2 (6.56 KB, patch)
2012-01-16 14:18 PST, :aceman
mozilla: review+
Details | Diff | Splinter Review

Description Mark Banner (:standard8) 2011-12-30 05:45:07 PST
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) 2011-12-30 05:46:12 PST
Created attachment 584949 [details] [diff] [review]
The fix

David, can you assess the likely impact of not having this fix?
Comment 2 David :Bienvenu 2011-12-30 07:37:53 PST
(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) 2012-01-16 03:56:02 PST
Not tracking as we won't actually hit this code.
Comment 4 :aceman 2012-01-16 13:22:49 PST
Should the variable be removed?
Comment 5 :aceman 2012-01-16 13:56:29 PST
Created attachment 588992 [details] [diff] [review]
removal of the code
Comment 6 Mark Banner (:standard8) 2012-01-16 14:01:54 PST
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 2012-01-16 14:18:59 PST
Created attachment 589008 [details] [diff] [review]
removal v2
Comment 8 David :Bienvenu 2012-01-27 12:57:09 PST
Comment on attachment 589008 [details] [diff] [review]
removal v2

looks good, thx.
Comment 9 Mark Banner (:standard8) 2012-01-28 14:57:30 PST
Checked in: http://hg.mozilla.org/comm-central/rev/7cf8ce4693a4

Note You need to log in before you can comment on or make changes to this bug.