358124 - Replying to mail with plaintext attachment quotes the attachment

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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 [:bzbarsky]]

Oh, you need to have quoting of replies enabled, of course.

Comment 2

[David :Bienvenu]

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 [:bzbarsky]]

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]

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]

Attachment: 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]

Comment on attachment 243835 [details] [diff] [review]
proposed fix

I'm on board with trying this new behavior out.

Comment 7

[David :Bienvenu]

fixed on trunk - nominating for tb 2, to keep on radar, but will let bake on trunk for a while.

Comment 8

[Mike Cowperthwaite]

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]

I doubt it - that crash has been around for weeks, or at least one that looks similar...

Comment 10

[Mike Cowperthwaite]

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]

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]

Comment on attachment 243835 [details] [diff] [review]
proposed fix

David, should we take this on the branch?

Comment 13

[David :Bienvenu]

I haven't heard any complaints on the trunk - so yeah, we should try this out on the branch.

Comment 14

[Scott MacGregor]

Comment on attachment 243835 [details] [diff] [review]
proposed fix

I've been enjoying this behavior on the trunk.

Comment 15

[David :Bienvenu]

I already landed this, after beta 1.

Comment 16

[Steve Lustbader]

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 McCauley]

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]

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]

There is an enhancement request (bug 384599) pending to introduce this option.