Closed Bug 384599 Opened 13 years ago Closed 12 years ago

want pref to quote text attachment in reply


(MailNews Core :: Composition, enhancement)

Not set


(Not tracked)



(Reporter: steeve.mccauley, Assigned:




(1 file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv: Gecko/20070515 Firefox/
Build Identifier: version (20070604)

In thunderbird version 1.5 when replying to a message with a text attachment, Content-type: text/, the contents of the attachment were included with the message.  This functionality now seems to be gone in thunderbird 2.0.0.x.

Reproducible: Always

Steps to Reproduce:
1. select message with text attachment
2 [review]. hit reply
3. contents of attachment not included in reply
Actual Results:  
Text attachment not included in reply

Expected Results:  
Text attachment included in reply
Yes, that was considered a bug, fixed in bug 358124.
Blocks: 358124
Severity: normal → enhancement
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
Summary: text attachment not included in reply → want pref to quote text attachment in reply
Version: unspecified → 2.0
Damn, I liked that bug^H^H^Hfeature :(
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.
This RFE has some merit. While attachments that can be inlined may be annoying when included in a reply, especially in the case of large text files (prompting that patch), there are other cases where routinely that information should be included in the reply. Thus, making the behavior configurable IMHO makes sense.

It appears that the fix for bug 358124 has been reversed and replaced by the fix for bug 161775. The initial patch was restricted to text/plain attachments (the current branch behavior), whereas the latter patch now removes all attachments in the trunk versions.
Gave it a try, this works for me.

The first part extents the patch for bug 161775 so that the additional conditions are only applied when the new "mail.reply_quote_inline" pref is false. If that preference is true, the behavior is the same as before the fix for bug 358124 was applied. Note that in this case, whether or not inlinable attachments are quoted in a reply still depends on the "Display Attachments Inline" setting.

The remainder of the patch is needed to introduce the new preference.

Any comments or suggestions?
Comment on attachment 297599 [details] [diff] [review]
Patch to make bug 161775 fix configurable

Same reviewers as for the original patch to maintain continuity.

There were quite a few comments in the other two bugs already, thus I'm flagging this for review now. Both reviewers had approved the patch for bug 161775 with some doubts if it was the right thing to do. Those concerns should be addressed by this extended patch. The default of not including any attachments into replies retains the intent of the original patch, while also providing a mechanism to include inlined attachment contents into a reply for users who want or need to do so.
Attachment #297599 - Flags: superreview?(mscott)
Attachment #297599 - Flags: review?(bienvenu)
Comment on attachment 297599 [details] [diff] [review]
Patch to make bug 161775 fix configurable

looks good, thx
Attachment #297599 - Flags: review?(bienvenu) → review+
Assignee: nobody →
Thanks David & Magnus.

Looking at the flags, I don't think this should stay specific for Thunderbird. The other two bugs were both Core bugs, and the patch is affecting Core modules. Thus, if nobody objects, I'll go ahead and change this respectively.
Component: Message Compose Window → MailNews: Composition
Product: Thunderbird → Core
Version: 2.0 → Trunk
QA Contact: message-compose → composition
Joshua: Should this bug block or depend on bug 421086 as it affects MIME code?
Scott: Ping for sr?
Comment on attachment 297599 [details] [diff] [review]
Patch to make bug 161775 fix configurable

Reassigning superreview as discussed by e-mail (no apparent bitrotting).
Attachment #297599 - Flags: superreview?(mscott) → superreview?(dmose)
I'm a bit leery of adding to the testing matrix with yet another pref here.  The only actual use case I see mentioned here (and in bug 358124) is arguably a bug in the ticketing system in question: shouldn't it be using "Content-Disposition: inline" for the attachment, which would result in Thunderbird doing the right thing as it stands today?  

Are the other interesting use cases here?
Is there anyway that thunderbird can mark an attachment as 'inline' when sending?  Not that I see.  We do remote code reviews, and patches are often sent in emails for review and we used to just do a reply so that we could comment inside the indented patch.  The alternative is to sve the attachment, edit the attachment and reply with the commented patch, whcih is what we're doing now.
Dan, it is my understanding that the ticketing system already has a correct
"Content-Disposition: inline" for the attachment, otherwise steeve wouldn't
have been able to use this with the old TB 1.5 behavior. Due to bug 358124,
and bug 161775 with the extension to image attachments, those are no longer
included in replies regardless of the content disposition. The proposed
patch here would allow the contents of those attachments to be included
into the reply as part of the message, therefore being able to edit or
place their replies inbetween the quotes.

Another use case frequently seen in the forums is that people received a
message/rfc822 attachment and want to include its contents when forwarding
or replying to the message. With bug 161775 landed, that's no longer a
possible option as well. Thus, I think we have some use cases here...
Comment on attachment 297599 [details] [diff] [review]
Patch to make bug 161775 fix configurable

steeve: good point, it might well be worth having UI of some sort to do that.  Apparently Eudora Classic has it.  File a spin-off bug?

rsx11: indeed, I had misunderstood part of the code when I made my suggestion.

Attachment #297599 - Flags: superreview?(dmose) → superreview+
Thanks! Check-in for trunk then, please.
Keywords: checkin-needed
Checking in mailnews/mime/src/mimei.cpp;
/cvsroot/mozilla/mailnews/mime/src/mimei.cpp,v  <--  mimei.cpp
new revision: 1.90; previous revision: 1.89
Checking in mailnews/mime/src/modlmime.h;
/cvsroot/mozilla/mailnews/mime/src/modlmime.h,v  <--  modlmime.h
new revision: 1.38; previous revision: 1.37
Checking in mailnews/mime/src/mimemoz2.cpp;
/cvsroot/mozilla/mailnews/mime/src/mimemoz2.cpp,v  <--  mimemoz2.cpp
new revision: 1.237; previous revision: 1.236
Checking in mailnews/mailnews.js;
/cvsroot/mozilla/mailnews/mailnews.js,v  <--  mailnews.js
new revision: 3.314; previous revision: 3.313
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9
(In reply to comment #14)
> steeve: good point, it might well be worth having UI of some sort to do that. 
> Apparently Eudora Classic has it.  File a spin-off bug?

Yes, if some UI is wanted for changing the "mail.reply_quote_inline" and "mail.content_disposition_type" preferences, this should probably be a
separate RFE. That would be application specific while the patch here has
covered the back-end part. Also see bug 65794 for the disposition issue.

Closed: 12 years ago
Resolution: --- → FIXED
See bug 441659 (SeaMonkey) and bug 445088 (Thunderbird) for the frontend.
Product: Core → MailNews Core
Duplicate of this bug: 522523
You need to log in before you can comment on or make changes to this bug.