215514 - email_in.pl should accept inline text attachments

Status: NEW

(Bugzilla :: Incoming Email, enhancement, P4)

Description

[Alex Schuilenburg]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)
Build Identifier: 

When submitting a patch along with a description as an email, mozilla sends 
mail as multipart/mixed as expected with the patch added as inline content 
disposition, type text.  bug_email simply appends the patch to the 
description.  While this may be acceptable for some, this is an inconvenience 
when attempting to extract a patch.  

This could be worked around by the mail application (e.g. in in mozilla by 
ensuring attachments are always sent as attachments by setting user_pref
("mail.content_disposition_type", 1); in .mozilla/.../prefs.js) but that 
impacts the user mail-reader.

My preferred approach is to take the first text portion of a multipart 
message, use that as the description, and then have the remaining parts be 
added as attachments.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Alex Schuilenburg]

Attachment: Patch to provide the behaviour desired

Comment 2

[Max Kanat-Alexander]

I would tend to agree, and I know that bug_email will take on more importance
for justdave very soon. I can't actually confirm that this doesn't work properly
at the moment, though I would believe it does. (bug_email is pretty *old* right
now.)

However, please re-submit that patch without the whitespace changes. It's nearly
impossible to tell what's actually changed.

-M

Comment 3

[Alex Schuilenburg]

Attachment: Updated patch to provide the behaviour desired

Patch without whitespace changes

Comment 4

[Alex Schuilenburg]

(In reply to comment #2)
> I would tend to agree, and I know that bug_email will take on more importance
> for justdave very soon. I can't actually confirm that this doesn't work properly
> at the moment, though I would believe it does. (bug_email is pretty *old* right
> now.)
 
OOI, I have made further mods to bug_email.pl, namely, corrected the handling
and insertion of attachments.  I can provide patches if required - I have had no
complaints or problems reported since the last set of changes in August 2003 and
the system is in regular use at http://bugs.ecos.sourceware.org/

-- Alex

Comment 5

[Max Kanat-Alexander]

Comment on attachment 139895 [details] [diff] [review]
Updated patch to provide the behaviour desired

>+            if (($msg_part =~ /^inline/) && ($type =~ /^(text|message)$/) && ($Body eq '')) {
>+                if ($IO = $body->open("r")) {
> [snip]

You might want to add a brief comment (with this bug number) at the top of this
block, just explaining what it does. I'm not a reviewer, but that would be my
first comment if I was. :-)

-M

Comment 6

[Max Kanat-Alexander]

Oh, whoops. It made me the person requesting review. That's no good. It's
actually the patch author, Alex. -M

Comment 7

[Alex Schuilenburg]

Attachment: Updated patch with comments as suggested

Added comments to the patch as suggested.

Comment 8

[Vlad Dascalu]

Comment on attachment 139895 [details] [diff] [review]
Updated patch to provide the behaviour desired

Clearing r? flag on obsolete attachment.

Comment 9

[Shane H. W. Travis]

*** Bug 279727 has been marked as a duplicate of this bug. ***

Comment 10

[Shane H. W. Travis]

This need on which this patch is predicated is actually a Core/Thunderbird 
issue, not a Bugzilla one; see bug 65794 for discussion. 

This patch still applies to the tip (probably because almost nobody ever 
touches the stuff in contrib), but I am of the opinion that we should not be 
fixing problems with other tools by implementing workarounds in Bugzilla... not 
in the core code anyway. (OTOH, if someone wants to post a patch for anyone to 
take if they need it for a local site, though, then that's fine; I would 
consider this to fall into that category.)

Others' opinions may differ, and I'm sure we'll hear from them soon. :)

Comment 11

[Dave Miller [:justdave]]

I'd tend to agree with Shane...  I've seen several mail clients that add your
signature block to the end of a message by making it an inline text attachment.
 So we'd likely wind up with attachments with nothing but a 3 or 4 line email
signature in them.

This is one of those stupid places about email where fixing it so it works in
one casae will break it in another, so neither way is ideal.

Comment 12

[Alex Schuilenburg]

Oddly, I disagree ;-)

There are unfortunately a couple of email clients that automatically inline text
attachments (i.e. not only moz/TB, but also evolution and webmail). Since the
patch is simple enough, I could modify the patch to be a configuration option so
the bz admin can choose how they want to handle inline text attachments.

And yes, inline .sigs will also get added as an attachment if implemented, an
unfortunate side-effect of this solution. Don't think I have not tried to find a
way around this :-(  However, generally this is not an issue since the format of
bugmail implies that .sigs should be excluded. Also, in our case everyone who
uses bugmail also do not use HTML mail or inline .sigs, so this is a non-issue.

Comment 13

[Marc Schumann [:Wurblzap]]

Is this a dupe of bug 176190?

Comment 14

[Håvard Wigtil]

This is not an exact duplicate of bug 176190, although they are related.
bug_email_append.pl has no attachment handling at all, this is about one aspect
of the already implemented attachment handling for bug_email.pl

Comment 15

[Frédéric Buclin]

Comment on attachment 139966 [details] [diff] [review]
Updated patch with comments as suggested

bitrotten, probably because all tabs have been removed.

Comment 16

[Peter Monthan]

The proposed solution may fix the Thunderbird problem but may be a problem if a bug report is submitted from e.g. gmail, which will have the string  "Content-Disposition: inline" for the body of the mail. So, to fix both the inline attachment problem and also make sure bug reports from gmail clients are accepted I had to change line:
if(  $msg_part =~ /^attachment/ ) {

to 
if((  $msg_part =~ /^attachment/ ) || (( $msg_part =~ /^inline/) && ($msg_part =~ /filename/)) ) {

This change will only treat a message part with "Content-Disposition" set to "inline" as an attachment, if it is followed by a filename tag. Otherwise it is left to be handled as the message body.

Comment 17

[Dave Miller [:justdave]]

Enhancement request + people already working on it = confirmed