Last Comment Bug 384599 - want pref to quote text attachment in reply
: want pref to quote text attachment in reply
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Composition (show other bugs)
: Trunk
: All All
: -- enhancement with 2 votes (vote)
: mozilla1.9
Assigned To: rsx11m
:
:
Mentors:
: 522523 (view as bug list)
Depends on:
Blocks: 358124
  Show dependency treegraph
 
Reported: 2007-06-15 09:00 PDT by steeve
Modified: 2009-10-15 19:34 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch to make bug 161775 fix configurable (5.61 KB, patch)
2008-01-17 12:46 PST, rsx11m
mozilla: review+
dmose: superreview+
Details | Diff | Splinter Review

Description steeve 2007-06-15 09:00:16 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4
Build Identifier: version 2.0.0.4 (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
Comment 1 Magnus Melin 2007-06-15 13:38:45 PDT
Yes, that was considered a bug, fixed in bug 358124.
Comment 2 steeve 2007-06-15 13:58:37 PDT
Damn, I liked that bug^H^H^Hfeature :(
Comment 3 Andrew Culver 2008-01-11 13:16:03 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 4 rsx11m 2008-01-11 17:24:43 PST
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.
Comment 5 rsx11m 2008-01-17 12:46:57 PST
Created attachment 297599 [details] [diff] [review]
Patch to make bug 161775 fix configurable

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 6 rsx11m 2008-01-31 18:58:41 PST
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.
Comment 7 David :Bienvenu 2008-01-31 19:09:37 PST
Comment on attachment 297599 [details] [diff] [review]
Patch to make bug 161775 fix configurable

looks good, thx
Comment 8 rsx11m 2008-02-01 14:01:08 PST
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.
Comment 9 rsx11m 2008-03-22 10:32:47 PDT
Joshua: Should this bug block or depend on bug 421086 as it affects MIME code?
Scott: Ping for sr?
Comment 10 rsx11m 2008-04-09 05:51:32 PDT
Comment on attachment 297599 [details] [diff] [review]
Patch to make bug 161775 fix configurable

Reassigning superreview as discussed by e-mail (no apparent bitrotting).
Comment 11 Dan Mosedale (:dmose) 2008-05-28 12:19:17 PDT
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?
Comment 12 steeve 2008-05-28 12:28:13 PDT
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.
Comment 13 rsx11m 2008-05-28 12:31:56 PDT
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 14 Dan Mosedale (:dmose) 2008-05-28 17:12:47 PDT
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.

sr=dmose
Comment 15 rsx11m 2008-05-28 18:01:20 PDT
Thanks! Check-in for trunk then, please.
Comment 16 Mark Banner (:standard8, limited time in Dec) 2008-05-29 05:36:37 PDT
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
done
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
done
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
done
Checking in mailnews/mailnews.js;
/cvsroot/mozilla/mailnews/mailnews.js,v  <--  mailnews.js
new revision: 3.314; previous revision: 3.313
done
Comment 17 rsx11m 2008-05-29 06:08:19 PDT
(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.

-> FIXED
Comment 18 rsx11m 2008-07-13 22:34:42 PDT
See bug 441659 (SeaMonkey) and bug 445088 (Thunderbird) for the frontend.
Comment 19 rsx11m 2009-10-15 19:34:16 PDT
*** Bug 522523 has been marked as a duplicate of this bug. ***

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