82764 - Undo is broken for across-server move/copy for multiple messages

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Navin Gupta]

I will investigate for all the different combinations between NNTP, IMAP and 
local. It looks like only NNTP to local is working right now. 

Also we are still doing move across server when we should be doing a copy.

Comment 1

[Navin Gupta]

moving to mozilla 0.9.2. 

Comment 2

[Navin Gupta]

Attachment: proposed fix.

Comment 3

[Navin Gupta]

The first part of the fix are the js changes that do a copy when the servers are
different. The next is when we copy multiple imap messages to local we were 
not adding the msgKey in EndMessage() that gets called after each message. 
Also prevent adding the key in another place if multiple messages are being 
copied from imap to local. When we do local/nntp to imap we need to add the 
msgIds to the imapundo object, also when we issue a search for finding the uid 
we need to be in the selected state. 

cc david for review. 

Comment 4

[David :Bienvenu]

r=bienvenu, cc'ing Seth for sr.

Comment 5

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

everything looks ok, but instead of adding something specific like
"GetSrcIsImap()", would it be better to add "GetSrcFolder(nsIMsgFolder
**folder)" and have the caller QI to see what the folder type is?  What if later
a caller wants to tell if the src folder is news, or another type?

Comment 6

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

[minor nit] it should be GetSrcIsIMAP(), not GetSrcIsImap()

sr=sspitzer

concerning my earlier comments (about making the solution generic):

naving is just adding a getter to get at a member variable on the undo 
transaction, (m_srcIsImap4), so I can't object to that.

in an ideal world,  nsLocalUndoTxn.cpp would be written in a generic way, not 
depending on IMAP.  but that's not going to happen any time soon [and we've got 
bigger fish to fry], so there is no reason to make naving go out of his way to 
make his patch general when the underlying code isn't.

that would just be re-arranging the chairs on the titanic

Comment 7

[scottputterman]

adding PDT+.  Please check into the trunk as soon as possible.

Comment 8

[Christopher Blizzard (:blizzard)]

a=blizzard on behalf of drivers for the trunk

Comment 9

[Navin Gupta]

fix checked in. 

Comment 10

[Sheela Ravindran]

I moved and copied about 5 messages which were both plain text and html message.
The following scenariors were tested with move-undo move, copy-undo copy.
imap-local folders, local-imap, news-imap(copy only), news-local(copy 
only),imap-pop,pop-imap, local-local, imap-imap, pop-pop. I used inbox, drafts, 
userdefined folders for moving, copying message with the above scenarios.

If you find any other specific scenario which has problems with move, copy or 
undo please log a seperate bug for each different issue and do not reopen this 
bug.  

builds used in verifying the above:  2001061807win98, 2001061808 linux, 
2001061808 mac.