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

VERIFIED FIXED in mozilla0.9.2

Status

VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: naving, Assigned: naving)

Tracking

Trunk
mozilla0.9.2
x86
Windows NT

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PDT+]Have fix)

Attachments

(1 attachment)

(Assignee)

Description

18 years ago
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.
(Assignee)

Comment 1

18 years ago
moving to mozilla 0.9.2. 
Target Milestone: --- → mozilla0.9.2
(Assignee)

Comment 2

18 years ago
Created attachment 36428 [details] [diff] [review]
proposed fix.
(Assignee)

Comment 3

18 years ago
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. 

Updated

18 years ago
QA Contact: esther → sheelar

Comment 4

18 years ago
r=bienvenu, cc'ing Seth for sr.
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?
(Assignee)

Updated

18 years ago
Whiteboard: Have fix
[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

17 years ago
adding PDT+.  Please check into the trunk as soon as possible.
Whiteboard: Have fix → [PDT+]Have fix

Updated

17 years ago
Blocks: 83989
a=blizzard on behalf of drivers for the trunk
(Assignee)

Comment 9

17 years ago
fix checked in. 
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 10

17 years ago
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. 
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.