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)

RESOLVED FIXED in Thunderbird 3.3a1

Status

MailNews Core
MIME
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: Phil Lacy, Assigned: Phil Lacy)

Tracking

(Depends on: 1 bug, Blocks: 3 bugs)

Trunk
Thunderbird 3.3a1
Dependency tree / graph

Firefox Tracking Flags

(blocking-thunderbird3.1 -, thunderbird3.1 wanted)

Details

(Whiteboard: [needs bug 583419 fixing before branch consideration])

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
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--
(Assignee)

Comment 1

8 years ago
Created attachment 431896 [details]
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

8 years ago
Phil, do you think this is a regression? Which version are you running?
(Assignee)

Comment 3

8 years ago
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.
OS: Windows XP → All
Version: unspecified → Trunk
(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.
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)
(Assignee)

Comment 6

8 years ago
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.
blocking-thunderbird3.1: --- → ?
Depends on: 182627
(Assignee)

Comment 7

8 years ago
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

8 years ago
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.
(Assignee)

Comment 9

8 years ago
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

8 years ago
Created attachment 432224 [details] [diff] [review]
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!
Blocks: 182627
No longer depends on: 182627
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.
Summary: multipart/alternate with inline part and text/plain part doesn't display → multipart/alternate with inline part and text/plain part doesn't display (When name of Content-Type and/or filename of Content-Disposition:inlinet is specified for mail-body part with text/xxx in multipart/alterniative)
(Assignee)

Updated

8 years ago
Summary: multipart/alternate with inline part and text/plain part doesn't display (When name of Content-Type and/or filename of Content-Disposition:inlinet is specified for mail-body part with text/xxx in multipart/alterniative) → 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)
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!

Updated

8 years ago
blocking-thunderbird3.1: ? → -
(Assignee)

Comment 13

8 years ago
(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

8 years ago
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.
status-thunderbird3.1: --- → wanted
(Assignee)

Updated

7 years ago
Assignee: nobody → philbaseless-firefox
(Assignee)

Comment 15

7 years ago
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.
(Assignee)

Comment 16

7 years ago
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.
(Assignee)

Comment 17

7 years ago
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

7 years ago
(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

7 years ago
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

7 years ago
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.
(Assignee)

Comment 21

7 years ago
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

7 years ago
(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.
(Assignee)

Comment 23

7 years ago
Created attachment 444804 [details] [diff] [review]
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.
(Assignee)

Comment 24

7 years ago
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
(Assignee)

Comment 25

7 years ago
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.
(Assignee)

Comment 26

7 years ago
(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.
(Assignee)

Comment 27

7 years ago
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.
(Assignee)

Comment 28

7 years ago
Created attachment 445055 [details] [diff] [review]
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.
Attachment #444804 - Attachment is obsolete: true
Attachment #445055 - Flags: review?(bienvenu)
No longer blocks: 182627
Depends on: 182627
Blocks: 229075, 314728, 182627
No longer depends on: 182627
(In reply to comment #27)
I set dependency to this bug for three bugs and assigned to you.
(Assignee)

Updated

7 years ago
Blocks: 568574

Comment 30

7 years ago
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.
Attachment #445055 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 31

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

can you sr it too, David.
Attachment #445055 - Flags: superreview?(bienvenu)

Comment 32

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

sure, thx for the patch.
Attachment #445055 - Flags: superreview?(bienvenu) → superreview+
Keywords: checkin-needed
(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 ?
Flags: in-testsuite?
(Assignee)

Comment 34

7 years ago
Can the test be linked to a new bug and resolve this to keep the trunk builds up to date?
(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
Checked in: http://hg.mozilla.org/comm-central/rev/dfb5264119e4
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.2a1
Phil/David, should we be thinking about this for 3.1.x?
(Assignee)

Comment 38

7 years ago
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

Updated

7 years ago
Depends on: 583419
(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.
Whiteboard: [needs bug 583419 fixing before branch consideration]
Depends on: 1018606
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.