Plaintext attachments are always displayed inline

RESOLVED FIXED in Thunderbird 5.0b1

Status

defect
--
major
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: JoeS1, Assigned: Bienvenu)

Tracking

(Depends on 1 bug, {regression})

Trunk
Thunderbird 5.0b1
x86
Windows XP
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-thunderbird5.0 needed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Posted file testcase eml
Noted in:
Mozilla/5.0 (Windows; Windows NT 5.1; rv:2.0b3pre) Gecko/20100728 Shredder/3.2a1pre ID:20100728032601
Not seen in 20100722 build.

STR:
View the attached eml

The content-disposition is attachment, yet the text displays regardless of pref settings.

Looks like fallout from bug 551698
Also reproduced in Mozilla/5.0 (Windows; Windows NT 5.1; rv:2.0b3pre) Gecko/20100730 SeaMonkey/2.1a3pre, TB 3.1 Lanikai builds are ok.
Component: Mail Window Front End → MIME
Product: Thunderbird → MailNews Core
QA Contact: front-end → mime
Severity: normal → major
in view orig html I get the message and background
a divider line and 'this body part will be downloaded on demand' and attached.txt in attachments

in view plain I get
This body part will be downloaded on demand.
same divider line--attached.txt
This body part will be downloaded on demand.
and part 1.1.1.2 and attached.txt in attachments

to get the attachment to display I have to set display attachments inline
(In reply to comment #2)
> in view plain I get
> This body part will be downloaded on demand.
> same divider line--attached.txt
> This body part will be downloaded on demand.
> and part 1.1.1.2 and attached.txt in attachments
> to get the attachment to display I have to set display attachments inline

Phil Lacy, how did you view the test mail?
(A) Import .eml to local mail folder (Drag&Drop .eml to thread pane)
(B) File/Open Saved Message... of local .eml file by Tb
(C) Click attachment(message/rfc822) of this bug by Browser => Opened by Tb
(D) Import .eml to IMAP offline-use=on  folder(Drag&Drop .eml to thread pane)
(E) Import .eml to IMAP offline-use=off folder(Drag&Drop .eml to thread pane)

I could observe "This body part will be downloaded on demand" only by (E) with next trunk build.
> Mozilla/5.0 (Windows NT 5.1; rv:2.0b4pre) Gecko/20100806 Shredder/3.2a1pre
I think it's a variant of known issue(you probably already know) around mismatch among multipart-on-demand, MIME, Mail Display when complex structure.
 
Phil Lacy, as (B),(C),(D),(E) may produce other problem than this bug's main problem, I recommend you to check by (A) first.
Note: Even (D), problem like bug 572974 comment #7 can happen, although such problem is very hard to reproduce with attached test mail to this bug.
(In reply to comment #2)
> in view orig html I get the message and background
> a divider line and 'this body part will be downloaded on demand' and
> attached.txt in attachments
> 
I probably should have added that the original symptoms were seen in pop3 mail.
But if you open the testcase directly by TB or as a local copy (as WADA suggested) you should see the attachment over-writing the background (always) and not under control of the "display attachments inline pref"
I see it now in local folders. I was in imap with mime parts on demand set to true.
Whiteboard: [tbtrunkneeds]
the issue here is the ignoring of the pref, not the content disposition. In other words, the pref trumps the content disposition.
Flags: blocking-thunderbird-next+
Whiteboard: [tbtrunkneeds]
blocking-thunderbird5.0: --- → needed
Flags: blocking-thunderbird-next+
Duplicate of this bug: 617874
David seems to have an idea about what this is about, so passing to him.
Assignee: nobody → dbienvenu
Attachment #461719 - Attachment mime type: application/x-mimearchive → text/plain
the fix for bug 551698 did indeed cause this regression, in particular, I think the first chunk of the patch. I'll see if there's some way of fixing this without breaking bug 551698, but failing that, I'd back out that patch.
Posted patch proposed fix (obsolete) — Splinter Review
I think displaying the cached part inline is always the right thing to do.
Comment on attachment 528447 [details] [diff] [review]
proposed fix

Phil, does this seem like the right thing to do? It fixes this bug, without regressing the earlier bug.
Attachment #528447 - Flags: feedback?(philbaseless-firefox)
Whiteboard: [has possible fix]
Posted patch better fix (obsolete) — Splinter Review
this is a bit less hacky fix. It backs out the regression causing patch, and fixes that bug a different way by explicitly telling mime_create to force the part to be inline, in this case. I don't know quite how to write a unit test for this.
Attachment #528447 - Attachment is obsolete: true
Attachment #528447 - Flags: feedback?(philbaseless-firefox)
Attachment #530136 - Attachment is obsolete: true
Attachment #530395 - Flags: review?(bugmail)
Whiteboard: [has possible fix] → [has fix for review]
Comment on attachment 530395 [details] [diff] [review]
fix with unit test

(I think Jim is the king of attachments these days :)
Attachment #530395 - Flags: review?(bugmail) → review?(squibblyflabbetydoo)
Comment on attachment 530395 [details] [diff] [review]
fix with unit test

Review of attachment 530395 [details] [diff] [review]:

Aside from the two nits I have below, this looks good. r=me with those addressed.

::: mailnews/mime/test/unit/test_text_attachment.js
@@ +34,5 @@
+
+/**
+ * This test creates some messages with attachments of different types and
+ * checks that libmime reports the expected size for each of them.
+ */

This should be changed to describe the contents of this test.

@@ +51,5 @@
+// Create a message generator
+const msgGen = gMessageGenerator = new MessageGenerator();
+// Create a message scenario generator using that message generator
+const scenarios = gMessageScenarioFactory = new MessageScenarioFactory(msgGen);
+

Do we need anything here other than just the "gMessageGenerator = new MessageGenerator();" part?
Attachment #530395 - Flags: review?(squibblyflabbetydoo) → review+
fix checked in with comments addressed, thx for the review!
http://hg.mozilla.org/comm-central/rev/d538b947880c
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
thanks for fix, David, I been out of country traveling on and off for some time with only my iPad so can't do much to help or test without TB.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Thunderbird 3.3a4 → ---
thx for getting back to me, Phil. I think you/bugzilla accidentally re-opened, so re-marking fixed.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Whiteboard: [has fix for review]
You need to log in before you can comment on or make changes to this bug.