551698 - multipart/alternative with inline part and text/plain part doesn't display (When name of Content-Type and/or filename of Content-Disposition:inline is specified for mail-body part with text/xxx in multipart/alternative)

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Phil Lacy]

if you have display body as original html the message is blank. The alternate html file or body doesn't show. I can attach email later.



Content-Transfer-Encoding: binary
Content-Type: multipart/alternative; boundary="_----------=_1268324732523914"

This is a multi-part message in MIME format.

--_----------=_1268324732523914
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain


--_----------=_1268324732523914
Content-Disposition: inline; filename="message.html"
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html; name="message.html"


--_----------=_1268324732523914--

Comment 1

[Phil Lacy]

Attachment: example of bug

typical craigslist.org email
set display body as orig html
clear view attachments inline

drag message to local folder and view

Comment 2

[David :Bienvenu]

Phil, do you think this is a regression? Which version are you running?

Comment 3

[Phil Lacy]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a2pre) Gecko/20100221 Shredder/3.2a1pre

I don't know if it's a regression. Unfortunately it can be fixed by the universal mime cure-all of turning on 'display attachments inline', so that may have kept users from reporting it sooner.

Comment 4

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #1)
> example of bug

> --_----------=_1268324732523914
> Content-Disposition: inline; filename="message.html"
> Content-Transfer-Encoding: quoted-printable
> Content-Type: text/html; name="message.html"> 

Checked with Tb 3.0.3(Win-XP).

(1) If changed to next, HTML part is displayed as expected. 
> --_----------=_1268324732523914
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
> Content-Type: text/html

(2) If original(with filename, name), HTML part is displayed
    by View/Display Attachments Inline=On.

Probably same as old/known problem with multipart/mixed.
  If name and/or filename is specified for text part used as mail body,
  Tb considers the part as attachment instead of mail body,
  then the text part is not displayed.
  However, the text part is part which is considered mail body first,
  (first part of mixed. same on text/html part of alternative,Original HTML)
  the non-displayed text part is not displayed at attachment pane.

Comment 5

[WADA:World Anti-bad-Duping Agency]

Old/known bug is Bug 182627. Problem occurs even on simplest text/plain mail.
> Bug 182627 Message body is not displayed for some messages if view attachments inline is false
> (name in Content-Type/filename in Content-Disposition for mail-body/mail-body-part
> forces "attachment" for Tb even though real mail body)

Comment 6

[Phil Lacy]

I say bump this since setting the view attachments inline is a performance hit on imap. Turning off 'view attachments...' is a nice feature that shouldn't be broken.

David I may try working on this can you tell me if this is mime code issue or a display issue or some other problem.

Comment 7

[Phil Lacy]

I'm thinking if disposition is inline and text/plain or text/html the part should be displayed and if the disposition is 'attachment' then it should be shown in attachment pane and displayed if display attachments inline is set.

Comment 8

[David :Bienvenu]

Phil, I figured out the issue, and am looking at a fix. But I'm happy to give you what I have so far and you can improve on it. Basically, the code is doing this on purpose, and doesn't have enough information to even special case the first  body part.

Comment 9

[Phil Lacy]

Be my guest. The fix appeared it might be a bit involved for me since it affected local folders and not just imap.

If you don't think you can squeeze it in your schedule then post your what you have so far.

Comment 10

[David :Bienvenu]

Attachment: starting point for a fix

This shows you why the last couple messages in the test folder fail to display anything for a body, and fixes them. It doesn't fix the display of multiple parts in some of the other messages, if I'm understanding the issue correctly.

Phil, thx for looking at this!

Comment 11

[WADA:World Anti-bad-Duping Agency]

FYI.
As I wrote in bug 182627 comment #38, for next two types of mail,
  - Simple text/plain mail, with name parameter
  - text/plain in multipart/mixed mail, with name/content-disposition/filename
behaviour of Tb 3.0.3 was consistent with explanation in source code.
  - When Display Attachments Inline=Off,
    mail body(or mail body part) is not displayed at message display box
  - When Display Attachments Inline=Off and On,
    mail body(or mail body part) is displayed at attachment pane in any case
    (problem of "not displayed in some cases" was already resolved)

(In reply to comment #7)
> I'm thinking if disposition is inline and text/plain or text/html the part
> should be displayed and if the disposition is 'attachment' then it should be
> shown in attachment pane and displayed if display attachments inline is set.

"no displayed mail body, no attachment at attachment pane" is also observed with next part in multipart/alternative.
> --_----------=_1268324732523914
> Content-Transfer-Encoding: quoted-printable
> Content-Type: text/html; name="message.html"
What do you think about this case?
  "name=..." exists == generated by attach operation && no "inline"
  ==> not mail body, then attachmet?
     (same as mail-body of simple text/xxx, text/xxx part in multipart/mixed) 
  If so, I think it's better to be displayed at attachment pane,
  even though a part in multpart/alternative, because text/html matches
  with "View/Message Body As=HTML".

How about "Content-Disposition:inline" for text & image/jpeg part which is a part of multipart/related, which is not reffered via cid: by HTML(<img src="cid:..">, <iframe src="cid:...">).
I think Content-Diposition:(inline or attachment) should be ignored.

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Right now, for a bug to block 3.1, it needs to either:

a) make the upgrade experience from TB2 very painful for a large number of users

or 

b) be a new, reproducible, severe quality issue (eg dataloss, frequent crashes)

Just from skimming this bug, it's not clear to me how this would fit into those buckets.  Setting as blocking-, based on that.

Phil, if I'm missing something, please renominate with details.  Thanks!

Comment 13

[Phil Lacy]

(In reply to comment #12)
> Phil, if I'm missing something, please renominate with details.  Thanks!

No, no problem, explanation wasn't necessary. I'm on 3.2pre and wanted to bump it for next release but couldn't find a 3.2 flag.

Comment 14

[David :Bienvenu]

We'd take a fix for this in 3.1, as long as we have one. We just wouldn't block on it. Marking wanted.

Comment 15

[Phil Lacy]

David, we assume if it has a filename then it was meant to be attachment not a typed message, and we make it a mimeExternalObjectClass, but how do we get these types to show up in the attachment bucket? That would probably take care of this bug. In the craigs list example, it was probably intended recipients should open html in outside browsers for security.

Comment 16

[Phil Lacy]

actually, that doesn't sound right, if it is marked inline, text/html, with file name, and part of mp/alt then it was probably meant to display in html mode and show up in the attachment bucket.

Comment 17

[Phil Lacy]

OExp shows it like comment 16, in text or html mode and always as an attached file.
I suspect any part with a file name should be loading into our attachment bucket.

Comment 18

[Mike Cowperthwaite]

(In reply to comment #16)
> if it is marked inline, text/html, with
> file name, and part of mp/alt then it was probably meant to display in html
> mode and show up in the attachment bucket.

I think if it was marked inline, it was not intended as an attachment, period.  The alternative setting to "inline" is "attachment."  The superfluous filename is probably an artifact of the MIME generator.

We should only handle "inline" items as attachments if we don't support inline rendering for the type.

Comment 19

[David :Bienvenu]

How do you save an attachment if it's not in the attachment bucket, because it's inline? I believe that's the point of showing it in the attachment pane.

Comment 20

[Mike Cowperthwaite]

Wait -- we're talking about the message body.  The problem is, the message body has been tagged with a filename (for no readily apparent reason) -- and due to a BUG in Mozilla's Not-Wonderful MIME Implementation, the name is overriding the 'inline' flag.

Why would we care if it's saveable as an attachment?

And in the context of the message, you should be able to tell it's the body, because it's one of the top-level parts of a multipart/alternative structure.  Now, I can readily believe that the MIME implementation is not sophisticated enough to make that determination, but the MIME structure has enough information.

Comment 21

[Phil Lacy]

with a 'name =' or 'filename =' I'd say sender intended showing an attached file and with it being 'inline' and the html alternative part then sender intended also having it displayed.

Haven't found out where we can do that yet. It would save me more looking if anyone knows where we determine what goes in our attachments display, I guess it's not really the 'bucket' that is the compose window name.

Comment 22

[David :Bienvenu]

(In reply to comment #20)
> Wait -- we're talking about the message body. 
> I think if it was marked inline, it was not intended as an attachment, period. 

I'm sorry - I thought you were talking about any inline part.

Comment 23

[Phil Lacy]

Attachment: header with filenames goes to attachment view

David, this is only a partial fix but comment on it if you get a chance.
This patch will put *any* part with a name= or filename= parameter in its header into the attachment view.

Comment 24

[Phil Lacy]

Comment on attachment 432224 [details] [diff] [review]
starting point for a fix

>diff --git a/mailnews/mime/src/mimemsg.cpp b/mailnews/mime/src/mimemsg.cpp
> 
>-  body = mime_create(ct, msg->hdrs, obj->options);
>+  body = mime_create(ct, msg->hdrs, obj->options, obj);

I don't believe this will work when mime_create gets called from mimealt.cpp

Comment 25

[Phil Lacy]

Comment on attachment 432224 [details] [diff] [review]
starting point for a fix

>       /* It's a text type.  Write it only if it's the *first* part
>          that we're writing, and then only if it has no "filename"
>          specified (the assumption here being, if it has a 

can we just re-assume this and say a filename is not a deal breaker in displaying.  If it's a text type, write it if it's the first_part.

Comment 26

[Phil Lacy]

(In reply to comment #23)
> Created an attachment (id=444804) [details]
> header with filenames goes to attachment view
> 
> David, this is only a partial fix but comment on it if you get a chance.
> This patch will put *any* part with a name= or filename= parameter in its
> header into the attachment view.

This needs fixing in so far as it is assuming all attachments have a filename parameter.

Comment 27

[Phil Lacy]

Bug 229075
Seems to be a lot of similiar bugs and maybe overlap. Wada, can you look at all these and close them as appropriate or assign me. I seem to be getting into another bug with my attachment 444804 [details] [diff] [review].

That attachment causes inline parts with filenames to be added to attachment view and assigned attachment status. This bug should only address the display problem with inline parts that have filenames.

Comment 28

[Phil Lacy]

Attachment: inline parts with filenames displayed and show as attachment

We had assumed a filename parameter meant it wasn't typed-in in spite of it being inline. I removed that since it was more of a guess then an assumption.
This may fix a gamut of bugs. Maybe we can do a try serve build for someone to help in testing.
I'll think about a possible test although it will be limited to the new 'assumption'.
This patch fixes the craigslist.org standard letters to display and have an attachment, like OExpress works.

Comment 29

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #27)
I set dependency to this bug for three bugs and assigned to you.

Comment 30

[David :Bienvenu]

Comment on attachment 445055 [details] [diff] [review]
inline parts with filenames displayed and show as attachment

sorry for the delay; I agree that this is the right thing to do.

Comment 31

[Phil Lacy]

Comment on attachment 445055 [details] [diff] [review]
inline parts with filenames displayed and show as attachment

can you sr it too, David.

Comment 32

[David :Bienvenu]

Comment on attachment 445055 [details] [diff] [review]
inline parts with filenames displayed and show as attachment

sure, thx for the patch.

Comment 33

[Ludovic Hirlimann [:Usul]]

(In reply to comment #31)
> (From update of attachment 445055 [details] [diff] [review])
> can you sr it too, David.

Any way to add some unit test for this so we make sure not to regress in the future ?

Comment 34

[Phil Lacy]

Can the test be linked to a new bug and resolve this to keep the trunk builds up to date?

Comment 35

[Ludovic Hirlimann [:Usul]]

(In reply to comment #34)
> Can the test be linked to a new bug and resolve this to keep the trunk builds
> up to date?

we can keep this one resolved with the in-testsuite ? set

Comment 36

[Mark Banner (:standard8)]

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

Comment 37

[Mark Banner (:standard8)]

Phil/David, should we be thinking about this for 3.1.x?

Comment 38

[Phil Lacy]

Being a basic design flaw fix I'd say it should be ok for 3.1.x. I haven't tested it on 3.1 though

Comment 39

[Mark Banner (:standard8)]

(In reply to comment #38)
> Being a basic design flaw fix I'd say it should be ok for 3.1.x. I haven't
> tested it on 3.1 though

Looking at bug 583419 it appears we'd need to fix that before taking this on a branch.