55774 - IMAP subscribe doesn't handle servers with hierarchy seperator "."

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P3)

Description

[leg+mozilla]

an example of the IMAP protocol generated when navigating through a Cyrus IMAP
server trying to subscribe to "comp.mail.imap":

<0<14 list "" "%"
<0<15 list "" "%.%"
<6<16 list "" "comp/%"
<0<17 list "" "comp/%/%"

it correctly populates the initial subscription window with the top level
mailboxes and displays the second level mailboxes when the arrow is clicked.
arrows never appear on second level mailboxes due to the incorrect LIST commands.

cyrus.andrew.cmu.edu accepts ANONYMOUS IMAP connections (or log in as user
anonymous) to see this behavior.  (try subscribing to INBOX.a.b.c, for instance.)

Comment 1

[(not reading, please use [e-mail] instead)]

yes, you are correct.  currently, I am assuming that that hierarchy seperators
are "/" for mail and "." for news.

what I need to do is get the proper hierarchy seperator from the server.
this should not be hard, since the nsIMsgImapMailFolder.idl has the necessary
attribute, wchar hierarchyDelimiter.

Comment 2

[(not reading, please use [e-mail] instead)]

wait, it looks like both delimiters are used here.

I need to look into this more...

Comment 3

[(not reading, please use [e-mail] instead)]

david recently fixed the delimiter code.  david, is bug still valid?

in the past, we were getting "^" as the delimiter, so we'd return "/" (the
default delimiter) to the subscribe code.

Comment 4

[David :Bienvenu]

I suspect this is still valid - I'll try it.

Comment 5

[Anthony Baxter]

I'm still seeing this problem with this build (2001021508) on
linux, when talking to a Cyrus 2.0.x IMAP server. I can not 
subscribe to existing subfolders under the inbox. 

Comment 6

[Steve Meredith (gone)]

The particular server mentioned is using "." as the delimiter. Checking the
protocol log, the mail client is trying to subscribe to INBOX/subfolder when it
should be subscribing to INBOX.subfolder, so the subscribe command fails. 

Examining the code, I find that nsImapIncomingServer::SubscribeToFolder() is
passing the rootfolder to nsImapService::SubscribeFolder(), which askes the
rootfolder for the delimiter. The rootfolder has "^" as the delimiter, which
gets converted to "/". 

Any hints from the people who worked on the delimiter code on how to proceed to
fix this? One possible solution to ask the server for the delimiter after
connecting and save it in the rootfolder (you can ask with the IMAP LIST command
with null parameters.)

I can take ownership of this, but would appreciate any guidance.

Comment 7

[John G. Myers]

The code should be getting the hierarchy separator from the LIST or LSUB 
response containing the first level folder ("INBOX" or "comp").  It would also 
work to get this from the LIST/LSUB response for the parent folder.

Per protocol, every top-level folder can have a different hierarchy separator, 
but every sub-folder must have the same hierarchy separator as its parent.

Comment 8

[Steve Meredith (gone)]

Then it sounds like the right thing to do is to get the delimiter from the
top-level folder object for the folder being subscribed to, not from the root
folder as the code is currently doing.

Comment 9

[Steve Meredith (gone)]

I've spent some more time investigating this. The delimiter character is lost
after the folder name has been read from the mail server. For unsubscribed
folders, the folder name is stored internally with '/' as the delimiter, and
that's how the subscribe dialog shows it no matter what it really is--is that
behavior correct? So INBOX.subfolder is displayed as INBOX/subfolder, and that's
the name that is used to try to subscribe. The '.' is not saved in the client as
far as I can tell. Since each top-level mailbox can have its own delimiter to
use for all its subfolders, it needs to be saved or requested again from the
server given the top-level mailbox name. 

Comment 10

[Steve Meredith (gone)]

This is a regression from 4.x, which could handle a . as the hierarchy seperator
no problem. 

Also, 4.x displays folders in the subscribe dialog with the correct seperator
(e.g. "mailbox.folder1"). 6.x always displays a / in that dialog, even if the
seperator is something else (e.g. "mailbox/folder1"). I think the 4.x way is
correct. Yes? If so, would that be a seperate defect?

Comment 11

[David :Bienvenu]

No, the subscribe UI should be using the real hierarchy delimiter, and I believe
this bug covers that.

Comment 12

[Steve Meredith (gone)]

The javascript code for the subscribe dialog has one global var for the
delimiter, so it's not going to work right if two top-level mailboxes have
different delimiters. 

Comment 13

[(not reading, please use [e-mail] instead)]

you are right, type down will not work when there are two different delimiters.

Comment 14

[(not reading, please use [e-mail] instead)]

steve, see #76552

the subscribe code is eventually going to be switched over to the outliner.
primarily for performance and so we can add "search" and "new" views.

fixing subscribe (and type down) to work with multiple delimiters before we do 
that will be work we'll have to redo anyways.

do you have any interest in helping with the subscribe outliner effort? 

Comment 15

[scottputterman]

adding nsenterprise+

Comment 16

[(not reading, please use [e-mail] instead)]

slide to 0.9.5

Comment 17

[(not reading, please use [e-mail] instead)]

moving back, I hope to fix both nsenterprise+ subscribe bugs for 0.9.4

Comment 18

[Cavin Song]

Taking from sspitzer.

Comment 19

[Cavin Song]

Attachment: Fixed the problem.

Comment 20

[Cavin Song]

Basically, in nsImapProtocol::NthLevelChildList() we need to restore the folder 
pathname back to original one (ie, the one sent by the server for the list/lsub 
response). For example, 'INBOX/subfld' should be changed/converted to 
'INBOX.subfld' before sending it to the Cyrus server, otherwise the list cmd 
will find nothing.

bienvenu and sspitzer can you r and sr it? Thanks.

Comment 21

[David :Bienvenu]

I think this is the wrong place to fix this - this code expects the delimiter to
have been fixed already. Did you try stepping through void
nsImapProtocol::DiscoverAllAndSubscribedBoxes()
 and see what ns->GetDelimiter() was returning in this situation? (I think
that's where the relevant code is) My guess is that it's not returning the right
value, and that should be fixed so that anyone who uses that method will get the
right delimiter in the future.

Comment 22

[Cavin Song]

Ok, David, I'll look into it more.

Comment 23

[Cavin Song]

A new patch is coming soon and here is the description of the fix:

In nsImapService::GetListOfFoldersWithPath(), we should not pass 'rootMsgFolder' 
to DiscoverChildren() because rootMsgFolder's delimiter is hard coded to '^'.  
In this case, we'll never get the right online server delimiter. So the fix is 
to try to find the right nsIFolder pointer (under the root folder) and pass that 
to DiscoverChildren().

Comment 24

[Cavin Song]

Attachment: A better fix.

Comment 25

[David :Bienvenu]

don't need to do = nsnull here.  nsCOMPtr<nsIMsgFolder> msgFolder=nsnull;


otherwise, good job, r=bienvenu.

Comment 26

[Cavin Song]

Right, cut & paste error.

Comment 27

[(not reading, please use [e-mail] instead)]

looks good.

in addition to bienvenu's nit, here's one more:

the aFoo notation is for method arguments.  since aFolder is a local variable,
name it something else.

I'd suggest something like this:

+  if (rootMsgFolder && folderPath && (*folderPath))
+  {
+    nsCOMPtr<nsIFolder> subfolder;
+    rv = rootMsgFolder->FindSubFolder(folderPath, getter_AddRefs(subfolder));
+    if (NS_SUCCEEDED(rv))
+      msgFolder = do_QueryInterface(subfolder);
+  }

other than that, sr=sspitzer

excellent work cavin!

Comment 28

[Cavin Song]

Make sense, again, cut & paste issue :-(.

Comment 29

[Cavin Song]

Fix checked in.

Comment 30

[Karen Huang]

273[3fde950]: buggy.mcom.com:S-INBOX:SendData: 51 list "" "INBOX.%"
..................................

273[3fde950]: buggy.mcom.com:S-INBOX:SendData: 52 list "" "INBOX.%.%"
...............

273[3fde950]: buggy.mcom.com:S-INBOX:SendData: 53 list "" "INBOX.ka0828_1.%"
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: * LIST () "." 
INBOX.ka0828_1.ka0828_2
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: 53 OK Completed
273[3fde950]: buggy.mcom.com:S-INBOX:SendData: 54 list "" "INBOX.ka0828_1.%.%"
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: * LIST () "." 
INBOX.ka0828_1.ka0828_2.ka0828_3
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: 54 OK Completed
273[3fde950]: buggy.mcom.com:S-INBOX:SendData: 55 list "" 
"INBOX.ka0828_1.ka0828_2.%"
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: * LIST () "." 
INBOX.ka0828_1.ka0828_2.ka0828_3
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: 55 OK Completed
273[3fde950]: buggy.mcom.com:S-INBOX:SendData: 56 list "" 
"INBOX.ka0828_1.ka0828_2.%.%"
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: * LIST () "." 
INBOX.ka0828_1.ka0828_2.ka0828_3.ka0828_4
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: 56 OK Completed
273[3fde950]: buggy.mcom.com:S-INBOX:SendData: 57 list "" 
"INBOX.ka0828_1.ka0828_2.ka0828_3.%"
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: * LIST () "." 
INBOX.ka0828_1.ka0828_2.ka0828_3.ka0828_4
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: 57 OK Completed
273[3fde950]: buggy.mcom.com:S-INBOX:SendData: 58 list "" 
"INBOX.ka0828_1.ka0828_2.ka0828_3.%.%"
273[3fde950]: buggy.mcom.com:S-INBOX:CreateNewLineFromSocket: 58 OK Completed

Comment 31

[Karen Huang]

Now. It is handling the subscribed subfolders on Cyrus now.....
I will check on Lotus IMAP & Courier IMAP as well for this bug....

Comment 32

[Karen Huang]

OK.
I saw some problems for specific on Courier IMAP.
After confirm with Cavin, I am marking as verified for this bug and I willl log 
those bugs for tracking Courier IMAP subscribe/unsubscribe problems.......
Marking as verified.

Comment 33

[Karen Huang]

Verified Builds were: WinNT, Linux & Mac 08-30-12-trunk builds.

Comment 34

[Karen Huang]

*** Bug 80871 has been marked as a duplicate of this bug. ***

Comment 35

[Vadim Berezniker]

*** Bug 108910 has been marked as a duplicate of this bug. ***