loop over all identities on sending unsent messages, or fix how we find the unsent message folder

VERIFIED FIXED in mozilla0.9.9

Status

MailNews Core
Backend
P1
normal
VERIFIED FIXED
16 years ago
9 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: Cavin Song)

Tracking

Trunk
mozilla0.9.9
x86
All

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

nsMsgSendLater::GetUnsentMessagesFolder() takes an identity and calls
LocalMessageFolder()
which calls
GetUnsentMessagesFolder()
which calls
GetFoldersWithFlag(MSG_FOLDER_FLAG_QUEUE,..)

I need to investigate, and see what happens if I try to send unsent message from
an account without an identity.

loop over all identities on sending unsent messages, or fix how we find the
unsent message folder

this might be related to why you're unable to send migrated unsent messages if
you migrating them from 4.x pop account.

Updated

16 years ago
Keywords: nsbeta1

Updated

16 years ago
Keywords: nsbeta1 → nsbeta1+

Comment 1

16 years ago
p1 for investigation and if it's the cause of not migrating the unsent messages
folder.
Priority: -- → P1

Updated

16 years ago
QA Contact: esther → sheelar

Updated

16 years ago
Target Milestone: --- → mozilla0.9.9

Comment 2

16 years ago
reassigning to Cavin.
Assignee: sspitzer → cavin
(Assignee)

Comment 3

16 years ago
I couldn't reproduce the "unable to send migrated unsent messages if you migrate 
them from 4.x pop account".  All msgs got sent fine. However, for each msg I got 
the following error msg:

  Couldn't open Sent Mail folder. Please verify that your Mail preference
  are correct.

and this is caused by that 4.x creates the following FCC: header for each of the 
unsent msg in the unsent message folder:

  FCC: /C|/Program Files/Netscape/Users/alldefaults_pop/mail/Sent

Since the destination sent folder is set we simply use it (which causes problems 
when we try to find it in rdf).

This is the only problem I see so far.
(Assignee)

Comment 4

16 years ago
Unsent msgs in migrated IMAP accounts has the same problem. In summary, the FCC: 
headers created by 4.x are the following:

 "/C|/Program Files/Netscape/Users/alldefaults_pop/mail/Sent" (for POP)
 "IMAP://nsmail-2"                                            (for IMAP)

In 6.x the valid folder urls are:

 "mailbox://qatest33@nsmail-1.netscape.com/Sent" (for POP)
 "imap://qatest22@nsmail-2.netscape.com/Sent"    (for IMAP)

One way to fix the problem is during migration where we know how to translate 
4.x folder urls to 6.x. But this is tedious because we need to parse the unsent 
msg folder and handle it specially.  Another way to do it is to make sure that 
the fcc folder url is valid during sending.
(Assignee)

Comment 5

16 years ago
Created attachment 66564 [details] [diff] [review]
Fix copying unsent msgs to sent folder problem.
(Assignee)

Comment 6

16 years ago
The fix is to make sure the fcc folder uri is valid and exists. If not then the 
configured one is used.

Can I get a review from ducarroz and sspitzer? Thanks.
Comment on attachment 66564 [details] [diff] [review]
Fix copying unsent msgs to sent folder problem.

the patch looks good.  I could have sworn this code already lived somewhere
else.
check with bhuvan and / or naving.  I think it has been written before,
but that might have been in js.  It seems like a common thing, not specific to
compose.

research that, and fix three minor style nits, and sr=sspitzer

here are my 3 nits:

waterson pointed out to me that this makes debugging hard, since you can't put
a break on the return.

>+  if (NS_FAILED(rv)) return PR_FALSE;

try this instead:

if (NS_FAILED(rv))
  return PR_FALSE;

>+  if (NS_FAILED(rv)) return PR_FALSE;

same thing.

a style suggestion:

>+  nsCOMPtr <nsIMsgFolder> thisFolder;
>+  thisFolder = do_QueryInterface(resource, &rv);
>+  if (!thisFolder || NS_FAILED(rv))
>+    return PR_FALSE;
>+
>+  // Parent doesn't exist means that this folder doesn't exist.
>+  nsCOMPtr<nsIMsgFolder> parentFolder;
>+  rv = thisFolder->GetParentMsgFolder(getter_AddRefs(parentFolder));
>+  if (NS_SUCCEEDED(rv) && parentFolder)
>+    return PR_TRUE;
>+  else
>+    return PR_FALSE;
>+}
Comment on attachment 66564 [details] [diff] [review]
Fix copying unsent msgs to sent folder problem.

The code looks good but I would rather see the function IsValidFolderURI in
nsMsgCompUtils.cpp or event better in mailnews/base/utils
Attachment #66564 - Flags: review+

Comment 9

16 years ago
you can just do GetParent() instead of doing GetParentMsgFolder() because
I don't see any uses of parentMsgFolder. 
(Assignee)

Comment 10

16 years ago
Created attachment 66908 [details] [diff] [review]
Moved new routine to compose utils and incorporated comments.

Comment 11

16 years ago
+  if (NS_SUCCEEDED(rv) && parentFolder)
+    return PR_TRUE;
+  else
+    return PR_FALSE;

personally, I think this should be "return NS_SUCCEEDED(rv) && parentFolder" -
that generates the least code.

I agree with JF that this would best be in base/utils, not comp utils, since I
imagine other code could use this.
(Assignee)

Updated

16 years ago
Attachment #66564 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #66908 - Attachment is obsolete: true
(Assignee)

Comment 12

16 years ago
Created attachment 66927 [details] [diff] [review]
Moved the routine to msgbsutl.
(Assignee)

Comment 13

16 years ago
Created attachment 66926 [details] [diff] [review]
Moved the routine to msgbsutl.

Comment 14

16 years ago
Comment on attachment 66927 [details] [diff] [review]
Moved the routine to msgbsutl.

sr=bienvenu,thx
Attachment #66927 - Flags: superreview+
Comment on attachment 66927 [details] [diff] [review]
Moved the routine to msgbsutl.

R=ducarroz
Attachment #66927 - Flags: review+
(Assignee)

Updated

16 years ago
Attachment #66926 - Attachment is obsolete: true
(Assignee)

Comment 16

16 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 17

16 years ago
Using commercial trunk:
2002030103 on NT 4.0
2002030412 on Mac 9.2.2
20022208 linux 2.2 (had problems w/current builds migrating profile)

Verified that a migrated profile w/usent mesgs gets migrated over
in current commercial trunk builds. And once you login, you can
send your unsent mesgs w/no problems and the mesgs are actually sent.
Tried both imap and pop mail accounts.

Cavin, in addiion I notice this:
-After sending my unsent mesgs, it fails to copy it to my 'sent folder' 
  either on mail acount or local folders. But If I do send later
  from commercial trunk, it does copy it. I guess when we migrate 'unsent
  mesgs' over, it doesn't know how to copy itself to sent folder.
  New bug?
- If I go back to the old 4.x profile, my unsent mesgs are still present.
  I assume not a bug?

I'll wait for your response before marking verified.

(Assignee)

Comment 18

16 years ago
> After sending my unsent mesgs, it fails to copy it to my 'sent folder' 
> either on mail acount or local folders.
>
So did you see any error dialog indicating that the sent folder could not be
opened? Or the process just went silently? I'm curious to know how do you know
it failed to copy sent msgs to sent folder.

> But If I do send later from commercial trunk, it does copy it.
>
Does normal send copy msgs to sent folder?

> If I go back to the old 4.x profile, my unsent mesgs are still present.
> I assume not a bug?
>
This is the right behavior.



Comment 19

16 years ago
>> After sending my unsent mesgs, it fails to copy it to my 'sent folder' 
>> either on mail acount or local folders.
>>
>So did you see any error dialog indicating that the sent folder could not be
>opened? Or the process just went silently? I'm curious to know how do you know
>it failed to copy sent msgs to sent folder.

It failed silently. After sending the Unsent mesgs, I checked the Sent
megs folder (either on mail or local folders acnts) and didn't see
a copy.

> But If I do send later from commercial trunk, it does copy it.
>
>Does normal send copy msgs to sent folder?

yes. I tried doing a regular send and doing a send later/send unsent
mesgs and it succesfully copied to the sent folder.
(Assignee)

Comment 20

16 years ago
Gary, after the account is migrated can you take a look at the migrated 6.x 
unsent message folder and see what the FCC: header is set to for each message?  
See Comment #3 of an example.

Comment 21

16 years ago
Sorry for not getting back to you earlier Cavin.

To answer your question:
If 4.x profile has sent folders dir under local act:

for my imap
/c|/Program Files/Netscape/Users/glinzilla/mail/Sent

if 4.x profile has sent folders under the mail act
iamp://linzilla.mcom.com

But since all profiles are stored under windows aplication folder
and not like 4.x is that why it isn't working?

Comment 22

16 years ago
Reopening Bug, Discussed with Cavin and he found the problem..
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 23

16 years ago
I see the problem and it's caused by patch of bugscape 10163. A better fix to 
the problem is to set the right FCC url when it's initialized in 
InitCompositionFields(). This way we'll always have the correct url for the 
compose/send code to deal with.
(Assignee)

Comment 24

16 years ago
Created attachment 74263 [details] [diff] [review]
Proposed patch for the regression.

Set the right FCC url at init time.

Updated

16 years ago
Attachment #74263 - Flags: review+
Comment on attachment 74263 [details] [diff] [review]
Proposed patch for the regression.

R=ducarroz

Comment 26

16 years ago
Comment on attachment 74263 [details] [diff] [review]
Proposed patch for the regression.

sr=bienvenu
Attachment #74263 - Flags: superreview+
(Assignee)

Updated

16 years ago
Attachment #66927 - Attachment is obsolete: true

Comment 27

16 years ago
Comment on attachment 74263 [details] [diff] [review]
Proposed patch for the regression.

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #74263 - Flags: approval+
(Assignee)

Comment 28

16 years ago
Fix checked in on trunk.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 29

16 years ago
Using commercial trunk builds:
2002032008 linux 2.2 
2002-03-21-08-trunk mac 9.2.2 
2002-03-21-10-trunk win 2k

Verified that a migrated profile w/unsent mesgs gets migrated over
in current commercial trunk builds. And once you login, you can
send your unsent mesgs w/no problems and the mesgs are actually sent.
Tried both imap and pop mail accounts. Also tested sending a mesg
to a newsgroup.

Tried sending via file menu, going offline then online and sending
mesg at prompt.

Also copy of the unsent mesg is NOW copied to the sent folder.

Marking as verified for Sheela since she's out.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.