82645 - Sending a web page while offline generate 2 messages in the unsent folder

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P2)

Description

[Jean-Francois Ducarroz]

I am working offline. I create a message and attach a web page. When I sen the 
message (Send Later), I get an alert telling me there is a problem with the 
remote attachment and asking if I want to continue without it. If I say yes, two 
messages will be created into the unsent message!

Expected result, should be only one message without attachment.

Comment 1

[Jean-Francois Ducarroz]

nominating nsbeta1. I think is bad that potentially we send twice the same 
message!

Comment 2

[scottputterman]

marking nsbeta1+.  Putting into 0.9.2 because I don't think attaching a web page
will be too common, but if you can get a fix soon I'd like to consider this for
0.9.1

Comment 3

[Jean-Francois Ducarroz]

Attachment: proposed fix, v1

Comment 4

[Jean-Francois Ducarroz]

please ignore the previous patch, it's for bug 82460.

Comment 5

[Jean-Francois Ducarroz]

Attachment: proposed fix, v1 This is the right one this time.

Comment 6

[Jean-Francois Ducarroz]

This is a simple fix:
In nsMsgComposeAndSend::HackAttachments, we should not call GatherMimeAttachments ourself if we call 
SnarfAttachment as SnarfAttachment will call indirectly (synchronously or asynchronously) 
GatherMimeAttachments.

This problem wasn't showing up until recently because nsIURILoader::OpenURI was not calling the onStopRequest 
function of the listener synchronously, even when we were offline. This is not the case anymore.

Comment 7

[scottputterman]

putting on 0.9.1 radar.  Please just make sure that attaching things besides web
pages works online and offline.

Comment 8

[varada]

r=varada

Comment 9

[David :Bienvenu]

is it really right to ignore m_pendingAttachment count? Not that I know the
first thing about that code...

Comment 10

[David :Bienvenu]

Put another way, is there a reason you're not checking that pending attachment
count anymore?

Comment 11

[Jean-Francois Ducarroz]

Yes, needToCallGatherMimeAttachments replace the old logic that was using 
m_pendingAttachment. The goal is to call GatherMimeAttachments when we are done 
fetching remote attachment or if we don't have remote attachment. In the case we 
have remote attachment, we will call snarfAttachment which will call 
GatherMimeAttachments itself when done. In this case, I set 
needToCallGatherMimeAttachments to false to avoid calling GatherMimeAttachments 
a second time.

Comment 12

[David :Bienvenu]

ok, r=bienvenu

Comment 13

[Jean-Francois Ducarroz]

thanks, seeking approval now...

Comment 14

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to 0.9.1 (on behalf of drivers)

Comment 15

[Jean-Francois Ducarroz]

fixed and checked in

Comment 16

[laurel]

OK using may31 commercial trunk builds: linux rh6.2, mac OS 9.0, win98