From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020721 BuildID: 2002072104 Using both Mozilla 1.0 and 1.1b I tried dragging an IMAP mailbox from a subdirectory to the root mail directory. Actual mailbox file is moved from ~/mail/<subdir>/<mailbox> to ~/<mailbox>. Should have moved to ~/mail/<mailbox>. .mailboxlist lists file as simply <mailbox> instread of ~/mail/<mailbox> as well. Reproducible: Always Steps to Reproduce: 1. set IMAP server directory to "~/mail/" 2. drag a subscribed mailbox into root directory 3. get Alert "The current command did not succeed. The mail server responded: SELECT failed: Can't open mailbox ~/mail/<folder>: no such mailbox. Actual Results: File is moved into ~/<mailbox> instead of ~/mail/<mailbox>. .mailboxlist has entry for <mailbox> not for ~/mail/<mailbox>. Used shell access to move back and correct entries in .mailboxlist Expected Results: should have moved file to ~/mail/<mailbox> and listed ~/mail/<mailbox> in .mailboxlist not sure if this affects any other imap servers than UW imap. Also the behaviour is fine when moving between sub-directories, only occurs when moving to root folder

I can reproduce this bug using 1.3a on Mac OS X 10.2.2 using Sun's IMAP server.

confirming. cc'ing Henry Jia.

I will look into this one soon.

In nsImapUrl::CreateServerDestinationFolderPathString, mozilla should always use AllocateServerPath() but should not set requested DestinationFolderPathString to "" directly when we move a folder to root. And in nsImapUrl::AddOnlineDirIfNeccessary, we should also give a check if the onlineMailboxName is "".

Pls review. Thanks. :)

instead of + if ( !PL_strcmp(onlineMailboxName,"") ) can't you just use *onlineMailboxName? it's less code, and faster.

Thanks David,I change it.Please r=? if it's all right.Thanks.

Comment on attachment 114063 [details] [diff] [review] patch for review r=bienvenu

It should be !*onlineMailboxName.

My falt. Sorry,David&Henry. This is the correct patch. It runs. Pls r=?&sr=?

Comment on attachment 114072 [details] [diff] [review] correct patch for review Carry David's r= here. It's just a very small change between the two patches. sr=Henry Require for approval 1.3. This is a small patch.

assuming I'm understanding the code (and your patch), instead of: if ( onlineDirWithDelimiter.Last() != delimiter ) onlineDirWithDelimiter += delimiter; if ( !*onlineMailboxName ) onlineDirWithDelimiter.SetLength(onlineDirWithDelimiter.Length()-1); I think this would be better: if ( onlineDirWithDelimiter.Last() != delimiter && *onlineMailboxName) onlineDirWithDelimiter += delimiter; Why add the delimiter and then remove it, right? re-assign to philip.zhao@sun.com

philip, let me know if you need help testing on another server (other than UW IMAP). we'd want to verify that this code does the right thing on non-UW IMAP servers.

Comment on attachment 114072 [details] [diff] [review] correct patch for review clearing approval request until I hear back from philip or henry about the change I suggested and if they need help testing on non-uw imap servers.

You're right,sspitzer. I changed it. Please see if it's well then. And I tested this patch on iPlanet mail server , it works well. Please give me your Cyrus server account and Courier server account, I will test them soon then. Thx. :)

But onlineDirWithDelimiter may already have the delimiter before the code goes to if ( onlineDirWithDelimiter.Last() != delimiter ), i.e. it's 'INBOX.'. We still need to remove the delimiter if onlineMailboxName is "". Right?

Yes,henry. I test again. If the IMAP directory is set "INBOX." like, the drag of a subfolder to root will still error with attachment 114292 [details] [diff] [review]. Then we will turn to attachment 114072 [details] [diff] [review]. And look for sspitzer's approval? Thanks sspitzer.

Comment on attachment 114072 [details] [diff] [review] correct patch for review => not obsolete.

Comment on attachment 114292 [details] [diff] [review] patch with sspitzer's suggestion => obsolete

I've got access to UW, Cyrus and Netscape Messaging Server 4.15 I'll try out your patch tomorrow, and let you know.

Comment on attachment 114072 [details] [diff] [review] correct patch for review sr=sspitzer stepping through the code, if (!m_destinationCanonicalFolderPathSubString), AllocateServerPath() will still do the right thing. I tested on UW, Cyrus, and Netscape Messaging. my only nit would be to fix nsImapUrl::CreateServerDestinationFolderPathString() and remove nsresult rv = NS_OK; and do: nsresult rv = AllocateServerPath(...);

Thanks sspitzer! While, still wait for an approval. :)

Comment on attachment 115322 [details] [diff] [review] patch, with minor nit cleanup r/sr=henry.jia,sspitzer

fixed on the trunk. not fixed on the 1.3 branch yet.

Comment on attachment 115322 [details] [diff] [review] patch, with minor nit cleanup a=asa (on behalf of drivers) for checkins to 1.3

before I land on 1.3, I'll wait for this to be verified. moving to 1.4 alpha, so there is no confusion as to when it was fixed. if I land in 1.3 final, I'll change the TM back to 1.3 final.

has approval for 1.3 final, but I'll wait until the bug is verified.

I back ported this fix to the 1.3 branch.