TB 11.0.forwarding plain-text (non-MIME) emails creates empty "Attached Message Part" even when forward messages is set Inline, if no Content-Type header, no parameter in Content-Type:.

RESOLVED FIXED in Thunderbird 23.0

Status

MailNews Core
MIME
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Jim Lawson, Assigned: Magnus Melin)

Tracking

({regression})

Thunderbird 23.0
regression
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Build ID: 20120310194926

Steps to reproduce:

I received a plain-text (non-MIME) message and forwarded it, while using "forward inline" mode.


Actual results:

Thunderbird forwarded the message inline as I requested, but also added a zero-length attachment of Content-Type: text/plain name="Attached Message Part"


Expected results:

The original message should be forwarded inline, with no empty attachment.  The empty attachment is confusing to the recipient.
(Reporter)

Comment 1

5 years ago
This was seen on Thunderbird 11.0.1 on Win7 and OSX 10.7.3.
OS: Linux → All
(Reporter)

Comment 2

5 years ago
This problem also exists in 12.0.b4

Comment 3

5 years ago
(In reply to Jim Lawson from comment #2)
Works for me:
User Agent: Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20120327 Thunderbird/11.0.1
Application Build ID: 20120327115527

Try to start Thunderbird without add-ons and check.
(Reporter)

Comment 4

5 years ago
Thanks for trying to recreate.  Starting without add-ons does not make the problem go away.

IMAP server is Dovecot 2.1.4.  Although copying the original plain text message to a local folder does not change the behavior.

I will try a Linux install of 11.0.1 when I get to that setup...
(Reporter)

Comment 5

5 years ago
Was able to recreate problem in 11.0.1 on Linux (Ubuntu 10.04.4 LTS) on x86_64

Name=Thunderbird
Version=11.0.1
BuildID=20120327115527

Try an RFC822 complaint message, one WITHOUT MIME-Version:, Content-Type:, and Content-Transfer-Encoding: headers.  The problem is not triggered if these headers are present.
Problem is observed in next cases(With MIME-Version: 1.0 existent).
(1) No Content-Type: header
(2) Content-Type: (no parameter)
(3) Content-Type: unknown/unknown
(4) Content-Type: ???### (syntactically invalid)
Problem doesn't occur if major type is text.
(5) Content-Type: text/??? (syntactically invalid)
(6) Content-Type: text/unknown
Perhaps a regression by "quirks for malformed mail of non-displayable content-type like audio/wav, application/pdf". By it, such malformed mail is internally treated as if multipart/mixed in order to show the non-displayable message body of RFC822 mail as if attachment.

http://tools.ietf.org/html/rfc2045#section-5.2 says default is following.
  Content-type: text/plain; charset=us-ascii

What mailer or mail system did send mail without Content-Type: header?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: TB 11.0.forwarding plain-text (non-MIME) emails creates empty "Attached Message Part" even when forward messages is set Inline → TB 11.0.forwarding plain-text (non-MIME) emails creates empty "Attached Message Part" even when forward messages is set Inline, if no Content-Type header, no parameter in Content-Type:, unknown or corrupted mime-type in Content-Type:.
Component: General → MIME
Product: Thunderbird → MailNews Core
QA Contact: general → mime
(Reporter)

Comment 7

5 years ago
Sendmail which receives a "bare" message without headers will not add Content-Type headers, as it is not required for compliance with RFC2822 (or, it appears, RFC2042 if text/plain is the default.)
Created attachment 616015 [details]
Mail folder file (8 mails for test)

Problem is observed also on multipart/mixed mail with no attachment part.
(7) multipart/mixed, single part only in multipart/mixed,
    no Content-Type: header of text data part(message body part)
(8) multipart/mixed, single part only in multipart/mixed,
    Content-Type: unknown/unknown of text data part(message body part)
In (3) Content-Type: unknown/unknown and (4) Content-Type: ???### cases, phenomenon of "message body is not shown by view" is also observed in Tb 11.0.1.

Quick check result with a Tb 7.0a2 build. 
> Mozilla/5.0 (Windows NT 5.1; rv:7.0a2) Gecko/20110816 Thunderbird/7.0a2
Note: "This bug occurs" in Tb 7.0a2 = Attachment part is created by Forward,
                                      data is message body text instead of null.
(1) No Content-Type: header         This bug doesn't occur
(2) No parameter in Content-Type:   This bug doesn't occur
(3) Content-Type: unknown/unknown   Attachment is shown by view.
                                    This bug occurs. 
(4) Content-Type: ???###            Attachment is shown by view.
                                    This bug occurs. Bug 738284 occurs.
(5) Content-Type: text/???          This bug occurs.
(6) Content-Type: text/unknown      This bug occurs.
(7) multipart/mixed, single part only in multipart/mixed,
    no Content-Type: header of text data part(message body part)
                                    This bug doesn't occur.
(8) multipart/mixed, single part only in multipart/mixed,
    Content-Type: unknown/unknown of text data part(message body part)
                                    Attachment is shown by view.
                                    This bug occurs.

No Content-Type: case and No parameter of Content-Type: case looks regression.
Other cases seems old problem.
Keywords: regression, regressionwindow-wanted
Created attachment 629984 [details] [diff] [review]
[checked in] Fix

This bug is a regression by bug 679476.
Assignee: nobody → hiikezoe
Status: NEW → ASSIGNED
Attachment #629984 - Flags: review?(dbienvenu)
Duplicate of this bug: 752177

Comment 11

5 years ago
Comment on attachment 629984 [details] [diff] [review]
[checked in] Fix

thx, Hiro, yes, that gets the code back to what it did before the regression.
Attachment #629984 - Flags: review?(dbienvenu) → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/d5a2b2d481d5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Keywords: checkin-needed, regressionwindow-wanted
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0

Comment 13

5 years ago
A few employees at my company just upgraded to Thunderbird 15, and we're seeing this bug again; phantom empty "nsmail.tmp" being created when someone hits Forward.
Please read comment 12. This issue will be solved in Thunderbird 16. Sorry for the patience.

Comment 15

5 years ago
This bug also exists in TB 16.0.1(In reply to Hiroyuki Ikezoe (:hiro) from comment #14)
> Please read comment 12. This issue will be solved in Thunderbird 16. Sorry
> for the patience.

This bug also exists in TB 16.0.1

Comment 16

5 years ago
We are seeing this problem but only with a few machines here. We are running TB 16.0.2 on Windows XP pro. Anyone have the solution?

Comment 17

4 years ago
Still seeing this at TB 17.0 running on Windows 7 64 bit.

Comment 18

4 years ago
Confirmed and seeing this problem on Thunderbird Windows 7 x64 in 17.0.2 even with add-ons disabled

Comment 19

4 years ago
I'm having this issue as well, it doesn't do it on all emails, but the ones it does do this on if you reply it does no attachment. This only happens when forwarding.

Comment 20

4 years ago
The problem is still here in 17.0.2. Can someone reopen this bug?

Comment 21

4 years ago
Created attachment 712133 [details]
email that cause the problem

email that cause the problem
> http://hg.mozilla.org/comm-central/annotate/65b8e2fa4eba/mailnews/mime/src/mimedrft.cpp#l489
>           hg@0 489 //It's possible we must treat the message body as attachment!
>       mwu@8587 490 bool bodyAsAttachment = false;
> hiikezoe@10396 491 if (mdd->messageBody &&
> hiikezoe@10396 492 mdd->messageBody->m_type.Find("text/html", CaseInsensitiveCompare) == -1 &&
> hiikezoe@10396 493 mdd->messageBody->m_type.Find("text/plain", CaseInsensitiveCompare) == -1 &&
> hiikezoe@10396 494 !mdd->messageBody->m_type.LowerCaseEqualsLiteral("text"))
>  bugzilla@9082 495 bodyAsAttachment = true;
Patch by this bug is still active.

I also see problem in Tb 17.0.3 on Win-XP as Serg Iv says, and as seen in Bug 846343. With any test mail of my attachment 616015 [details] , I could still see problem of this bug.

Re-opening.

Ikezoe-san, did you actually test the patch?

Is default string of "text/plain" set in mdd->messageBody when no Content-Type: header?
If no, it looks for me the patch resolved problem with "Content-Type: Text" like case...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
mistake in my pasting... sorry for spam.
  mdd->messageBody  ==>  mdd->messageBody->m_type

Comment 24

4 years ago
Also seeing this issue in Tbird 17.0.3 on Win 7 64bit, x-ref bug report 846343

Comment 25

4 years ago
Hello  WADA ,

Is there anything I can do to help you guys solve this? Where on Thunderbird can I see the default string for content type?

This issue is still on 17.03.
Following is compose format determination logic.
> http://mxr.mozilla.org/comm-central/source/mailnews/mime/src/mimedrft.cpp#1336
> 1336     if (mdd->messageBody)
> 1337     {
> 1338       MSG_ComposeFormat composeFormat = nsIMsgCompFormat::Default;
> 1339       if (!mdd->messageBody->m_type.IsEmpty())
> 1340       {
> 1341         if(mdd->messageBody->m_type.Find("text/html", CaseInsensitiveCompare) != -1)
> 1342           composeFormat = nsIMsgCompFormat::HTML;
> 1343         else if (mdd->messageBody->m_type.Find("text/plain", CaseInsensitiveCompare) != -1 ||
> 1344                  mdd->messageBody->m_type.LowerCaseEqualsLiteral("text"))
> 1345           composeFormat = nsIMsgCompFormat::PlainText;
> 1346         else
> 1347           //We cannot use this kind of data for the message body! Therefore, move it as attachment
> 1348           bodyAsAttachment = true;
> 1349       }
> 1350       else
> 1351         composeFormat = nsIMsgCompFormat::PlainText;

Ikezoe-san, I think following is better. Is it right?
> 489   //It's possible we must treat the message body as attachment!
> 490   bool bodyAsAttachment = false;
> 491   if (mdd->messageBody &&    ===>    if (mdd->messageBody && !mdd->messageBody->m_type.IsEmpty() &&
> 492       mdd->messageBody->m_type.Find("text/html", CaseInsensitiveCompare) == -1 &&
> 493       mdd->messageBody->m_type.Find("text/plain", CaseInsensitiveCompare) == -1 &&
> 494       !mdd->messageBody->m_type.LowerCaseEqualsLiteral("text"))
> 495      bodyAsAttachment = true;
And, if TEXT, Text, texT(no / nor subtype) is considered text, I think TEXT/, Text/, texT/ (with / and no subtype) is better considered text.
Some other thoughts on code.
1. If texT/ like one is cared, comparison like CaseInsensiveStartWith("text/") is better.
2. If logic like 1336 to 1351 is used at multiple places, Objects's method like mdd->messageBody->m_type.Is_text_family() may be useful, because duplicate code is eliminated.
Duplicate of this bug: 846343

Updated

4 years ago
Duplicate of this bug: 829317

Comment 31

4 years ago
I have regressed bug 745919 and confirmed that the first nightly build in which it appears is 2011-09-23. That means that it was almost certainly caused by the commit in bug 679476, which landed on 2011-09-22.

That means that the bug has been in the code base for over 18 months, and for over 9 months since it was allegedly fixed by the commit in bug 745919 which didn't actually fix it.

It would be nice if one of the people responsible for writing or reviewing the commit in bug 679476, or one of the people responsible for writing or reviewing the patch in bug 745919, would step up and fix this bug properly.

It's pretty appalling that it's still in the code base after over 18 months.
(Assignee)

Updated

4 years ago
Attachment #629984 - Attachment description: Fix → [checked in] Fix
(Assignee)

Comment 32

4 years ago
I have a fix for this, thx for the detective work WADA!
Assignee: hiikezoe → mkmelin+mozilla
Hardware: x86_64 → All
(Assignee)

Updated

4 years ago
Blocks: 856456
(Assignee)

Comment 33

4 years ago
Created attachment 737167 [details] [diff] [review]
proposed fix

Fix the case of non-mime message (with no content type), and add test. (Well, testing bug 856456 which this also fixes, and tagging on the test there's no attachment either.)
Attachment #737167 - Flags: review?(mconley)

Comment 34

4 years ago
So what is the fix for this problem now, is there even one?
(Assignee)

Comment 35

4 years ago
If you do your own build, apply the patch. Otherwise wait until the patch is reviewed and checked in, then it is fixed in nightly builds.
Comment on attachment 737167 [details] [diff] [review]
proposed fix

Review of attachment 737167 [details] [diff] [review]:
-----------------------------------------------------------------

Just some nits here, but I think this looks alright.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +2558,5 @@
>  {
>    var send_default_charset = gMsgCompose.compFields.defaultCharacterSet;
>  //  dump("send_default_charset is " + send_default_charset + "\n");
>  
> +  var compFieldsCharset = gMsgCompose.compFields.characterSet ||

While you're here, let not var.

::: mail/test/mozmill/composition/test-save-changes-on-quit.js
@@ +34,5 @@
>    collector.getModule("prompt-helpers").installInto(module);
>    collector.getModule("window-helpers").installInto(module);
>  
>    folder = create_folder("PromptToSaveTest");
> +  

Trailing whitespace.

@@ +44,5 @@
>  
> +function msgSource(aSubject, aContentType) {
> +  let msgId = Components.classes["@mozilla.org/uuid-generator;1"]
> +                        .getService(Components.interfaces.nsIUUIDGenerator)
> +                        .generateUUID() +"@invalid";

Space after +

@@ +247,5 @@
> +/**
> + * Test there's no prompt on close when no changes was made in reply/forward
> + * windows - for the case the original msg had content type "text".
> + */
> +function test_no_prompt_on_close_for_unmodified_content_type_text() {

Nice tests.

::: mailnews/mime/src/mimedrft.cpp
@@ +478,5 @@
>    nsMsgAttachedFile           *tmpFile = NULL;
>  
>    //It's possible we must treat the message body as attachment!
>    bool bodyAsAttachment = false;
> +  if (mdd->messageBody && !mdd->messageBody->m_type.IsEmpty() &&

Let's put this new term on its own line.
Attachment #737167 - Flags: review?(mconley) → review+

Comment 37

4 years ago
I don't have my own build, do you know how long it will be before Thunderbird adds this fix to their newer builds?
(Assignee)

Comment 38

4 years ago
It will be in nigthly builds in a guess a day or a few, depending on when i have the chance to update the patch and check it in. (When i mark this bug FIXED, it's in the next nigthly build.)
(Assignee)

Comment 39

4 years ago
http://hg.mozilla.org/comm-central/rev/eabed5c2dfe1 -> FIXED

Removing the part about corrupted mime-type from summary. The patch didn't change those, and i don't know how/if those shoud be handled - in any case they should be rare.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
Summary: TB 11.0.forwarding plain-text (non-MIME) emails creates empty "Attached Message Part" even when forward messages is set Inline, if no Content-Type header, no parameter in Content-Type:, unknown or corrupted mime-type in Content-Type:. → TB 11.0.forwarding plain-text (non-MIME) emails creates empty "Attached Message Part" even when forward messages is set Inline, if no Content-Type header, no parameter in Content-Type:.
Target Milestone: Thunderbird 16.0 → Thunderbird 23.0
(Assignee)

Updated

4 years ago
Flags: in-testsuite- → in-testsuite+
You need to log in before you can comment on or make changes to this bug.