Open Bug 166603 Opened 22 years ago Updated 2 years ago

IMAP: Drafts/Sent/etc not recognized as special folders, when using namespaces.

Categories

(MailNews Core :: Networking: IMAP, defect)

Product:
MailNews Core
Component:
Networking: IMAP
Platform:
x86
All
Type:
defect

Tracking

(Not tracked)

People

(Reporter: mozilla-bugs, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: helpwanted, Whiteboard: [patchlove] workaround: comment 22)

Attachments

(8 obsolete files)

I have in my prefs.js: user_pref("mail.server.server3.namespace.other_users", ""); user_pref("mail.server.server3.namespace.personal", "\"INBOX.Sys.\",\"INBOX.Work\",\"INBOX.Folders.\""); user_pref("mail.server.server3.namespace.public", ""); user_pref("mail.server.server3.override_namespaces", false); and user_pref("mail.identity.id2.draft_folder", "imap://nogin@cuttlefish.cs.caltech.edu/Drafts"); user_pref("mail.identity.id2.drafts_folder_picker_mode", "0"); With this setup Mozilla (1.1 on Red Hat Linux 7.3) correctly saves messages to INBOX.Sys.Drafts when I do "save as draft". However when I go to the Drafts folder, the message is displayed same as if it was an ordinary folder and there is no way to continue editing the draft (other then "Edit as new")!
Which build are you using? Can you provide the build info here? What kind of the IMAP Server are you connecting with?
I am using 1.1 release (using RPMs from ftp.mozilla.org) on Red Hat Linux 7.3. The server is Courier IMAP.
Blocks: 160644
Same with Sent - the sent messages will be filed there normally, but when viewing the folder, it would show the sender instead of showing the recipient (as it supposed to do in Sent). P.S. Not sure if this is an IMAP issue, an FE issue, or both...
Summary: IMAP: Drafts is not recognized as a special folder, when using namespaces. → IMAP: Drafts/Sent/etc not recognized as special folders, when using namespaces.
I reproduced this bug in my fastmail account (Cyrus server) too. And also I can't save massage in mydraft when IMAP server directory is set INBOX.
Yeah, still there in BuildID 2002103002 (trunk).
This may have something to do with the server.Need server prepare a pair of Drafts and Sent and Templates folders for every personal namespace? If server don't support default Drafts and Sent ... folders initially, mozilla can be changed into create those folders when it first discoverfolders. Just like 1.CreateDefaultMailboxes,2.SetFlagsOnDefaultMailboxes.( And the key of the solution will be flags' setting and using.) But it is expensive.
Re: comment #6 > This may have something to do with the server Regardless of server, Mozilla is clearly confused - it knows to _save_ messages into those folders, but for some reason faile to realize they are special when _displaying_ them.
This is the client issue.
Does someone have tracked if the flags is set correctly for Drafts folder?Do Aleksey Nogin mean even if the flags is set correctly, we still can't see the 'edit draft...' button or other errors?Thnks:)
How do I check the flags? My guess is they are *not* set correctly - at least in the folders pane the "normal folder" icons are used instead of the special ones.
If the MSG_FOLDER_FLAG_DRAFTS or such other flags(which means that this is a special folder) is set correctly, I find the "Edit draft..." button and other special functions works well. I'm finding if there is something wrong with the setting of flags or recognize these special folder. Thks Aleksey.
The reason should be the flags are *NOT* set correctly because of the namespace.
If drafts (or other special ones) is parrelled with 'INBOX' or subfolder of 'INBOX',flags is set. If they are in more deeper levels, for example,'INBOX.Sys.Drafts',flags is not set. A question is ,need mozilla keep a pair of Drafts and Templates and Sent folders for each personal namespace?
> A question is ,need mozilla keep a > pair of Drafts and Templates and Sent folders for each personal namespace? Why "each"??? Right now Mozilla (IMHO correctly) keeps Drafts, Trash and Sent in the *first* personal namespace, but does not set any flags. In any case, if all available namespaces are subspaces of INBOX. (as in my case), then Mozilla does not have amny other choice but to be able to use one of those (and set the correct flags), which is what this bug is about.
Attached patch add a patch. (obsolete) — — Splinter Review
Comment on attachment 105540 [details] [diff] [review] add a patch. >+ if(name->Equals(NS_LITERAL_STRING(onlineNameWithoutNamespace))) This fails to compile: nsImapMailFolder.cpp:390: `LonlineNameWithoutNamespace' undeclared
Attachment #105540 - Flags: review-
Comment on attachment 105540 [details] [diff] [review] add a patch. Hi, philip, Thx for the patch anyway. But there are something needed to be payed attention to. 1. try to build and test the code before you give out the patch 2. pay attention to the code style, refer to http://www.mozilla.org/hacking/mozilla-style-guide.html, also please refer to the existing code 3. As for the bug, Drafts/Sent/etc should not be recongnized by comparing the literals, they can be any names. Also, nsMsgFolder::SetPrefFlag (nsMsgAccountManager::SetSpecialFoldersForIdentities) should be better place for the patch. Henry
Thanks Aleksey and henry. I will be careful later.:-) I also find that nsMsgFolder::SetPrefFlag where mozilla set flags when it initially find out Draft and other special ones will be the best place to add patch. But nsIMAPNamespace *nsImapMailFolder::GetNamespaceForFolder() is a protect method of nsImapMailFolder and we can't use it in mailnews/base/util/nsMsgFolder.cpp. How can we get the folder's namespace in nsMsgFolder.cpp then? Because nsMsgFolder mainly deal with those actions which needn't know what kind of a folder it is, if we add a protect method GetNamespaceForFolder here(using it only to get namespace of this folder when server supports namespace), it will seems a not small change to mozilla's whole code structure. Should we do that? Thank you.
try nsIImapIncomingServer::getUriWithNamespacePrefixIfNecessary
Attached patch a patch for test (obsolete) — — Splinter Review
1.Reason of this bug is SENTMAIL,DRAFTS,TEMPLATES flags are not set correctly because of namespace. 2.In nsMsgFolder::SetPrefFlag(),mozilla find out special folders using 2.1 accountMgr->GetFirstIdentityForServer(server, getter_AddRefs(identity)) 2.2 identity->GetFccFolder(getter_Copies(folderUri)); 2.3 rdf->GetResource(folderUri, getter_AddRefs(res)) If these three steps all succeed, flag will be set using folder->SetFlag. 3.I guess 2.1 will not fail, for no identity no all. And 2.2 will not fail either because the fail of GetFccFolder will only demonstrate there are no special folders initially supported by server(Is it correct?). So only 2.3 may fail. 4.Thus I wrote this patch,which means if folderUri (a uri without namespace prefix) is not enough to let mozilla find the folder's resource, we will give it a complete uri folderUriWithNamespace, it may find the folder and set flags then. 5.Pls test it when you have time,I have no this bug's environment.:-)
Comment on attachment 106183 [details] [diff] [review] a patch for test This patch does not fix the problem for me. However: 1) I agree with the reasoning behind the patch: It is likely that the "rdf->GetResource" is the one that fails. But note - this is *not* what your patch is currently doing - you seem to have attached "else" to the *wrong branch*: >@@ -1540,6 +1543,22 @@ > folder = do_QueryInterface(res, &rv); > if (NS_SUCCEEDED(rv)) > rv = folder->SetFlag(MSG_FOLDER_FLAG_SENTMAIL); >+ else >+ { e.g. your "else" would only become active if do_QueryInterface(res) fails, *not* if "rdf->GetResource" fails!
Attachment #106183 - Flags: review-
BTW, your reasoning have suggested a workaround! If I choose the folder to use by selecting Place a copy in -> "Sent" Folder on: -> Work then I see this bug. But if instead I choose Place a copy in -> Other: -> Sent on Work (e.g. I manually point to the folder I want), then it works fine! P.S. Would it be better to try to fix the GetFccFolder function itself, rather than trying to fix the place(s) that use it?
>2.In nsMsgFolder::SetPrefFlag(),mozilla find out special folders using > 2.1 accountMgr->GetFirstIdentityForServer(server, getter_AddRefs(identity)) > 2.2 identity->GetFccFolder(getter_Copies(folderUri)); > 2.3 rdf->GetResource(folderUri, getter_AddRefs(res)) > If these three steps all succeed, flag will be set using folder->SetFlag. > >3.I guess 2.1 will not fail, for no identity no all. And 2.2 will not fail >either because the fail of GetFccFolder will only demonstrate there are no >special folders initially supported by server(Is it correct?). So only 2.3 may >fail. It's not correct that 2.3 will fail. 2.3 always succeed. You need just add the namespace if needed. Fixing GetFccFolder is a good suggestion. Pay attention to the fcc_folder, etc may locate on another server with different namespace. You need to setup an enviroment for the work. You may refer to our internal web site for some test enviroment.
Attached patch a new patch (obsolete) — — Splinter Review
Pls try this. Added namespace before folder name of Draft etc.
Comment on attachment 106317 [details] [diff] [review] a new patch See comment 23 Have you tried to test the patch?
Attachment #106317 - Flags: review-
Yes.Henry.I have tested it. The differentce of the new patch with the second patch is simple, only let the code of else to run.(I only add a '!' before NS_SUCCEEDED(rv) of line a111) a111>if (!NS_SUCCEEDED(rv)) a222> rv = folder->SetFlag(MSG_FOLDER_FLAG_SENTMAIL); a333> else a444> {//code here will run In my test account of courier server matchpoint,username mailer1 ,password mailer1, whatever override_namespace is true or false, or if we have configured personal namespace by hand, the code all work well. I can see these special folders with special icons now.And when I save a massage as draft, the 'Edit Draft...' button shows .It works well. In the original code,the code of line a222 do set flag.But flag is not set on correct folder. Originally,set flag on "imap://mailer1@matchpoint/Sent"(no this folder acturlly) Now,set flag on "imap://mailer1@matchpoint/INBOX/Sent".(though we may not see Sent folder at the beginning,but once we ever use it,we will see it from that time on.The same is with Drafts and Templates.) So I think it is correct. Of course,the code should have a clean. But that will be easy. So we may test it first.
The logic of your code is not right. You should check if the uri includes namespace first before trying to get the related folder object.
Acturlly, in nsIImapIncomingServer::getUriWithNamespacePrefixIfNecessary, mozilla has checked if the uri(we give to this method as a parameter) have namespace. That means, if uri is "imap://mailer1@matchpoint/INBOX/Sent", doesn't change, if uri is "imap://mailer1@matchpoint/Sent", insert 'INBOX/' into the uri.
While,I think whatever the uri is "imap://mailer1@matchpoint/Sent" or "imap://mailer1@matchpoint/INBOX/Sent" ,we should both setflag to the folder object. Because in some other conditions the folder is just "imap://mailer1@matchpoint/Sent". The code should be changed to this. This is what I wanted in patch 2.
> if uri is "imap://mailer1@matchpoint/Sent", > insert 'INBOX/' into the uri. Ah, I think you've nailed it - that sound exactly where the bug is coming from! It should insert the first namespace here, not the "INBOX/".
You are right.I think it did just as you say.I mean that too.
Comment on attachment 106317 [details] [diff] [review] a new patch I am starting to think - is the "old" branch necessary at all? Would not it work better if the code just *always* use GetUriWithNamespacePrefixIfNecessary and never even tries running GetResource(folderUri)? This way the brunch before the "else" would be totally unnecessary. Sure, the code will be slightly less efficient in the simple (no namespaces) case, but I am sure the difference will be negligible and it's more important that the code will be simpler.
Comment on attachment 106317 [details] [diff] [review] a new patch Sorry, ignore what I just said - of course, we can not always use GetUriWithNamespacePrefixIfNecessary since it only exists for IMAP.
Attached patch better than the 3rd one (obsolete) — — Splinter Review
This patch add a logic to check if the server is a imap server. Henry,I'm not sure what you mean to "check if the uri includes namespace first before trying to get the related folder object". Amazing...:-) I'd like to talk with you about it.
Comment on attachment 106491 [details] [diff] [review] better than the 3rd one Hi, Philip, Thx for the hard work. This patch looks good. Just a few small tips. 1. you may just use nsCOMPtr<nsIImapIncomingServer> imapServer = do_QueryInterface(server); instead of rv = server->GetType(&serverType); to check if the mail server is imap. This can make the code a little more efficient. And, try not to use the hardcode, such as "imap". 2. pay attention to the code style, such as the indent character number. 3. try to make the line is less or equal to 80 characters. 4. get rid of //fccfolder treatment end since you just have only one such comment. 5. get rid of // && NS_SUCCEEDED(rdf->GetResource(folderUri, getter_AddRefs(res)))) As for "check if the uri includes namespace first before trying to get the related folder object", I mean you should check if the server is imap server and check if the uri includes namespace before you "folder = do_QueryInterface(res, &rv);". You have done so in your new patch. :-) Keep on going. Henry
Attachment #106491 - Flags: review-
Attached patch a new patch (obsolete) — — Splinter Review
I changed the lines as your comment,henry. 1.Using nsCOMPtr<nsIImapIncomingServer> imapServer = do_QueryInterface(server) and if(imapServer) to check if the server is a imap server. 2.Bring some lines out for efficiency. 3.Other is corrected as your comment.
Add check of fcc_folder_picker_mode (and also with Drafts,Templates folder's picker_mode) in this patch.
Philip, when you update patches, can you mark the old ones as obsolete (provided, of course, they are indeed obsolete)? Thanks!
I get it. Thx! Nogin!
Comment on attachment 106627 [details] [diff] [review] patch with folder_picker_mode checks 1. use 'nsXPIDLCString folderPickerMode;' instead of 'char * folderPickerMode;', thus you don't need to take care of the memory release. 2. still the code style :-)
Attachment #106627 - Flags: review-
Attached patch updated new patch (obsolete) — — Splinter Review
Having changed it as your comment, henry. 3x!:-)
Attachment #105540 - Attachment is obsolete: true
Attachment #106183 - Attachment is obsolete: true
Attachment #106317 - Attachment is obsolete: true
Attachment #106491 - Attachment is obsolete: true
Attachment #106555 - Attachment is obsolete: true
Attachment #106627 - Attachment is obsolete: true
Comment on attachment 106637 [details] [diff] [review] updated new patch 1. use folderPickerMode.Equals("0") instead of PL_strcmp(folderPickerMode.get(), "0") 2. still code style. pay attention to the intend char number. Thx.
Attachment #106637 - Flags: review-
Attached patch updated new patch (obsolete) — — Splinter Review
henry,pls see this one. 1.using folderPickerMode.Equals("0") 2.when the parameter is more than 2, using a new line for each parameter in a method. :-)
Attachment #106637 - Attachment is obsolete: true
Comment on attachment 106747 [details] [diff] [review] updated new patch It's OK. Thx, Philip. r=henry
Attachment #106747 - Flags: superreview?(bienvenu)
Attachment #106747 - Flags: review+
Comment on attachment 106747 [details] [diff] [review] updated new patch This patch works for me, thanks!
Comment on attachment 106747 [details] [diff] [review] updated new patch first of all, there's no way the imap code should be in nsMsgFolder.cpp. For imap, this could be in nsImapIncomingServer::PossibleImapMailbox. Also, this patch duplicates a lot of code that could be in a helper routine.
Attachment #106747 - Flags: superreview?(bienvenu) → superreview-
What's the meaning and useness of redirector_type(always three values found: "" and "aol" and "netscape") in nsImapIncomingServer ?
If we still based on currently used namespace architechture, 2 advanced bugs are sure to be found when we have fixed bug #166603 and bug #154778. Bug #166603 means that special folders like Drafts Templates Sent are not set flags correctly. Bug #154778 means that we can't access folders on Courier server when we set IMAP server directory to "INBOX". Now supposed we have conquered this two bugs. 1. Special flags are set because in our patch we can find the folder correctly. 2. We can access the folders when we set IMAP server directory on Courier server. This bug will be found soon: When the folderPickerMode is '1' which means we use "others" in "Copies & Folders" and we manully selected a folder to let it act as a special folder, and if we set IMAP server setting into "INBOX", the flags is not there any more. Acturely, they are not set correctly again. We have set flags into folder "INBOX.aaa.bbb", but in the folder pane the folder is "aaa.bbb". It has no flags set.
Hi, David, Can you see attachment 106747 [details] [diff] [review] for a more time? I still resist it. :-)
QA Contact: huang → gchan
I just put a long description in a very related bug 123707. The problem with that old bug is that it is listed as enhancement instead of normal. I would hope that either this bug gets updated to include the whole Windows trunk, or that old one gets a new lease on life with a normal bug status. Bug 114533 seems to be similar. Bug 174670 is another one. I am reporting this problem on Windows 2000 and Windows 98. Probably all Windows clients are affected. I'm also seeing the bug on Mozilla 1.2.1 and 1.3a. I have Courier IMAP server. Please see my fuller description in bug 123707.
also on windows
OS: Linux → All
bug 168060 may be blocked/a result by/of this bug.
Product: MailNews → Core
Does this still fail for you? I have "" in namespace and special folders works, but perhaps "" is just ignored?
I am using the workaround from comment #22 and a fairly old version of TBird, so I would not know if this works in a recent version without the workaround...
last patch author, philip zhao, is gone - email address is dead. no one has picked up the patch.
Assignee: mscott → bienvenu
QA Contact: grylchan → networking.imap
Assignee: bienvenu → nobody
Keywords: helpwanted
Whiteboard: [patchlove] workaround: comment 22
Product: Core → MailNews Core
Comment on attachment 106747 [details] [diff] [review] updated new patch Patch has bitrotted. nsMsgFolder.cpp is no longer present at mailnews/base/util/ See: http://hg.mozilla.org/comm-central/file/25a5e9e27cc4/mailnews/base/util/
Attachment #106747 - Attachment is obsolete: true
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: