Closed Bug 55774 Opened 24 years ago Closed 23 years ago

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

Categories

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

Product:
SeaMonkey
Component:
MailNews: Message Display
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.4

People

(Reporter: leg+mozilla, Assigned: cavin)

References

Details

Attachments

(2 files)

Fixed the problem.
1.53 KB, patch
Details | Diff | Splinter Review
A better fix.
1.12 KB, patch
Details | Diff | Splinter Review 
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.)
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.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
QA Contact: esther → huang
wait, it looks like both delimiters are used here.

I need to look into this more...
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.
I suspect this is still valid - I'll try it.
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. 

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.
OS: Linux → All
Hardware: PC → All
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.

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.
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.
Keywords: nsenterprise
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?
Keywords: 4xp
No, the subscribe UI should be using the real hierarchy delimiter, and I believe
this bug covers that.
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. 
you are right, type down will not work when there are two different delimiters.

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? 
adding nsenterprise+
Keywords: nsenterprise → nsenterprise+
Target Milestone: --- → mozilla0.9.4
slide to 0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
moving back, I hope to fix both nsenterprise+ subscribe bugs for 0.9.4
Target Milestone: mozilla0.9.5 → mozilla0.9.4
Taking from sspitzer.
Assignee: sspitzer → cavin
Status: ASSIGNED → NEW

    
    

    
  
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.

    
    

    
  
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.

    
    

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

    
    

    
  
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().

    
    

    
  

      
      
      
      

        Attached patch
          
          
        A better fix.
        — 
          — Splinter Review
      

    


  

    
    

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


otherwise, good job, r=bienvenu.

    
    

    
  
Right, cut & paste error.

    
    

    
  
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!

    
    

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

    
    

    
  
Fix checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

    
  
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


    
    

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

    
    

    
  
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.
Status: RESOLVED → VERIFIED

    
    

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

    
    

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

    
    

    
  
*** Bug 108910 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey

    
  
Component: MailNews: Subscribe → MailNews: Message Display
QA Contact: huang → search

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: