Closed Bug 448550 Opened 16 years ago Closed 16 years ago

can't move more than 2 folders to an imap folder

Categories

(MailNews Core :: Backend, defect)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED
Thunderbird 3.0b3

People

(Reporter: wsmwk, Assigned: Bienvenu)

References

Details

(Keywords: regression)

Attachments

(4 files, 2 obsolete files)

Attached file imap log (deleted) —
can't move more than 2 folders to another imap folder Steps 1. create 3 folders in local or imap account 2. attempt to copy/move 3 folders to an imap folder Results: only two folders can be copied. must restart thunderbird. fails: version 3.0a2pre (2008071603) and iirc builds from early this year works: TB 2.0.0.13 so trunk regression * OK [CAPABILITY IMAP4REV1 LITERAL+ SASL-IR LOGIN-REFERRALS STARTTLS LOGINDISABL ED] rain IMAP4rev1 2006h.380 at Mon, 28 Jul 2008 04:38:22 -0400 (EDT) . capability * CAPABILITY IMAP4REV1 LITERAL+ IDLE UIDPLUS NAMESPACE MAILBOX-REFERRALS BINARY UNSELECT SCAN SORT THREAD=REFERENCES THREAD=ORDEREDSUBJECT MULTIAPPEND SASL-IR L OGIN-REFERRALS STARTTLS LOGINDISABLED . OK CAPABILITY completed
WFM using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b1pre) Gecko/20080927030807 Shredder/3.0b1pre
2-3 local to imap fails, only moves first folder Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080927031346 Shredder/3.0b1pre
Do they need to be any certain size? Had a few crashes (bug 457515) testing it, but at least with small folders it worked.
in my tests folders are empty
See details in bug 450910. Fails also with empty folder.
Ok, I see it on linux now, testing with a few empty folders
Flags: blocking-thunderbird3+
OS: Windows XP → All
Hardware: PC → All
Product: Core → MailNews Core
still see this. doesn't deserve sev=major
Severity: major → minor
Are you copying the folders one at a time, or copying an empty parent folder with multiple children?
Assignee: bienvenu → nobody
Status: NEW → ASSIGNED
Component: Networking: IMAP → Backend
QA Contact: networking.imap → backend
Target Milestone: --- → Thunderbird 3.0b3
Attached patch proposed fix (obsolete) (deleted) — Splinter Review
this fixes the problem, but I'm having a heck of a time getting a test case to work...
Assignee: nobody → bienvenu
Attached patch proposed fix with test case (obsolete) (deleted) — Splinter Review
We were not always notifying the copy service when the folder copy finished, in the case of copying imap folders. That left us thinking there was still a request active on the target parent folder, which makes subsequent move/copies to that folder fail.
Attachment #368191 - Attachment is obsolete: true
Attachment #368273 - Flags: superreview?(neil)
Attachment #368273 - Flags: review?(bugzilla)
Comment on attachment 368273 [details] [diff] [review] proposed fix with test case The test case works, the code looks reasonable but it doesn't work on my imap server :-( I'll attach a log in a moment. >+ function copyFolder1() { >+ dump("gEmpty1 " + gEmptyLocal1.URI + "\n"); >+ let folders = new Array; nit: I think it should be either new Array(), or even better just []
Attachment #368273 - Flags: review?(bugzilla) → review-
Attached file Still broken imap log (deleted) —
I could be wrong, but that log looks like you moved/copied a folder hierarchy within the server. This patch is for when you copy across servers, or copy from a local folder account to an imap server. I don't believe the code I changed gets called at all when you move within a server...
Comment on attachment 368273 [details] [diff] [review] proposed fix with test case >+ nsImapMailFolder *newImapFolder = static_cast<nsImapMailFolder*>(m_newDestFolder.get()); Hmm, I think it would be clearer to make m_newDestFolder an nsRefPtr<nsImapMailFolder> and static cast on assignment. Or do what /local/ does and add onCopyCompleted to nsIImapMailFolder.
Standard8, pinging about #14
(In reply to comment #16) > Standard8, pinging about #14 You're right I was doing it in-server. I've just tried copying across servers - to another imap server, nothing happened; to local folders it seemed to work but I got the following error raised twice: ************************************************************ * Call to xpconnect wrapped JSObject produced this error: * [Exception... "'[JavaScript Error: "aFolder is not defined" {file: "file:///Users/moztest/comm/main/tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/modules/activity/moveCopy.js" line: 211}]' when calling method: [nsIMsgFolderListener::folderMoveCopyCompleted]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "<unknown>" data: yes] ************************************************************
(In reply to comment #17) > You're right I was doing it in-server. I've just tried copying across servers - > to another imap server, nothing happened; I'll try that. > to local folders it seemed to work this bug is about copying from local folders to imap server. > but I got the following error raised twice: > > ************************************************************ > * Call to xpconnect wrapped JSObject produced this error: * > [Exception... "'[JavaScript Error: "aFolder is not defined" {file: > "file:///Users/moztest/comm/main/tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/modules/activity/moveCopy.js" > line: 211}]' when calling method: > [nsIMsgFolderListener::folderMoveCopyCompleted]" nsresult: "0x80570021 > (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "<unknown>" data: > yes] > ************************************************************ that happens without this patch as well - I'll look at it, but I suspect it's a problem in the activity manager.
this fixes the error you saw moving folders, and corrects the status message to show source and dest accounts, when different, like we do for message move/copy.
Attachment #370300 - Flags: review?(bugzilla)
Standard8, sadly, copying imap folders between imap servers is working for me - I tried both a single folder and a two folder hierarchy - can you see if there's anything on the console, or in an imap log when it fails for you.
Attached patch fix addressing Neil's comment (deleted) — Splinter Review
This patch is the same as before, except I did Neil's suggestion, which I agree does make for nicer code.
Attachment #368273 - Attachment is obsolete: true
Attachment #370302 - Flags: superreview?(neil)
Attachment #370302 - Flags: review?(bugzilla)
Attachment #368273 - Flags: superreview?(neil)
Attachment #370302 - Flags: superreview?(neil) → superreview+
Whiteboard: [needs review/test Standard8]
Attachment #370300 - Flags: review?(bugzilla) → review+
Attachment #370302 - Flags: review?(bugzilla) → review+
Comment on attachment 370302 [details] [diff] [review] fix addressing Neil's comment ok, that's better now I know what I'm testing, though we possibly need to look at the other copies as well. Note that moveCopy.js is already in your other patch I just reviewed.
Whiteboard: [needs review/test Standard8]
both patches checked in.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
v.fixed move 2 empty local folders to imap in one action. however, moving imap folder with mail to different imap account crashed - filed Bug 487288 note: /undo/ after moving folders leaves the moved folders in imap target. But I think it did that before this fix. (related to Bug 59688? Undo not working completely for imap move/copy message)
Status: RESOLVED → VERIFIED
I opened what sounds like a dupe of this bug (see bug 496905) because I can still reproduce this problem w/ Shredder built from a fresh checkout yesterday, 2009-06-08.
When we get to nsImapMailFolder::OnStopRunningUrl when doing an imap folder move, m_copyState is null (which it might just be for an intra-server imap folder move), and we don't send any notifications for the imapAction, which I believe is nsImapMoveFolderHierarchy - maybe it's just a matter of sending the copy completed notification for this type of url (though I think we might need a copy state to pass the right info to the copy service).
My suggestion would be to make imap intra-server folder moves call InitCopyState (see nsImapMailFolder::CopyFolder, where it calls imapService->MoveFolder), make InitCopyState handle a null messages parameter, and add a case in nsImapMailFolder::OnStopRunningUrl for the imap action nsImapMoveFolderHierarchy, which would call OnCopyCompleted.
InitCopyState uses nsImapMailCopyState - shouldn't we instead use nsImapFolderCopyState ... ? What's the "right way" of getting the srcFolder to OnStopRunningUrl, so that I could invoke nsMsgFolderNotificationService::NotifyFolderMoveCopyCompleted? I'm going to try using nsImapFolderCopyState and see how far it gets me ... :-)
(In reply to comment #28) > InitCopyState uses nsImapMailCopyState - shouldn't we instead use > nsImapFolderCopyState ... ? > > What's the "right way" of getting the srcFolder to OnStopRunningUrl, so that I > could invoke nsMsgFolderNotificationService::NotifyFolderMoveCopyCompleted? The copy state has the src folder. The notification service is not the same as the copy service - I suspect the notification service is notified in a different place. You do want to call nsImapMailFolders' OnCopyCompleted(m_copyState->m_srcSupport, aExitCode) method... > > I'm going to try using nsImapFolderCopyState and see how far it gets me ... You can try that - it's used for cross server copies right now, because cross-server copies also have to copy the messages.
OK, I gave up trying to use nsImapFolderCopyState - I think the names of the classes involved are a bit misleading - and am using nsImapMailCopyState now. Will let you know how it goes.
This bug still exists in : Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20110812 Thunderbird/6.0 Build ID:20110812055110 If I copy one folder from the hierarchy to the top (INBOX), then I can't copy another folder. There isn't any error message. After restarting Thunderbird, i can move another folder...then restart.. move.. It isn't too easy, when you have 5-10 folder to move.
pnagy, with TB 6, you should be able to turn on copy service logging, along with IMAP logging, and get us a bit of info about what's failing. https://wiki.mozilla.org/MailNews:Logging The interesting log modules would be IMAP,MsgCopyService,timestamp
First I tested some move operations. The folder moves are working if i only move folders below INBOX. When I moving folder from a subfolder of INBOX to INBOX(ex: INBOX.some.test to INBOX.test), i can do it once at the second move i got: 2011-08-17 21:30:19.093272 UTC - -1260102960[b4c17060]: request a13cb1f0 QueueRequest - src imap://info%40xyz.tld@imap.xyz.tld/INBOX/Hosting/SOMEDIR dest imap://info%40xyz.tld@imap.xyz.tld numItems 0 type=2
Can you e-mail me the whole log? The fact that the second request was queued indicates that something happened with the first request, and I'm hoping that's in the log.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: