Closed Bug 46233 Opened 24 years ago Closed 21 years ago

Attachments downloaded again when saved

Categories

(MailNews Core :: MIME, defect, P3)

Product:
MailNews Core
Component:
MIME
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: phil, Assigned: Bienvenu)

References

Details

(Keywords: perf, Whiteboard: [nsbeta3-])

Attachments

(1 file, 3 obsolete files)

proposed fix
2.70 KB, patch
sspitzer
: superreview+
Details | Diff | Splinter Review 
Using 2000-07-21-08 commercial build on NT

1. Send yourself a large HTML document as an attachment (not a web page)
2. Read the message, HTML document displays inline
3. Save the attachment

Actual: it takes ~10 secs (esp. over a slow link) to download some or all of the
message again, status bar says "downloading message..."

Expected: since the HTML document was displayed inline, it should be quickly
accessible to be saved.
Hmm...will have to see why the cache isn't stepping in here. 

Scott, any ideas?

- rhp
Status: NEW → ASSIGNED
Target Milestone: --- → M18
QA Contact: lchiang → pmock
Hi Scott,
This was the cacheing issue I was telling you about. Thanks!

- rhp
Assignee: rhp → mscott
Status: ASSIGNED → NEW
performance bug...adding keywords
Keywords: nsbeta3, perf
mail triage marking nsbeta3-
Whiteboard: [nsbeta3-]
Assign it to myself.
QA Contact: pmock → fenella
To Esther..
QA Contact: fenella → esther
Target Milestone: M18 → ---
Occurs for all attachments... Very painful!

I have noticed: if the attachment appears inline, right-click over the inline
object and select "Save As..." will save to disk from the cache, no network
activity at all, yet doing the same from the attachments pane reloads it from
the network.

This is a big problem if you are on a slow link and have multiple messages with
large attachments. I spent over thirty minutes downloading some news, _twice_,
saving the damned attachments. I'm on Linux.

If this isn't nscatfood nothing is. BTW, sometimes the same reloading from the
network occured in Nav4.x and was one of the more infuriating bugs.
Keywords: mozilla0.9.3, mozilla1.0, nsCatFood
OS: Windows NT → All
Hardware: PC → All
Summary: HTML attachment downloaded again when saved → Attachments downloaded again when saved
*** Bug 83979 has been marked as a duplicate of this bug. ***
Darin, Doug: you might like to take a peek and confirm the attachments are
indeed in the cache and that the mailnews code can get to them; if not, this is
not mscott's :)

Should this really be in the MIME component?
*** Bug 92875 has been marked as a duplicate of this bug. ***
*** Bug 87311 has been marked as a duplicate of this bug. ***
*** Bug 87344 has been marked as a duplicate of this bug. ***
Antonio,

Seems reasonable to dupe this against bug 93208.
This is not a duplicate of bug 93208 - read the bug descriptions closely.
Ok I got it now.  Thanks Hakan, bienvenu's comment also helped explain the
difference in the other bug, what threw me was that they are about downloading
messages more than once, vs the distinct differences here.
QA Contact: esther → trix
when mozilla open a mail from imap server,it will only cache mail body,so when 
save the attachment,mozilla will reload from imap server for attachment. So i 
want to cache attachment when mozilla run in mimemoz2.cpp,but i can't get the 
inputstream object,i seriously want to fix this bug,but now i really need 
afflatus.who can give some advice!!!!!
David: is this really MIME?
Phil: Do you see this on IMAP and/or POP?
This is really imap, and Phil is long gone. I would think that this is bug is
invalid in most cases now that we use the memory cache (it would still exist in
cases where the message had both inline and non-inline parts, because those
messages don't get put in the memory cache). I don't know how to fix that case,
other than putting the html page in the memory cache like we do for images, or,
put the partial message in the memory cache and handle requests for the
attachments we have downloaded by reading from the memory cache. I don't know
how to do the former, and the latter is extremely dangerous because if we put
partial messages, in the memory cache, we have to be extremely careful not to
think we have the whole message in the cache for things like forward, etc. I
don't recommend either approach.
Hi Bienvenu,i add a patch,this patch can fix thix bug for imap4,so please rv my
patch,and please give me some advice.this patch can make mozilla open mail body
from cache,when i save a attachment.but when mozilla open a mail from a
IMAP4Rev1 server,if the mail have attachment,mozilla will only load mail
content,and siplay mail,but mozilla didn't load mail's attachment,so i can't
cache attachment before i  save the mail's attachment.
Hi Mscott,please Assign this bug to me,and please rv my patch
I've explained this in a private e-mail a week ago, but I'll put the info in here:

After stepping through the code, it looks to me like the problem is that for
html attachments, we're downloading the whole message (instead of just the html
part) to save the html attachment. I believe this is because the url for the
attachment is of a form that we are not parsing correctly in
nsImapService::FetchMimePart. If you set a breakpoint here, and compare the urls
for saving a .exe attachment vs a .html attachment, you'll see the difference.
One possibility is to fix the url stored in the attachment data for the .html
attachment to be like the .exe attachment url. I'm not familiar with this code,
so I'm not sure why they're different.

good format: "imap-message://bienvenu@nsmail-1/Personal#7568?section=2&part=1.2"
bad
imap-message://bienvenu@nsmail-1/Personal#7318imap://bienvenu@nsmail-1:993/fetch>UID>/Personal>7318?part=1.2"

the good one is for non-inline attachments like .exe files. The bad one is for a
.html attachment. I tried a jpeg as well, and it had the bad format:-
"imap-message://bienvenu@nsmail-1/Personal#150imap://bienvenu@nsmail-1:993/fetch>UID>/Personal>150?part=1.2"

Note that the "bad format" is the imap url format with the fetch instruction
coded in.
Hi Bienvenu,i don't know whether,you can see my comment,but i will explain what
elements of my fix.
    In imap4,mozilla will load all mail body from imap4 server and pick up the
attachment from mail body ,when a attachment be saved in the first,so my fix is
do that,when save a attachment,i will force mozilla load mail body from
cache,because mozilla will cache all mail content when i open a mail,so after i
open a mail,all of the mail's content is cached by mozilla.
    In imap4rv1 ,mozilla will only load part of mail,that can be displayed,so i
can't fix it in here,because i can't load all of mail'content from cache.
    Now have some problem, When i add your patch for bug93208,mozilla won't
display all img in imap4rv1,i hope you can see my patch,and tell me how to judge
the types of the imap server in nsImapMockChannel::OpenCacheEntry,because your
patch is for imap4rv1,i must avoid conflict with your patch.
 

I'll just point you at my previous comment again. I don't think the way you're
fixing this is the right way to fix this. The way to fix this is to fix the
attachment url. At least that's the approach we're going to try.
i test this fix in imap4,all attachment can load from cache,so this way is 
force mozilla pick up attachment from mail body in the cache,but in imvp4rv1,i 
can't do it.i notice the "section" in imap url,i will try to debug it.
with imap4rev1 servers (the vast majority of servers out there, btw), we fetch
the body structure of the message and don't fetch the non-inline parts when we
display the message. This makes things very complicated, as I've explained in
this bug, and related bugs.
In imap4rv1,i can't get non-inline parts from cache,but in imap4,i can do it,so 
my patch is for imap4,but i don't know how do judge the types of the imap 
server in nsImapMockChannel::OpenCacheEntry?

Hi bienvenu,thank for your advice,i change my patch to ignore img url,so when 
mozilla will display img,i don't change url,i think url is a key in cache,we 
can get mail's data from cahce with key,so in imap4,when mozilla display a 
mail ,it will cache all mail'data that include non-inline parts,so i will force 
mozilla fetch attachment from cache,in imap4rv1,when mozilla display a mail,it 
will only mail body and inline parts,so when i save a attachment ,mozilla will 
load from imap server,but all inline parts will cache,like html file,so i can 
force mozilla save the html file from cache,so i must change ths key when 
mozilla query cache with key.
this change is to avoide conflict with bug93208's patch,so please rv my patch!
again, I believe the correct fix is to fix the code that's generating these part
urls in the attachment menu, not to patch the imap cache code to deal with the
mal-formed urls.
Looks like this one is hibernating. 
I'm seeing this bug with uuencoded attachments. When I open the message from
IMAP server, Mozilla loads entire encoded text attachment too. Save the
attachment and it happens again. If you get impatient and move into 2nd message
while the attachment from the 1st one is downloading, you get a partial download.
*** Bug 176088 has been marked as a duplicate of this bug. ***
Related: Bug 181842

Just to confirm that this bug still exists in release 1.2. For people working on
slow links, this merits a priority > P3 -- try it sometime.

Maybe the following should be an RFE (or even 2 RFEs), but I will drop them here
since they are so closely related and could potentially be addressed at the same
time as this bug:

1) An option to not (automatically) download all attachments, or just not
download attachments greater then N bytes would be welcome for users on slow links.

2) When an attachment is displayed in the mail window, there appears to be no
way to get back to the initial mail without selecting another mail (if there is
one) then re-selecting the mail you want to see.
*** Bug 144346 has been marked as a duplicate of this bug. ***
My testings show that attachments in a news group message ARE downloaded with
the message. The text of the message is NOT displayed until all of the message
plus all attachments are loaded. Hitting <ctrl-u> to get the source code, shows
that the attachment(s) are there, included inside the message. If I want to save
the attachment, a zip file for example, Mozilla proceed to download a new copy
of it.

Alain
is Bug #131586 copy of this one ?
There may be a relation or some similarity. This one concern attachements in
news messages. The attachements ARE downloaded before vewing the message, then
again if you want to save them. An image saved by the right menu is instantly
saved, but redownloaded if you then save it using the attachment list.
as I see it
probably same patch could fix both problems

if before downloading message there is verification
is it localy stored (and if it is verify has it been shanged since)
This one is for news messages? I didn't know that.. 

FWIW, if you maximize the attachment by double-clicking on it, it's NOT
downloaded again for the new window. You may then do "save page" and it's saved
without downloading again. Concerning IMAP mail. 
It is related to bug 183068 and 181842.
This issue is also a shocker for multimegabyte images being displayed inline.
They must be downloaded again to be saved, and given the slow download speeds
(see bug 182383) can be major slowdown.

I've seen a user sit for 30 seconds waiting for a message with a large image to
download, then have to wait for the same time again after saving the image to disk.

I'm fairly sure the server in question (MDaemon 3) is NOT imap4r1 compatable,
however speeds aren't much better working from Cyrus in some quick tests I did.

Craig Ringer
Um, is it just me, or has this bug been fixed?  Using Save Image from a binaries 
newsgroup message with an image attachment, the image is being saved immediately 
without a re-download.  Can anyone confirm?
  Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5b) Gecko/20030820
Mike, if you save the image by right clicking on the image itself, we get the
image from the memory cache. But, if you use the attachment list, and save as
from there, we don't get the attachment from the memory cache, because, as I
said before, the url in the attachment menu doesn't match what we've put in the
memory cache.
Assignee: mscott → bienvenu
Eek, I just spent half an hour saving 24 attachments of a message. All the
attachments were shown inline (text and jpg's) and it took initially about 65
seconds to load the message to display. We need to get this fixed..
I'll try to fix the URI's run when we do save so that they hit the memory cache
correctly.
Status: NEW → ASSIGNED
I poked around a bit and found that we mess a perfectly good url in
nsImapService.cpp around line 600 by adding extra stuff into it. I changed it so
that only "?header=print" is appended if printing and it seemed to cure the two
cases I tried. 

In nsMessenger::SaveAttachment, I also removed a suspicious assignment of
fullMessageUri to messageUri, but didn't have time to check it thoroughly yet.

Thoughts?
Ere, you're right about the first part. I'm not sure the assignment is
suspicious, or, rather, it might be the whole problem - I'll have to step
through the code, and try it with local mail as well.
OK, I think that in some cases the code ~line 600 of nsImapService.cpp is
required, e.g., double clicking on an attachment in the attachment list.
would soulution for this solve also bug #131586
it is not that only attachments are downloaded 
again and again but whole messages are redownloaded
on every change of focus (and again solving that issue
would probably solve this one too :)
That raises the question why the is URI different when double-clicking and when
saving. I think it shouldn't make any difference for FetchMimePart().
yes, I'll have to sit down and figure out the various code paths.
I think the problem is here in nsMessenger::SaveAttachment - mimePart ends up
being the whole fetch url, so fullMessageUri is ends up being two uri's
concatenated together.

      {
        PRInt32 sectionPos = urlString.Find("?section");
        nsCString mimePart;
        urlString.Right(mimePart, urlString.Length() - sectionPos);
        // ###this is bogus - this is much more than the mime part - it's the
whole uri
        fullMessageUri.Append(mimePart);
   
        messageUri = fullMessageUri.get();

I don't know if this fixes the problem, but it prevents us from generating that
very wrong uri.
Attachment #131711 - Flags: review?(ere)
*** Bug 181842 has been marked as a duplicate of this bug. ***
*** Bug 210996 has been marked as a duplicate of this bug. ***
Attached patch proposed fix — — Splinter Review 
this fixes it so that both save as and open get the attachments from the memory
cache.
Attachment #67388 - Attachment is obsolete: true
Attachment #68096 - Attachment is obsolete: true
Attachment #131711 - Attachment is obsolete: true
Attachment #139278 - Flags: superreview?(mscott)
Attachment #139278 - Flags: superreview?(mscott) → superreview+
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Does this fix in any way affect or improve the way e-mails with attachments are
moved from the inbox to the local folders (or another account's folder)?

Sorry for bugspam, but a query for "attachment local folders" wasn't successful.
no, it has no effect on that. Moving messages to local folders or other accounts
fetches the whole message. If it's in the memory cache, the cached copy should
be used...
*** Bug 119049 has been marked as a duplicate of this bug. ***
David, think we should put this on the M4 branch?
Blocks: 230700
yes, I'll put it on the m4 branch.
fixed on m4 branch.
Attachment #131711 - Flags: review?(ere)
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: