246415 - [MIME] "This body part will be downloaded on demand" strikes back

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Olden]

User-Agent:       Opera/6.12 (OpenBSD 3.4; U)  [en]
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040514

Exact same symptoms (and probably, cause) as
http://bugzilla.mozilla.org/show_bug.cgi?id=190905
except that I've experienced this issue with Mozilla 1.7rc2
(Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040514)

[Sorry, I whish I could just have re-opened #190905]

The message was in text/rtf with 3 attachments, the last one being plain txt.
I'll try and attach the source as moz1.7rc2_bug190905.txt (whole message but
with private data removed)

Funny fact: the fix previously submitted for #190905 actually seems to be the
cause of the problem this time.
The email is displayed fine only if "View > Message Body As > Plain text" is
selected.


Reproducible: Always
Steps to Reproduce:
Same as http://bugzilla.mozilla.org/show_bug.cgi?id=190905

Comment 1

[Olden]

Attachment: Message causing the problem on Mozilla 1.7rc2

Comment 2

[Service Desk]

I have the same problem with Mozilla 1.5 US version. 
We use Imap connection.
--> The only solution I found is to select the message and "copy to" a Local
folder. Or simply move it on a local folder. Then you can see the full body of
the message instead of the "This body part will be downloaded on demand" message.

But it is still anoying users to get these type of message and not be able to
see them in there inbox.

Keep up the good work !!

PS: I wonder if this bug has anything to do with the mail client that is used to
renerate the email that has this message in it. For my experience, It tends that
 these message comes from MS Outlook mail clients...

Dubweb 

Comment 3

[Keith Hopkins]

Mozilla 1.7.3
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040913

"me too" 

Occurs (inappropriately, in my opinion) in this situation:
View/Message Body As/Original HTML
View/Display Attachments Inline (NOT selected)
  
First section of msg:
This is a multi-part message in MIME format.

------_=_NextPart_001_01C49F54.FE8DED80
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable

Second section of msg: 
------_=_NextPart_001_01C49F54.FE8DED80
Content-Type: text/html;
	name="HP Software News.htm"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
	filename="HP Software News.htm"

First section of message has actual text included.

I think the message "This body part will be downloaded on demand" is
inappropriate because it does not describe the situtaion.

(Workaround: Select View/Attachement Display Inline)
 

Comment 4

[Mike Cowperthwaite]

Confirming based on multiple reports here and the probable dupe from TB at 
bug 265022.

Comment 5

[Mike Cowperthwaite]

*** Bug 265022 has been marked as a duplicate of this bug. ***

Comment 6

[Mike Cowperthwaite]

(Duplicate is not about TB, but Moz 1.7.2 -- the bug was misfiled.)

Comment 7

[Todd]

I get this message when a co-worker (both of us using Thunderbird 0.7) who has
HTML composing off and a vcard attached, sends a message with other attachments.
 If I "view message body as plain text", it goes away and shows the correct MIME
body text.  I had her switch to a plaintext signature instead of the vcard, and
the problem went away.

I will install the latest Mozilla suite and read the problematic messages with
it, to see if the bug is there.

Comment 8

[Gerrit Hannaert]

I wonder if this could be a configuration issue.

Last week I copied a raw message with the known symptoms from one of our users
to my own mail folder on the server side and we both have the same Mozilla
version. He gets this error message I and I don't. We're both running 1.7.3
although previously he used 1.6 which I haven't used in a while.

I'll try and get a hold of the mail in question again and compare our .js files.

Server: MDaemon 7.2.1, client: Mozilla 1.7.3 on Windows 2000.

Comment 9

[Gerrit Hannaert]

Just for completeness here are the mail headers of the mail from last week
(sorry, I xx-ed out some headers for privacy reasons).

Note that in contrast to other reports this was NOT an Outlook sender. The mail
body goes on further (it's quite long).

-


Return-path: <[e-mail]>
Received: from xxxx.cropdesign.local ([10.1.1.11])
	by cropdesign.com (cropdesign.com)
	(MDaemon.PRO.v7.2.0.R)
	with ESMTP id md50000019790.msg
	for <[e-mail]>; Mon, 08 Nov 2004 01:11:11 +0100
Received: (from xxxxxxxx@localhost)
	by xxxx.cropdesign.local (8.11.6/8.11.6/SuSE Linux 0.5) id iA80B7217052;
	Mon, 8 Nov 2004 01:11:07 +0100
Date: Mon, 8 Nov 2004 01:11:07 +0100
X-Authentication-Warning: xxxx.cropdesign.local: xxxxxxxx set sender to Xxx
Xxxxx <[e-mail]> using -f
To: Yyyyyyyyy Yyyyyyy <[e-mail]>,
   Xxx Xxxxx <[e-mail]>,
Subject: Diffxxxxxxx
MIME-Version: 1.0
From: Xxx Xxxxx <[e-mail]>
X-Mailer: HTML Mime mail class (http://www.phpguru.org)
Content-Type: multipart/alternative;
	boundary="=_d944546a2735ed5c2d20baef1aeb3cf3"
Message-ID: <i6u36j.j2fxpp@>
X-Spam-Processed: cropdesign.com, Mon, 08 Nov 2004 01:11:11 +0100
	(not processed: message from valid local sender)
X-MDRcpt-To: [e-mail]
X-Rcpt-To: [e-mail]
X-MDRemoteIP: 10.1.1.11
X-Return-Path: [e-mail]
X-MDaemon-Deliver-To: [e-mail]
Reply-To: [e-mail]

--=_d944546a2735ed5c2d20baef1aeb3cf3
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 7bit


--=_d944546a2735ed5c2d20baef1aeb3cf3
Content-Type: text/html; charset="ISO-8859-1"
Content-Transfer-Encoding: quoted-printable

<table border=3D1><tr><th>XxxXx / XXxx</th><th>xxxxxxxxxx</th><th>XXX</th><=
th>xxxxxxxx</th><th>XXX</th></tr>

Comment 10

[David :Bienvenu]

do you have view | display attachments inline checked? If that's not checked,
you can run into this problem with some messages...

Comment 11

[Service Desk]

As proposed before, 

set the view options to "view message body as plain text or simple HTML"

Hopefully this resolves the bug.

Martin 

Comment 12

[Gabriel Alfonso]

I have the same problem in Mozilla Thunderbird 1.0.2 (20050317) with Windows XP.
Is it fixed in Thunderbird 1.0.6 ?

Thanks!

Comment 13

[Wolfgang Rosenauer [:wolfiR]]

I got a report that this error appeared in deerpark thunderbird (have to check
the detailed version). And "display attachments inline" is activated there.
Any logging which can be enabled to find out what happens?

Comment 14

[Piotr Komoda]

I get report in AviaryPL bugzilla system telling that this problem occur on Thunderbird pre-1.5 and after few hours of testing on Thunderbird 1.5 RC2 test build I'm able to fabricate message that will cause described problem. I can tell you this is not about headers. I've send two messages with identical body and text attachment, but in one message attachment was Windows-1250 encoded, and in other it was in UTF-8. Message with UTF-8 text file shows "This body part will be downloaded on demand" instead of body. The other one displays just fine. 

They both had the same headers (except of times, ID numbers and boundary numbers , which of course must differ) for body (sorry for those xxx's): 
From: Piotr Komoda <[e-mail]>
User-Agent: Thunderbird 1.5 (Windows/20051201)
MIME-Version: 1.0
To:  [e-mail]
Subject: Test IMAP 6
Content-Type: multipart/mixed;
 boundary="------------060103020005000405080707"
X-AntiVirus: pre-scanned (content of archives NOT checked)

This is a multi-part message in MIME format.
--------------060103020005000405080707
Content-Type: text/plain; charset=ISO-8859-2; format=flowed
Content-Transfer-Encoding: 7bit

And for attachment:
Content-Type: text/plain;
 name="2005-10-02-log.txt"
Content-Transfer-Encoding: 8bit
Content-Disposition: inline;
 filename="2005-10-02-log.txt"

So now we just have to look on what characters are making problems with mail.server.default.mime_parts_on_demand set to true. When this option is set to false, message displays alright. And yes, mail.imap.mime_parts_on_demand has nothing to do with this.

Comment 15

[Scott MacGregor]

flagging for investigation on the trunk.

Comment 16

[Joachim Achtzehnter]

I see this with TB 1.5 and it is rather frustrating. The work-arounds (switching the view to text or viewing message source) work but re-download the message via IMAP, which takes time and wastes my ADSL bandwidth.

It seems this happens often when somebody using Outlook and/or sending via an Exchange server sends an email with attachments in addition to a body. For example, just now I got one with the body and first attachment in text/plain and a second attachment text/xml. Below are some snippets from the message source window.

Joachim

X-MIMEOLE: Produced By Microsoft Exchange V6.0.6603.0
Content-type: multipart/mixed; boundary="Boundary_(ID_F/1M8LasFJSyVvaJjFQzyA)"

--Boundary_(ID_F/1M8LasFJSyVvaJjFQzyA)
Content-type: text/plain; charset=us-ascii
Content-transfer-encoding: 7BIT

...

--Boundary_(ID_F/1M8LasFJSyVvaJjFQzyA)
Content-type: text/plain; name=FILENAME.TXT
Content-transfer-encoding: 7BIT
Content-disposition: attachment; filename=FILENAME.TXT
Content-description: FILENAME.TXT

...

--Boundary_(ID_F/1M8LasFJSyVvaJjFQzyA)
Content-type: text/xml; name=file.xml
Content-transfer-encoding: 7BIT
Content-disposition: attachment; filename=file.xml
Content-description: file.xml

...

--Boundary_(ID_F/1M8LasFJSyVvaJjFQzyA)--

Comment 17

[David :Bienvenu]

is this the same as bug 321495 ? If so, it should be fixed in recent trunk builds.

Comment 18

[Mike Cowperthwaite]

(In reply to comment #17 [David Bienvenu])
> is this the same as bug 321495 ? If so, it should be fixed in recent trunk
> builds.

"This" meaning this bug, or the problem in comment 14 et seq?  Either way, I don't see the resemblance.

Comment 14, however, does mention for the first time in this bug the preference
  mail.server.server_X_.mime_parts_on_demand
which is described at bug 149771 comment 11 as a workaround.  The original report of this bug may be a dupe of that one.

Comment 19

[Gerald]

Bug is still present in TB 1.5.0.5 - i saw it today when trying to open an eMail in the "Sent" folder that consisted of 18 lines of message text, and two attachments: one 105kb .csv (comma-seperated-values) file and a 62kb .doc (microsoft word) file.

"View/Display Attachments Inline" made the message text appear, even though the message text itself was not an attachment and IMHO not even html.

Comment 20

[Michael Hallquist]

This bug is active in TB 2.0 Beta 2 and the behavior appears to have changed a bit from what others described. Specifically, selecting "display attachments inline" has no effect no matter its setting (on or off). I received a multipart/mixed MIME message and the first part (the text of the email) is a text file (below). The message initially displayed properly, but as soon as I downloaded one of the 6 attachments, I only see "This body part will be downloaded on demand." Now it won't go back to displaying the text email regardless of whether I close and reopen the program. Squirrelmail, my webmail client, has no problems displaying this message or downloading its attachments.

--=__PartFCD87876.0__=
X-Mozilla-IMAP-Part: 1
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

This body part will be downloaded on demand.

Comment 21

[Lance A. Brown]

I'm hitting this bug as well using Tbird 2.0.0.4.  Most of the instances I've hit are messages coming from AOL or Yahoo Mail.  I've got at least one example of a message that triggers the bug.

Comment 22

[raoul bhatia]

i'm too hitting this bug with one message. at first, i could save the attachments but now it is not working and i see no possibility to re-download the message.

Comment 23

[raoul bhatia]

ok, rebuilding the index solves this problem for me:

1) right click on the parent folder of the corrupted message
2) choose properties
3) choose "rebuild index"
4) wait for complete
5) done

Comment 24

[Rolf Leggewie]

(In reply to comment #23)
> 3) choose "rebuild index"

No such option here on Thunderbird version 1.5.0.12 (20070604) on ubuntu.

Comment 25

[Bill Scherlis]

Moving the msg to another folder solves the problem for me.

Comment 26

[Uli Stärk]

This bug is still present in version 2.0.0.6 (20070728).

Will attach a mail that causes the error.

Comment 27

[Uli Stärk]

Attachment: email that causes the "This body part will be downloaded on demand." error message with IMAP.

Comment 28

[Uli Stärk]

btw: Using the rebuild index option didnt fix the problem.

Comment 29

[Peter Lairo]

I see "This body part will be downloaded on demand" when I reply to some e-mails. It's the quoted part, instead of the text I'm replying to. Very confusing message, and frustrating to not have the quoted e-mail text.

Comment 30

[WADA:World Anti-bad-Duping Agency]

Next phenomenon itself is very easily observed;
  "This body part will be downloaded on demand" is displayed as HTML mail body
  even though mail data in offline-store is healthy.
by interfere of external program(file open of offline-store by external program.)

(1) Restart Tb, download all mail data in offline-store.
    Mail is displayed normally.
(2) Restart Tb(disable automatic check at startup, every NNN minutes for testing)
(3) Open(read open) offline-store file by external program.
(4) Open mail folder, view mail => "This body part will be downloaded on demand"
(5-A) If external program closes offline-store file
    before Tb detects error and considers it corruption of something,
    data in offline-store is normally read by Tb upon next click of the mail. 
(5-B) If external program keeps open of offline-store file for long time,
    and if Tb detects error and considers it corruption of something,
    (example. Compact Folders is invoked and fails due to interfere by others)
    "This body part will be downloaded on demand" is displayed,
    until external program closes offline-store and mail data is re-downloaded,
    by next auto-sych scheduling or by manual rebuild-index or by internal
    rebuild-index due something wrong.

Big mbox and big offline-store file?
External program such as auto-back up is used?

Comment 31

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #20)
> --=__PartFCD87876.0__=
> X-Mozilla-IMAP-Part: 1
> Content-Type: text/plain; charset=US-ASCII
>(snip)
> 
> This body part will be downloaded on demand.

"Partial data in offline-store" case is processed by Bug 468637.
Michael Hallquist, watch Bug 468637 if offline-use=on case, please.

Comment 32

[WADA:World Anti-bad-Duping Agency]

Phenomenon itself is very easily observed by next too.
  Delete offline-store file.
It can occur if "Compact Folders" is interfered by external program such as auto-backup. See Bug 498814 Comment #4 for it.

Comment 33

[WADA:World Anti-bad-Duping Agency]

Another case is found:
[ Step (8) of Bug 468637 Comment #10 with HTML test mail attached to Bug 501253 ]
  New mail arrives, and message headers are fetched.
  Before whole mail data is downloaded into offline-store, click mail.
    => "This body part will be downloaded on demand." is displayed.
  After a while, whole mail data is downloaded into offline-store by auto-sync.
    => "This body part will be downloaded on demand." is still displayed.
  Click other mail, and click the mail again.
    => Mail is properly displayed.

Comment 34

[WADA:World Anti-bad-Duping Agency]

To all problem reporters except bug opener:

What is your base or evidence that your case is same problem as original problem of this bug(comment #0)? Please note that "This body part will be downloaded on demand" itslef can be seen in some(not so small number) situations. Mixture of many different cases in a bug makes problem analysis impossible, and it'll wont produce solution. Please note that not all crash problem is same problem, even if "Tb crashes" is absolutely same.

To bug opener and all problem reporters:

"Partial data in offline-store" case was processed by Bug 468595. And Bug 468595 is already fixed. (See Bug 468595 Comment #55 for checkin id)
See also bugs listed in dependency tree for Bug 468595, please.

Can you reproduce original problem of this bug (comment #0) with newest nightly build, using two attached mails to this bug?
Can you reproduce your problem with newest nightly build?

>(Next Thunderbird 3.0)
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-comm-1.9.1/
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-comm-1.9.1-l10n/
>(Next SeaMonkey 2.0)
> http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/latest-comm-1.9.1/
> http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/latest-comm-1.9.1-l10n/
>(SeaMonkey trunk, with Gecko 1.9.3)
> http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/latest-comm-central-trunk/
> If MS Win, download win32.zip build. You can test by UNZIP only.

Note:
As I wrote in Bug 501253 Comment #21 to 23, and as I wrote in some previous comments, "This body part will be downloaded on demand" itself can be observed in
special situation.

Comment 35

[Felix]

I have experienced this in 3.0 rc1 (as well as repeatedly in 2.0)

As a normal user, this bug is very annoying! 
As an IT Manager it stops me recommending TB to my user base.

I would love to see it listed as a blocker.

I can help in debugging if given instructions.

Comment 36

[Mark Banner (:standard8)]

(In reply to comment #35)
> I would love to see it listed as a blocker.

With no steps to repeat and only very occasional reports of this we're not going to hold the significant upgrade of Thunderbird 3 from users. We'll consider a patch for a security & stability update if a solution can be found.

> I can help in debugging if given instructions.

Wada or David Bienvenu should be able to give you more instructions.

Comment 37

[Phil Lacy]

http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsIMAPBodyShell.cpp#747

the strlen of the text/html part, which is part "1", is not >= 2 so it is not fetched.

Comment 38

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #37)
> the strlen of the text/html part, which is part "1", (snip)

As seen in Bug 532779, Mozilla uses part=1.2 for second part in multipart/mixed.

Comment 39

[Phil Lacy]

In the second attached email this is the bodystructure the way Mozilla sees it:

Part 0          MIME-Version: 1.0
                Content-Type: multipart/related; boundary="=_a7feaa8c5fdfa8743acc7cadc190adf0"
                
Part 1          --=_a7feaa8c5fdfa8743acc7cadc190adf0
                Content-Type: text/html; charset="UTF-8"
                Content-Transfer-Encoding: quoted-printable
                
                
Part 2          --=_a7feaa8c5fdfa8743acc7cadc190adf0
                Content-Type: image/jpeg
                Content-Transfer-Encoding: base64
                Content-Disposition: inline; filename="Club-philips-logo_69x55.jpg"
                Content-ID: <8aa65aaf56c805fc71ef7c76fb22614b>
                
Part 3          --=_a7feaa8c5fdfa8743acc7cadc190adf0
                Content-Type: image/jpeg
                Content-Transfer-Encoding: base64
                Content-Disposition: inline; filename="clubphilips_nologo_560x100.jpg"
                Content-ID: <eda5d27e7e5fbc15d6fb48c5d6163fdd>
                
                
Part 4          --=_a7feaa8c5fdfa8743acc7cadc190adf0
                Content-Type: image/gif
                Content-Transfer-Encoding: base64
                Content-Disposition: inline; filename="mainlogo.gif"
                Content-ID: <ad177817e454e9f7ea4b1a57df1bae5b>
                
                
Part 5          --=_a7feaa8c5fdfa8743acc7cadc190adf0
                Content-Type: image/gif
                Content-Transfer-Encoding: base64
                Content-Disposition: inline; filename="solid.gif"
                
                --=_a7feaa8c5fdfa8743acc7cadc190adf0--

Comment 40

[Phil Lacy]

(In reply to comment #38)
> As seen in Bug 532779, Mozilla uses part=1.2 for second part in
> multipart/mixed.

if it is child of toplevel part 0 then it doesn't per here:
http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapServerResponseParser.cpp#2867

And the multipart with parent of rfc822/message takes on it's part number ("0" in this case) here:
http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsIMAPBodyShell.cpp#967

Comment 41

[Phil Lacy]

(In reply to comment #36)
> 
> With no steps to repeat and only very occasional reports of this we're not

STR:
1. write and save blank message to draft in local folders.
2. open drafts in editor and replace with attached second message all of the step one message from the x-mozilla flags down.
3. reindex local folder drafts and copy to IMAP folder for observation of problem.

Comment 42

[Phil Lacy]

based on attached email messages the dependency bug doesn't apply.
Please reset if I'm wrong.

Comment 43

[Phil Lacy]

Can we just fetch all with body type == "text" and let the display handle rendering plain text or sanitizedHTML or originalHTML.

Comment 44

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #40)
> (In reply to comment #38)
> > As seen in Bug 532779, Mozilla uses part=1.2 for second part in multipart/mixed.
> if it is child of toplevel part 0 then it doesn't per here:
> .../nsImapServerResponseParser.cpp

part=1.2 in that bug is for attachment display of mail in local mail folder. Sorry for my confusion and no care on IMAP.

Bug 516211 is for similar phenomenon to comment #0, to which next in comment #3 is applicable.  
> (Workaround: Select View/Attachement Display Inline)
In that bug, mail has comlex structure, and "Disk cache" is used by Tb 3 instead of very small memory cache for IMAP mail(may be 4MB).
Phil Lacy, can you look into Bug 516211.

Comment 45

[Phil Lacy]

Attachment: fetches text/html in body of top-level multipart (part 0)

Hopefully it is ok to submit a patch on your bug, but BODYSTRUCTURE seems to be problematic without tests and I don't mind debugging them.

This tests for bodystructures like:
(rfc822/message
  (related
     (text/plain)
     (text/html)
     (content other)
  )
)

This patch is not formatted for, hopefully, improved reading.

I tested with example 2 but that has text/html only. I'll try making a test email with text/plain only and see if it works.  I got a feeling it won't.

Comment 46

[Phil Lacy]

if a message body doesn't have text/plain but only text/html then setting to display original html shows download on demand (DOD) message, with this latest patch that is.

We really should be fetching all text body types. If not then the message is cached according to the setting of view->body-as, and then when the user switches it doesn't do another fetch it just displays the DOD message in cache.

Comment 47

[Phil Lacy]

Attachment: fetches text in body of top-level multipart (part 0)

this patch is built on top of bug 244741.

This gets text in top-level. It has a rather pathetic default of fetching the plain text if it is html and last text part. I don't know what case we have two text parts without alternative. Probably this can just load the text part regardless of preferPlainText setting? See what you think.

Comment 48

[Phil Lacy]

from this bug patch and bug 244741 I can solve the problems I see in bodystructure messages except one I fabricated which I can catch later:
part 0 message/rfc822
part 0 multipart/related
part 1 text/plain   download on demand under original html view
part 2 other
...
end

haven't tried but if the part 1 and plaintextonly setting were both toggled it may be DOD under plain view.

Comment 50

[David :Bienvenu]

Comment on attachment 417345 [details] [diff] [review]
fetches text in body of top-level multipart (part 0)

same comment as before about the unneeded elses...

I can't think of a way to write a unit test for this, unfortunately...

Comment 51

[Phil Lacy]

Attachment: added some reformatting and fix the 'else' nit

did some reformatting you better look at.

Comment 52

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #51)
> added some reformatting and fix the 'else' nit

If multipart/related, start parameter can be specified. Can your patch be torelant with start parameter?
(bug 471402 is for known problem when start is specified)

Comment 53

[Phil Lacy]

(In reply to comment #52)
> (In reply to comment #51)
> > added some reformatting and fix the 'else' nit
> 
> If multipart/related, start parameter can be specified. Can your patch be
> torelant with start parameter?
> (bug 471402 is for known problem when start is specified)

I can't test, that bug applies to local store as well.

Comment 54

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #53)
> I can't test, that bug applies to local store as well.

I see. I'll check wheteher IMAP specific problem occurs or not in bug 471402, after this bug wiil be fixed.

Comment 55

[Phil Lacy]

Comment on attachment 421759 [details] [diff] [review]
added some reformatting and fix the 'else' nit

>+      // This is the first text part of a top-level multipart of the toplevelmessage
>+      if (m_parentPart->GetType() == IMAP_BODY_MULTIPART &&
>+          !PL_strcmp(m_parentPart->GetPartNumberString(), "0") &&
>+          !PL_strcasecmp(m_bodyType, "text") &&
>+          ((!PL_strcasecmp(m_bodySubType, "plain") && preferPlainText) ||
>+            (!PL_strcasecmp(m_bodySubType, "html") && !preferPlainText)))
>+         return PR_TRUE;
> 

If this is the first text part of a top-level multipart of toplevel message then won't it only have one text part and we should fetch it regardless of preferPlainText setting?

Comment 56

[Phil Lacy]

yes that takes care of the plain text part I mentioned in IRC

multipart/related

text/plain

image/jpg other parts.

The above top level text was download on demand if view was set for HTML.
A multipart/related wouldn't happen with plain text I don't think but any text should be fetched and only those in alternative need scrutiny.

Comment 57

[Phil Lacy]

Comment on attachment 421759 [details] [diff] [review]
added some reformatting and fix the 'else' nit

needs further work

Comment 58

[Phil Lacy]

Attachment: added some reformatting and fix the 'else' nit

the previous patch fetched all text even attachments txt files.  This limits it to part 1 of top level.  I have a reminder comment that there is nothing to limit ordering of parts in messages. If you know of one I can remove the comment and we are ok to go.  If not we need to check for disposition 'attachment'

Comment 59

[Phil Lacy]

Attachment: adds body disposition and body language to imapbodyshell, rfc 3501

See what you think about getting these added to a body shell for leaf

Comment 60

[David :Bienvenu]

Comment on attachment 422088 [details] [diff] [review]
added some reformatting and fix the 'else' nit

wouldn't this just be return (if clause) ?

+      if (m_parentPart->GetType() == IMAP_BODY_MULTIPART &&
+          !PL_strcasecmp(m_bodyType, "text") &&
+          !PL_strcmp(m_parentPart->GetPartNumberString(), "0") &&
+          !PL_strcmp(m_partNumberString, "1"))
+         return PR_TRUE;
+
       return PR_FALSE;  // we can leave it on the server

In any case, testing the patches now.

Comment 61

[Phil Lacy]

(In reply to comment #60)
> (From update of attachment 422088 [details] [diff] [review])
> wouldn't this just be return (if clause) ?
> 

I like that. Bug 244741 introduces some more instances of just returning the if clause. You can see them in the upstream from this clause as that bug needs to be applied before this one.  I can make fix those here as well.

Comment 62

[Phil Lacy]

(In reply to comment #61)
 
> ...some more instances of just returning the if
> clause....

ha ignore this if you haven't aleady

Comment 63

[Phil Lacy]

Attachment: composite patch of 244741 and 246415

Comment 64

[David :Bienvenu]

Comment on attachment 425603 [details] [diff] [review]
composite patch of 244741 and 246415

this works for me - Phil, do you want me to just land this combined patch?

Comment 65

[Phil Lacy]

yes, I wouldn't wait for my tests even though I'm gradually making some progress on them.

Comment 66

[David :Bienvenu]

fix checked in - should both these bugs be resolved fixed? I agree that holding for the tests is not the right thing to do, since they're going to involve adding whole new functionality to the fake imap server.

Comment 67

[Kent James (:rkent)]

If by "fix checked in" you mean http://hg.mozilla.org/comm-central/rev/7e42eca3bfaf as its comment implies, this is just a change to a comment. Are you sure that you got the right patch?

Comment 68

[David :Bienvenu]

thx very much for catching that - correct patch pushed now.

Comment 69

[Kent James (:rkent)]

(In reply to comment #68)
> thx very much for catching that.

Thanks to an RSS feed https://hg.mozilla.org/comm-central/atom-log of all changes to comm-central that I scan sometimes.

Comment 71

[Phil Lacy]

David still needs to give some thought to attachment 422089 [details] [diff] [review]

Comment 72

[David :Bienvenu]

Comment on attachment 422089 [details] [diff] [review]
adds body disposition and body language to imapbodyshell, rfc 3501

until we have a use for it, I don't want to add code that parses it because it can only add risk...but if we can find a use for it, and expose it to the outside world, then we can reconsider.

Comment 73

[Phil Lacy]

(In reply to comment #72)
> (From update of attachment 422089 [details] [diff] [review])
> until we have a use for it, I don't want to add code that parses it because it
> can only add risk...but if we can find a use for it, and expose it to the
> outside world, then we can reconsider.

re: your one message that displayed attachments inline as 'download on demand'

"
html or plain text message.

---------------------------------------------------------
This part will be downloaded on demand
"

This is what made me think about adding the body disposition so we can determine that it is an attachment and not inline, if that makes sense.
I don't think it should even show a placeholder for an attachment like this. 

I'm not sure if that is an editor or mime bug or if it is intentional.

Comment 75

[Phil Lacy]

Bug 540676 has test for this.