Last Comment Bug 358124 - Replying to mail with plaintext attachment quotes the attachment
: Replying to mail with plaintext attachment quotes the attachment
Status: RESOLVED FIXED
: fixed1.8.1.1
Product: MailNews Core
Classification: Components
Component: MIME (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: David :Bienvenu
:
Mentors:
Depends on: 358275 384599
Blocks:
  Show dependency treegraph
 
Reported: 2006-10-25 21:02 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2008-07-31 04:30 PDT (History)
9 users (show)
mscott: blocking‑thunderbird2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (1.41 KB, patch)
2006-10-27 13:36 PDT, David :Bienvenu
mscott: superreview+
mscott: approval‑thunderbird2+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2006-10-25 21:02:53 PDT
STEPS TO REPRODUCE:

1)  Create a 600KB text file (like a profile, say).
2)  Send it as an attachment to an email.
3)  Read this email in mailnews.
4)  Reply to this email.

EXPECTED RESULTS:  Mailnews quotes the actual email text, but NOT the huge attachment.

ACTUAL RESULTS:  The CPU is at 100% and the app is locked up for a minute or two as we quote the whole 600KB of attachment.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2006-10-25 21:05:07 PDT
Oh, you need to have quoting of replies enabled, of course.
Comment 2 David :Bienvenu 2006-10-26 17:13:09 PDT
yeah, I've always hated this, and we've always done it, for as long as I can remember. The idea is probably that since we display text attachments, we should quote them. Especially in the case where the message body is just a txt attachment. I'd be happy to get rid of this behavior, personally. The change would be in libmime, I believe.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2006-10-26 17:23:38 PDT
I can see an argument for it if the message body is just the txt attachment, yeah.  But in general, it seems kind of odd...
Comment 4 David :Bienvenu 2006-10-27 09:43:36 PDT
This turns out to be non-trivial because of the way libmime works. I think the place to fix this is in MimeInlineTextPlain_parse_eof - check if we're quoting, and don't output any text in the case where the part is not the message body. But figuring that out might be tricky, because we have to handle the various mime multipart cases. One possibility is to check if the part has a filename and isn't a top-level part.
Comment 5 David :Bienvenu 2006-10-27 13:36:57 PDT
Created attachment 243835 [details] [diff] [review]
proposed fix

this works for me...maybe I'm particularly sensitive to this bug, because I often get sent imap protocol logs, and replying to those messages is a big pain, but I think the benefits outweigh the risk of occasionally not quoting something the user might want quoted.
Comment 6 Scott MacGregor 2006-10-27 15:33:56 PDT
Comment on attachment 243835 [details] [diff] [review]
proposed fix

I'm on board with trying this new behavior out.
Comment 7 David :Bienvenu 2006-10-30 08:44:09 PST
fixed on trunk - nominating for tb 2, to keep on radar, but will let bake on trunk for a while.
Comment 8 Mike Cowperthwaite 2006-10-31 12:06:16 PST
Could this be behind bug 358938?  No particular reason for me to assume so other than my attempts at testing this fix kept putting the crash in my face.
Comment 9 David :Bienvenu 2006-10-31 12:18:48 PST
I doubt it - that crash has been around for weeks, or at least one that looks similar...
Comment 10 Mike Cowperthwaite 2006-11-18 11:29:24 PST
OK, I've turned off inline-spellchecking and so I'm not suffering from the crash in the trunk anymore.  I can verify this is fixed (TB 3a1-1031, Win2K), but bug 161775 still remains open for all the other displayed-inline attachment types.  I won't mark the bug Verified, pending the TB2.0 decision.
Comment 11 Ravi 2006-11-28 18:49:18 PST
This bug should apply not only to txt attachments but to images also? Currently image attachments too get quoted in a reply.
I guess the proposed fix works anyway for all mimetype not just txt?
Comment 12 Scott MacGregor 2006-12-14 15:41:58 PST
Comment on attachment 243835 [details] [diff] [review]
proposed fix

David, should we take this on the branch?
Comment 13 David :Bienvenu 2006-12-14 15:43:00 PST
I haven't heard any complaints on the trunk - so yeah, we should try this out on the branch.
Comment 14 Scott MacGregor 2006-12-20 10:50:04 PST
Comment on attachment 243835 [details] [diff] [review]
proposed fix

I've been enjoying this behavior on the trunk.
Comment 15 David :Bienvenu 2006-12-20 10:51:53 PST
I already landed this, after beta 1.
Comment 16 Steve Lustbader 2007-05-10 07:57:53 PDT
Would it be possible to add an option to control this behavior?  I may be in the minority, but I actually liked the old behavior.
Comment 17 steeve 2007-06-15 14:00:07 PDT
I have to agree with Mr Lustbader, I miss this behaviour, it would be nice to have some about:config goodness to restore the original behaviour.
Comment 18 Andrew Culver 2008-01-11 13:15:22 PST
I'd like to voice my support for making this a configurable feature. I handle support tickets which our tracking system escalates to us by email with the ticket information as a text attachment. Since upgrading to 2.0, replying to the tickets doesn't automatically carry the ticket information anymore. I'd really like to have the option of quoting the attachment in my reply.
Comment 19 rsx11m 2008-01-11 17:19:06 PST
There is an enhancement request (bug 384599) pending to introduce this option.

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