Copy folder from IMAP or POP to Local folder only works once.

RESOLVED FIXED

Status

RESOLVED FIXED
16 years ago
14 years ago

People

(Reporter: Frank.Harper, Assigned: sspitzer)

Tracking

({regression})

Trunk
x86
Windows NT
regression

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
User-Agent:       Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.3a) Gecko/20021212
Build Identifier: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.3a) Gecko/20021212

I'm copying folders from an IMAP server to Local Folders, using drag 'n drop.
This works for one folder, but when I do the next one nothing happens. If I quit
and restart Mozilla I can then copy another folder.

Reproducible: Always

Steps to Reproduce:
1. Drag one folder from IMAP server to Local Folder
2. Drag another folder from IMAP server to Local Folder
3. Check that contents of second folder have been copied.

Actual Results:  
No visible results.

Expected Results:  
Copied messages from IMAP folder to Local Folder.

Updated

16 years ago
QA Contact: laurel → gchan

Comment 1

16 years ago
confirmed with current trunk
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030113


worked in 7.01.

nominating

couldn't find any dupes. similar bugs:
bug 175819,bug 142448, bug 180229
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: nsbeta1, regression

Updated

16 years ago
Keywords: nsbeta1+

Updated

16 years ago
Keywords: nsbeta1

Comment 2

16 years ago
Mail triage team: nsbeta1+/adt2
Whiteboard: [adt2]

Comment 3

16 years ago
Update, current behavior with build 20030326 on winxp and linux (haven't tested
Mac yet). 
1. Launch app and open Mail with IMAP account
2. DND an IMAP folder to top level of Local folder
Result: Folder and it's messages are copied to Local folders
3. DND another IMAP folder to top level of Local folder
Result: Folder does not show up in Local folders.
4. Copy or Move a message from any IMAP folder to any Local folder
Result: Message is moved AND the 2nd Folder that was dragged to Local Folders
now shows up with all it's messages.  
Note: Exiting and relaunching after step 3 does not display the 2nd folder only
a copy or move to any Local Folder will cause the 2nd folde to be displayed. 
This is regression and pretty bad. 

Comment 4

16 years ago
I tested bug 175819 which I believe is using POP accounts and Local folders and
found the same problem.  Marking that a dup of this bug and changing summary to
reflect it happens with IMAP and POP.
Summary: Copy folder from IMAP to Local folder only works once. → Copy folder from IMAP or POP to Local folder only works once.

Comment 5

16 years ago
*** Bug 175819 has been marked as a duplicate of this bug. ***

Comment 6

16 years ago
*** Bug 142448 has been marked as a duplicate of this bug. ***

Updated

16 years ago
QA Contact: gchan → esther

Comment 7

16 years ago
Taking.
Assignee: sspitzer → cavin

Comment 8

16 years ago
Created attachment 122496 [details] [diff] [review]
Proposed patch, v1

The problem is that FindRequest() can't find the right request when copy
finishes. FindRequest() is called with 'dstFolder' points to the newly created
folder (for example, "Local Folders\Test1") and it's never the same as
copyRequest->m_dstFolder (for example, "Local Folders") which is the parent
folder of the copied folder. The check works fine for copy msgs operations
because the destination folders never change but it does not work for copy
folder operations since a new folder is created in this case.

Updated

16 years ago
Attachment #122496 - Flags: superreview?(sspitzer)
1)

I'd rather we didn't use filespec, which is obsolete, and I don't even think we
need to.

I think you can just use the folder name, right?

  nsXPIDLString folderName;
  rv = m_dstFolder->GetName(getter_Copies(folderName));

if so, instead of nsCString, use nsString for m_dstFolderName

also, when do you clear m_dstFolderLeafName?  shouldn't you be doing that when
you clear m_dstFolder?

2)

also, what about copying folders between imap servers?
Comment on attachment 122496 [details] [diff] [review]
Proposed patch, v1

see my previous comments / questions.
Attachment #122496 - Flags: superreview?(sspitzer) → superreview-

Comment 11

16 years ago
> I think you can just use the folder name, right?
> if so, instead of nsCString, use nsString for m_dstFolderName
>
Yes, good suggestion.

> also, when do you clear m_dstFolderLeafName?  shouldn't you be doing that when
> you clear m_dstFolder?
>
m_dstFolderLeafName is a member of CopyRequest and CopyRequest is created per
copy operation and destroyed when copy finishes so we don't have to clear
m_dstFolderLeafName.

> 2) also, what about copying folders between imap servers?
>
Copying folders between imap servers is not allowed. You can move an imap folder
underneath another within the same server but that does not use copy service code.

Comment 12

16 years ago
Created attachment 122598 [details] [diff] [review]
Proposed patch, v2

Incorporated comments.
Attachment #122496 - Attachment is obsolete: true

Updated

16 years ago
Attachment #122598 - Flags: superreview?(sspitzer)
Comment on attachment 122598 [details] [diff] [review]
Proposed patch, v2

r/sr/a=sspitzer for 1.4 final.

one nit, no need for a new patch:

+    nsCOMPtr<nsIMsgFolder> srcFolder;
+    srcFolder = do_QueryInterface(aSupport, &rv);

just do

+    nsCOMPtr<nsIMsgFolder> srcFolder = do_QueryInterface(aSupport, &rv);

thanks for fixing this cavin.
Attachment #122598 - Flags: superreview?(sspitzer)
Attachment #122598 - Flags: superreview+
Attachment #122598 - Flags: review+
Attachment #122598 - Flags: approval1.4+

Comment 14

16 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
*** Bug 204927 has been marked as a duplicate of this bug. ***

Comment 16

16 years ago
using trunk build 20030516 on winxp, macosx and linux this is fixed for IMAP,
but when I tested moving POP folders to Local it didn't work after the 1st move.
 From that point on the IMAP didn't work.  I'm reopening this to Cavin can fix
the POP case too. 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 17

16 years ago
The problem is that if the source folder is empty (ie, no msgs) then
notification is never received. In
nsMsgLocalMailFolder::CopyFolderAcrossServer() we should call OnCopyCompleted()
to clear the request. Normally, when a copy completion notification is received
it's from nsMailboxProtocol::OnStopRequest():

nsMsgCopyService::NotifyCompletion()
nsMsgLocalMailFolder::OnCopyCompleted()
nsMsgLocalMailFolder::EndCopy()
nsCopyMessageStreamListener::EndCopy()
nsCopyMessageStreamListener::OnStopRequest()
nsMsgProtocol::OnStopRequest()
nsMailboxProtocol::OnStopRequest()

Another problem I found is that if you try to copy a folder which already exists
in the destination folder we are not cleaning up the request either.

A patch is coming for the above problems. 

Comment 18

16 years ago
Created attachment 123559 [details] [diff] [review]
Proposed patch for empty folder copy problem, v1

Updated

16 years ago
Attachment #123559 - Flags: superreview?(sspitzer)

Updated

16 years ago
Flags: blocking1.4?
taking.  I'll test and review (and drive in) cavin's fix.
Assignee: cavin → sspitzer
Status: REOPENED → NEW

Comment 20

16 years ago
This hasn't been checked in, has it? I have some dups of this, I think.

Comment 21

16 years ago
*** Bug 180229 has been marked as a duplicate of this bug. ***

Comment 22

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

Updated

16 years ago
Flags: blocking1.4?
Attachment #123559 - Flags: superreview?(sspitzer)
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.