In nsImapProtocol::DiscoverMailboxSpec in mozilla/mailnews/imap/src/nsImapProtocol.cpp, mozilla only handles the specific namespace delimiter '/'. This is not correct. See the code below. const char *nsPrefix = ns ? ns->GetPrefix() : 0; nsCString canonicalSubDir; if (nsPrefix) { PRUnichar slash = '/'; canonicalSubDir = nsPrefix; if (canonicalSubDir.Length() && canonicalSubDir.Last() == slash) canonicalSubDir.SetLength((PRUint32) canonicalSubDir.Length()-1); }

Make canonicalSubDir in canonical format.

setting the hierarchy delimiter to 0 doesn't seem like a good idea, since it will never work: + char delimiter = ns ? ns->GetDelimiter() : 0; what's the bug that occurs because of this?

1. setting the hierarchy delimiter to 0 doesn't seem like a good idea, since it will never work: + char delimiter = ns ? ns->GetDelimiter() : 0; Here we are handling namespace prefix. We want the canonical format of the prefix with the last delimiter omitted. I set delimiter to 0 (where there is no namespace) just for the continuous checking to see if we should go on to change the prefix's format. Indeed, after I re-checked the patch, there is no need to check if ns is nsnull, because if nsPrefix is not nsnull, ns must exist. I'll give a new patch soon. 2. what's the bug that occurs because of this? We uses namespace's prefix to check if we should set m_onlineBaseFolderExists to true, if we don't change it to canonical format, "m_onlineBaseFolderExists = PR_TRUE" will never be executed when the delimiter is not '/'. Though I really don't see the error caused by this, from the theory, it's not correct.

Add navin & cavin to CC list. Pls r= the new patch. Thx.

Comment on attachment 94281 [details] [diff] [review] new patch for review r=naving

We didn't use m_onlineBaseFolderExists at all. Onle set 0 or 1 to it.

What do you mean, Philip?

m_onlienBaseFolderExists only can be seen in these lines. /mailnews/imap/src/nsImapProtocol.cpp, line 405 -- m_onlineBaseFolderExists = PR_FALSE; /mailnews/imap/src/nsImapProtocol.cpp, line 4156 -- m_onlineBaseFolderExists = PR_TRUE; /mailnews/imap/src/nsImapProtocol.cpp, line 4271 -- m_onlineBaseFolderExists = PR_TRUE; /mailnews/imap/src/nsImapProtocol.h, line 652 -- PRBool m_onlineBaseFolderExists; So I think it won't lead to errors if we didn't set it. Onle(only) so:-)

Philip, you're right. We don't use m_onlineBaseFolderExists now. Hi, all, who know what is m_onlineBaseFolderExists ever used for? Or What will m_onlineBaseFolderExists be potentially used for? If it doesn't have any usage now, let's get rid of it. If it is potentially for some usage, then the patch (attachment 94281 [details] [diff] [review]) is needed. It correctly handle the conanical conversion. Here is the data output from debug without the patch. (gdb) p adoptedBoxSpec->allocatedPathName $4 = 0x8a28468 "INBOX/Trash" (gdb) p canonicalSubDir $5 = {<nsAFlatCString> = {<nsASingleFragmentCString> = {<nsACString> = { _vptr.nsACString = 0x402cd9c8}, <No data fields>}, <No data fields>}, <nsStr> = {{ mStr = 0x8d445c0 "INBOX.", mUStr = 0x8d445c0}, mLength = 6, mCapacityAndFlags = 1073741830}, <No data fields>} It is obvious that the conversion hasn't been done.

we should just remove that variable. It wasn't even used in 4.x, so I think it's safe to remove it.

Change the summary according to the comments above.

Here is the patch. naving & David, please r= & sr=. Thx.

Comment on attachment 106046 [details] [diff] [review] patch for review Please r= & sr=. Thx.

Comment on attachment 106046 [details] [diff] [review] patch for review sr=bienvenu

Comment on attachment 106046 [details] [diff] [review] patch for review Ok, looks good to me.:-)

In trunk.

verified fixed using lxr (code removal verification)