78791 - Folder creation fails due to incorrect verification (i.e., verifying parent..not itself)

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect, P1)

Description

[racham]

When we try to create a folder dynamically on IMAP server, we fail while the
ensureFolderExists returns the wrong value. David (bienvenu) says that it is due
to the fact that we are verifying the existence of the parent and returning true
instead of checking the exitence of folder itself. That makes the app to think
that the folder already existed and hence there is no need to create one. That
will be a problem once we try to copy Sent message/Draft message or template
into that folder.

Comment 1

[racham]

Unless we do the right thing here, we have to hold on fix for bug 23625. Marking
dependency.

Adding some of david's comments on this discussion :

OK, I see the problem. Basically, this code that tries to ensure that a folder
exists, is checking if the *parent* folder is verified online, not itself, which
is wrong.So it thinks the folder exists and doesn't try to create it.

- David

void nsImapProtocol::OnEnsureExistsFolder(const char * aSourceMailbox)
{

  List(aSourceMailbox, PR_FALSE); // how to tell if that succeeded?
  PRBool exists = PR_FALSE;
  if (m_imapMailFolderSink)
   m_imapMailFolderSink->GetFolderVerifiedOnline(&exists);

  if (exists)
  {
     Subscribe(aSourceMailbox);
  }
  else
  {

Comment 2

[scottputterman]

marking nsbeta1+ and moving to 0.9.1 to see if we can get this and 23625 in.

Comment 3

[David :Bienvenu]

Attachment: proposed fix

Comment 4

[David :Bienvenu]

bhuvan, could you review the patch? After that, I'll get an sr from seth or
mscott and check it in.

Comment 5

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

in the idl folderVerifiedOnline instead of FolderVerifiedOnline
the folder name is going to be the modified UTF7, right?

Comment 6

[racham]

We do allow i18n names for folders..right..? So, does the input param need to
wstring instead of string..?

bhuvan

Comment 7

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

that's why I asked about aSourceMailbox being in modified UTF7

Comment 8

[David :Bienvenu]

Attachment: patch to address Seth's concerns

Comment 9

[David :Bienvenu]

Add Seth to cc list.

Comment 10

[David :Bienvenu]

Bhuvan, just saw your comment - no, imap uses modified utf-7 to store folder
names, and we use utf8, both of which are normal strings, not wstrings.

Comment 11

[racham]

I was just talking to Naoki about that and also referred
ftp://ftp.isi.edu/in-notes/rfc2192.txt. 

r=bhuvan (on new patch).

Comment 12

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

sr=sspitzer

Comment 13

[David :Bienvenu]

fix checked in. Bhuvan, let me know if you have any problems. If this helps with
23652, you should probably mark this one verified.

Comment 14

[Karen Huang]

We need to verify all the 0.9.1 bugs,
Bhuvan, can you help for verifying this bug? Please mark as verified if you 
verify the code for this bug. Thanks.

Comment 15

[racham]

ok. sure.

Let me check with my code and I will update the status here.

bhuvan

Comment 16

[racham]

I tested this with my patch.

There are couple of points that I need to mention.

1. When I create a new IAMP account for the very first time (which has no 
sent,drafts or templates folders) and try to send a message which triggers the 
action of creating Sent folder, the folder creation failed. It came back with 
message that folder did not exist. Attaching the IMAP log (imapLog_FAILURE). So, 
just on the very first attepmt, it fails.

2. On the subsequent occasions, the folder creation happens without any 
problems. Will attach iamp log for this also (imapLog_SUCCESS)

David, you can look at the logs and let me know If the problem is somewhere else 
for that one case. If needed, I can open a new bug. Until then, I am reopening 
the bug and moving the TFV to 0.9.2 as bug 23625 also targeted to the same 
milestone.

Comment 17

[racham]

Attachment: imapLog_FAILURE

Comment 18

[racham]

Attachment: imapLog_SUCCESS

Comment 19

[David :Bienvenu]

I have no idea why it would fail the first time and work the second time - it
should be running the same code both times.

Comment 20

[David :Bienvenu]

I tried this with an account that didn't have a templates folder and the
templates folder was created fine (the code that creates sent/templates/etc is
the same). I'll investigate more, but I'm not sure what went wrong when you
tried it. I have a feeling it might have to do with running against a brand new
account, for some reason.

Comment 21

[David :Bienvenu]

this bug is fixed. The problem is in the compose code. I'll go update the parent
bug with comments.

Comment 22

[Karen Huang]

David, this bug's fix is ONLY for trunk build, right?

Comment 23

[David :Bienvenu]

no, it's been fixed for a while. But there is still a problem in the compose
code which makes it so you can't verify this bug.

Comment 24

[Karen Huang]

>this bug is fixed. The problem is in the compose code. I'll go update the 
>parent bug with comments.

Can I know that parent bug number? Is that bug 23625?

Comment 25

[David :Bienvenu]

yes, the bug that's dependent on this one is the parent bug.

Comment 26

[Karen Huang]

OK. Thanks for the info. Adding curent status on Status Whiteboard for this bug. 

Comment 27

[Karen Huang]

We need to verify all the 0.9.2 bugs, and since dependence bug 23625 fixed.
Bhuvan, can you help for verifying this bug? Please mark as verified if you 
verify the code for this bug. Thanks.

Comment 28

[racham]

This is fixed as we can create new folders on demand.

Verified.