525955 - Images added to an HTML message body now get an incorrect "attachment" disposition by default

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[rsx11m]

With bug 65794, the default content disposition was changed to "attachment" and is applied to all outgoing messages. This has the side effect that also images included into the message body using Insert > Image receive an "attachment" disposition by default, which is wrong. While Thunderbird and SeaMonkey ignore the disposition for content within an HTML part, some other clients (including web mail systems) may not and omit the image from from the display as they are advised to treat it as an attachment. So, this is a conflict situation between content-disposition versus content-id referencing.

Steps to reproduce:
1) ensure that mail.content_disposition_type = 1 (default);
2) compose a new HTML message;
3) attach an image file using the Attach button;
4) insert the same image using Insert > Image;
5) send or save as draft, inspect the encoded message.

Actual result:
- both attached and inserted image have "Content-Disposition: attachment".

Expected result:
- the image explicitly attached has "Content-Disposition: attachment",
- the image within the message has "Content-Disposition: inline".

To resolve the issue while waiting for a more comprehensive fix in bug 452092, the content disposition should be "inline" for content which is identified as part of the message body. Since we are in full control of the message encoding during composition, it is safe to assume that only content which is inserted by either Insert > Image, drag-and-drop, or copy-and-paste will have both a name and a content-id reference. This is the case to be covered here. Images quoted in replies don't have a name and receive "inline" disposition without regard to the preference setting with the current code.

Comment 1

[rsx11m]

Attachment: Possible patch

This patch adds another case to mime_generate_attachment_headers() where #1
we have a name, and #2 we also have a content id specified, in which case the setting of mail.content_disposition_type is ignored and "inline" selected.
This combination should only occur for content which is referred to in an HTML part of our composition and would serve as a good workaround for the 1.9.1 branch to avoid possible conflicts with other mail clients.

Comment 2

[Joe Sabash [:JoeS1]]

Unfortunately name, and filename gets dropped when you "edit as new" on a saved template. (unless you re-insert the image)
The only way you could be sure it was inline would be to compare the CID: <part x.y.z...  to the content_ID: <partx.y.z...

Comment 3

[rsx11m]

Yes, I've noticed that on a reply as well, but that has been before already. Removing the name of an attachment with a content-ID also removes the content disposition, which should then be implied as "inline". This code needs a major cleanup, the motivation here is to cover a specific well-defined case where a freshly inserted image gets an explicit wrong content disposition and may cause issues on the receiving side. So, it's a very limited but safe fix for the sake of being feasible for the upcoming release.

Comment 4

[Joe Sabash [:JoeS1]]

Agreed.
If I had regular discourse in html with a Gmail user I would definitely toggle the default pref. That's pretty much undiscoverable to the new user.

Comment 5

[rsx11m]

David, Neil: I was hoping to get this into a TB 3.0 RC2 (if it's coming) or at least into the TB/SM .1 updates, do you have any opinion on the patch so far?

Comment 6

[David :Bienvenu]

I don't think we're going to accept this for 3.0 at this point.  3.1 is possible.

Comment 7

[David :Bienvenu]

Comment on attachment 409745 [details] [diff] [review]
Possible patch

Seems to work - could use a comment in the code to explain why we're doing this.

Comment 8

[rsx11m]

Attachment: Proposed patch (v2)

Same patch as before, now with a comment added as requested.
Carrying forward r=bienvenu from attachment 409745 [details] [diff] [review].

Comment 9

[[e-mail]]

Comment on attachment 416938 [details] [diff] [review]
Proposed patch (v2)

I tried and failed to think of an easy way to avoid the pref read.

Comment 10

[rsx11m]

Thanks for the reviews, push for comm-central please.

Comment 11

[rsx11m]

(In reply to comment #9)
> I tried and failed to think of an easy way to avoid the pref read.

There shouldn't be much of a performance penalty, and I'd assume that any solution for bug 452092 will have to rework this part substantially anyway,
thus it's hopefully ok.

Comment 12

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/af7faefb7cb7