101480 - when replying to an IMAP message, fetch by part (so we only get the body, not the attachments)

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P2)

Description

[(not reading, please use [e-mail] instead)]

when replying to an IMAP message, fetch by part (so we only get the  body, not 
the attachments)

looking at an IMAP protocol log from 4.x, we used to just get the body on reply.

I've got a patch that accomplishes the goal, but it's a hack.

mscott / bienvenu, can you review?

Comment 1

[(not reading, please use [e-mail] instead)]

Attachment: hack fix

Comment 2

[(not reading, please use [e-mail] instead)]

from nsMsgQuote.cpp:

  if (!bAutoQuote)
    modifiedUrlSpec += "?header=only";
  else if (quoteHeaders)
      modifiedUrlSpec += "?header=quote";
  else
      modifiedUrlSpec += "?header=quotebody";

if this is the right approach, I might need to extend my change to nsImapUrl.cpp

Comment 3

[David :Bienvenu]

honestly, I'm not sure if this is the right fix or not. It could be. I'll go
look at 4.x and see what it did. Scott and I have made a few changes to this
logic in the past few months to fix a few bugs and we might have inadvertently
broken something. Or not...

Comment 4

[(not reading, please use [e-mail] instead)]

bienvenu, thanks for looking into how 4.x did this.  

adding perf keyword, since doing something like this would improve reply 
performance when replying to an imap message with an attachment.

instead of adding that call to PL_strstr() [which will be executed more often 
than necessary],  my original approach was to explicitly call 
SetFetchPartsOnDemand() in nsMsgQuote in the case where we wanted the body.  
But that made compose depend on imap.

But I could add something protocol agnostic to the nsIMsgMailNewsUrl interface, 
have the base implementation do nothing, and have the nsImapUrl implementation 
call SetFetchPartsOnDemand().

comments?

Comment 5

[Sheela Ravindran]

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

Comment 6

[(not reading, please use [e-mail] instead)]

over to cavin to do the investigation and finish this off.

let me know if you have questions, cavin.

if we can figure this out, it would be a big perf win for replying to messages 
with attachments.

Comment 7

[Cavin Song]

Seth's fix is almost perfect and I just added one more case ("?header=only") to 
the condition test.  "?header=only" is set when "Automatically quote original 
message when replying" is turned off (Edit|Preferences|Mail & Newsgroups|Message 
Compose). A patch is coming.

Comment 8

[Cavin Song]

Attachment: A revised patch to seth's.

Comment 9

[(not reading, please use [e-mail] instead)]

wow, we were fetching the whole message (and attachments!) on reply when we 
weren't even quoting?

looks good to me, but we need david / mscott to review.  

Comment 10

[scottputterman]

Scott or David, when you get a chance, could you review this?

Comment 11

[Scott MacGregor]

Comment on attachment 52070 [details] [diff] [review]
A revised patch to seth's.

sr=mscott

Comment 12

[Cavin Song]

Fix checked in.

Comment 13

[Eric Vaandering (no email)]

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

Comment 14

[Sheela Ravindran]

Tested this with changing prefs for replying(quote above, below). With messages 
that are displayed inline and with binary attachments.  The ones we display 
inline like gif, jpg shows up in the reply window but not in the attachment 
area.
We don't fetch the attachment part anymore when replying to a message with 
attachment. This is true in case of mail. Please see bug 110364 this seems to be 
not working with news.  I am verifying this bug since I logged 110364 for news.

Build id: 2001-11-15-06 win98, mac os x, linux. 

Comment 15

[(not reading, please use [e-mail] instead)]

this has regressed, around the time that quoting broking broke when there was 
that big necko landing.  I'm working on the regression in 
http://bugzilla.mozilla.org/show_bug.cgi?id=117864