85825 - Mozilla will repeatedly try to send out undeliverable messages from the outbox

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Christopher Blizzard (:blizzard)]

From Howard Jacobson:

Bug is that Mo repeatedly tries to send the badly addressed message as it
processes each remaining message in the folder.  So, if you have ten unsent
messages, and the misaddressed message is the third in the folder:

1 The first two messages go out.
2 On attempting to send the third message (misaddressed) message, which is not
the first in the folder, you get the two error dialogs.
3 The fourth message, which is now the second in the folder, goes out.
4 Mo attempts to send the misaddressed message again and you get the two dialogs
again.
5 The fifth message, which is now the second in the folder, goes out.
6 Repeat 4-5.

Comment 1

[David :Bienvenu]

send later is really more of a message compose/send feature, not an offline
feature. It's part of the mail sending code.

Comment 2

[grylchan]

changing component from offline to composition.

reassigning qa.

Comment 3

[Scott MacGregor]

I'll try to fix this for JF since he's on vacation till .9.4 is over and that's
too late for enterprise. 

Looks like after we successfully send a message, we refresh the enumerator for
the messages in the unsent folder. This refreshing causes us to resend the bad
message again. If we don't refresh the enumerator and just send each message in
it once then that should fix it I believe. Although I'm not sure what I can do
to an unsent message to corrupt  it so we won't send it to test this. 

Comment 4

[Scott MacGregor]

Attachment: the fix

Comment 5

[Scott MacGregor]

JF's on vacation till after .9.4 so I'm taking this bug since eMojo needs it by
.9.4. 

We were using an RDF enumerator on the unsent message folder to send the unsent
messages. Unfortunately while iterating over the enumerator, we are removing
messages from the folder which screws up the enumerator!  In the case where we
have  a failure and a message really isn't removed on send, our enumerator never
hits the end of the list. We always think we have at least one item left. 

To fix this, I got the enumerator and then threw the items into an
nsISupportsArray. Then I extracted an enumerator from this array. So basically
we take a snap shot of the unsent folder  then enumerate through that list
instead of  using an enumerator which changed.

Comment 6

[Scott MacGregor]

In addition to this problem, there are several others which aren't addresed by
this patch:

You get too many alert dialogs when we fail to send a message via send later.
you get an alert from the news server (I was posting on my bad message), 
an alert from SMTP saying "Send Message failed. SendMessage Failed." Yes the
text is repeated. Then you get a 3rd dialog saying "Send Message Failed."

I'll file a spin off bug for the error dialog problems and start investigating
those. 

QA, to test, I did the following:
1) add some valid messages to your unsent folder
2) add a news posting with a bogus newsgroup name (foakldfjlkajf.mcom.com) to
your unsent folder
3) add 3 or 4 more valid messages

Now send unsent mail. Verify that all the messages except for the bogus news one
are sent.

Comment 7

[Scott MacGregor]

96386 is the spin off bug for the error dialogs.

Comment 8

[David :Bienvenu]

why doesn't DeleteCurrentMessage
 delete the message from the folder, and then refreshing the enumerator work? It
seems like it should. Your change looks OK, but I'm confused why the existing
code didn't work.

Comment 9

[Scott MacGregor]

because refreshing the enumerator causes the bad message to be back in the
enumerator again. So we try to send it again. 

Comment 10

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

mEnumerator->First(); // is this necessary? 

If that isn't necessary, can you remove it?  if it is, can you add a comment
explaining why you need to do it?

other than that, r=sspitzer

Comment 11

[Scott MacGregor]

the call to first was not necessary. I took it out and seth re-reviewed it so we
are good to go.

Comment 12

[Asa Dotzler [:asa]]

a=asa on behalf of drivers

Comment 13

[Scott MacGregor]

I checked in the  fix for this. I listed steps to reproduce above.

Comment 14

[Sheela Ravindran]

mscott thanks for putting down the test case. It was easy to see the before and 
after fix. 
Verified that the undeliverable message is attempted to send once and the 
remaining 3 messages after the bad message was sent.

verified with build from 2001-08-29-06 win98, mac, linux