Closed
Bug 551698
Opened 15 years ago
Closed 14 years ago
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)
Categories
(MailNews Core :: MIME, defect)
MailNews Core
MIME
Tracking
(blocking-thunderbird3.1 -, thunderbird3.1 wanted)
RESOLVED
FIXED
Thunderbird 3.3a1
People
(Reporter: philbaseless-firefox, Assigned: philbaseless-firefox)
References
(Depends on 1 open bug, Blocks 3 open bugs)
Details
(Whiteboard: [needs bug 583419 fixing before branch consideration])
Attachments
(3 files, 1 obsolete file)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Bienvenu
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
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--
typical craigslist.org email
set display body as orig html
clear view attachments inline
drag message to local folder and view
Comment 2•15 years ago
|
||
Phil, do you think this is a regression? Which version are you running?
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
Comment 4•15 years ago
|
||
(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•15 years ago
|
||
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)
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: --- → ?
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•15 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.
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•15 years ago
|
||
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!
Updated•15 years ago
|
Comment 11•15 years ago
|
||
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.
Updated•15 years ago
|
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)
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)
Comment 12•15 years ago
|
||
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•15 years ago
|
blocking-thunderbird3.1: ? → -
Assignee | ||
Comment 13•15 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•15 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 | ||
Comment 15•15 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•15 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•15 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•15 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•15 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•15 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•15 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•15 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•15 years ago
|
||
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•15 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•15 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•15 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•15 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•15 years ago
|
||
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)
Updated•15 years ago
|
Updated•15 years ago
|
Comment 29•15 years ago
|
||
(In reply to comment #27)
I set dependency to this bug for three bugs and assigned to you.
Comment 30•14 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•14 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•14 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+
Updated•14 years ago
|
Keywords: checkin-needed
Comment 33•14 years ago
|
||
(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•14 years ago
|
||
Can the test be linked to a new bug and resolve this to keep the trunk builds up to date?
Comment 35•14 years ago
|
||
(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•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.2a1
Comment 37•14 years ago
|
||
Phil/David, should we be thinking about this for 3.1.x?
Assignee | ||
Comment 38•14 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
Comment 39•14 years ago
|
||
(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]
Updated•9 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•