Closed Bug 358124 Opened 14 years ago Closed 14 years ago

Replying to mail with plaintext attachment quotes the attachment

Categories

(MailNews Core :: MIME, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: Bienvenu)

References

(Depends on 1 open bug)

Details

(Keywords: fixed1.8.1.1)

Attachments

(1 file)

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.
Oh, you need to have quoting of replies enabled, of course.
Depends on: 358275
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.
Component: MailNews: Composition → MailNews: MIME
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...
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.
Attached patch proposed fixSplinter Review
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.
Assignee: nobody → bienvenu
Status: NEW → ASSIGNED
Attachment #243835 - Flags: superreview?(mscott)
Comment on attachment 243835 [details] [diff] [review]
proposed fix

I'm on board with trying this new behavior out.
Attachment #243835 - Flags: superreview?(mscott) → superreview+
fixed on trunk - nominating for tb 2, to keep on radar, but will let bake on trunk for a while.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: blocking-thunderbird2?
Resolution: --- → FIXED
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.
I doubt it - that crash has been around for weeks, or at least one that looks similar...
OS: Linux → All
Hardware: PC → All
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.
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 on attachment 243835 [details] [diff] [review]
proposed fix

David, should we take this on the branch?
Attachment #243835 - Flags: approval-thunderbird2?
Flags: blocking-thunderbird2? → blocking-thunderbird2+
I haven't heard any complaints on the trunk - so yeah, we should try this out on the branch.
Keywords: fixed1.8.1.1
Comment on attachment 243835 [details] [diff] [review]
proposed fix

I've been enjoying this behavior on the trunk.
Attachment #243835 - Flags: approval-thunderbird2? → approval-thunderbird2+
I already landed this, after beta 1.
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.
Depends on: 384599
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.
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.
There is an enhancement request (bug 384599) pending to introduce this option.
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.