David :Bienvenu Reporter
	Description • 15 years ago

If you move/copy multiple imap messages to a local folder, we run it as a single imap url with multiple messages. This bypasses the offline store cache checking, which means we'll fetch the messages from the server unnecessarily if we have the messages in the offline cache.

To fix this, I think we need to preflight the move/copy, and check if we have all the messages offline - if so, just copy the offline copies to the local folder. If we have a mix of offline and not offline messages, we could break it into two operations, but the homogeneous case is the low-hanging fruit.

	Wayne Mery (:wsmwk)
	Comment 1 • 15 years ago

hmm, that explains slowness (I frequently do this)

	Magnus Melin [:mkmelin]
	Comment 2 • 15 years ago

This sure would be nice.

	Wayne Mery (:wsmwk)
	Comment 3 • 14 years ago

if Bug 579520 doesn't get closed for odd reasons, perhaps this would help him.

	Magnus Melin [:mkmelin]
	Comment 6 • 5 years ago

Comment on attachment 9124922 [details] [diff] [review]
505456-proposed-change.patch

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

Nice low hanging fruit! r=mkmelin

::: mailnews/local/src/nsLocalMailFolder.cpp
@@ +1321,5 @@
>    nsCString protocolType;
>    rv = srcFolder->GetURI(protocolType);
>    protocolType.SetLength(protocolType.FindChar(':'));
>  
> +  // If TB is offline and the source folder is imap or news, to do the

nit: we usually comment in the "we" form.
If we're offline....

@@ +1343,5 @@
>        */
>        totalMsgSize += msgSize + 200;
>  
> +      // Check if each source folder message has offline storage regardless
> +      // of whether TB is online or offline.

here too

@@ +1349,5 @@
> +      bool hasMsgOffline = false;
> +      srcFolder->HasMsgOffline(key, &hasMsgOffline);
> +      allMsgsHaveOfflineStore = allMsgsHaveOfflineStore && hasMsgOffline;
> +
> +      // If TB is offline and not all messages are in offline storage, the copy

and here

@@ +1489,5 @@
>        (void)OnCopyCompleted(srcSupport, false);
>      }
>    } else {
> +    // This obtains the source messages from local/offline storage to do the
> +    // copy. Note: CopyMessageTo() actually handles one or more messages.

Maybe as a follow-up we could split up into two arrays and do both cases: first copy what we got locally, and only after that go and copy the ones we didn't have locally yet.

	Magnus Melin [:mkmelin]
	Comment 8 • 5 years ago

Comment on attachment 9125139 [details] [diff] [review]
505456-proposed-change(fixup1).patch

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

Thanks Gene! Looks good. One thing: the commit message should start "Bug 505456 - ", not "Bug 505456:"
I can fix that before landing.