185959 - Many function(For exam. Reply, Forward) works and leads to crash before user login

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Antonio.Xu]

The function of Reply, Forward to a message should not work before user enter
password and login. But They are working in Mozilla 1.2 and leads to crash.

Steps to reproduce for Reply:
1. Start an IMAP account and select its any folder to try to login, cancel the
password dialog.
2. Select a message in a folder('INBOX' or another), cancel the password dialog.
3. You can see the Reply, Reply All, Forward button freshed and can work now.
4. Select Reply button.
5. The reply window will jump out with the 'To:' editbox null. Press at the
'To:' to try to add an address.
6. Crash.

Actual Results: crash 
Expected Results:  no crash

Comment 1

[Matthias Versen [:Matti]]

*** Bug 186096 has been marked as a duplicate of this bug. ***

Comment 2

[Antonio.Xu]

Attachment: patch version 1.0

I have reserched this bug, I found when we replay a email, mailnews will try to
load the email's body from Imap server, but if we do it before login imap
server. mozilla will crash due to due to mailnews didn't set
m_imapMiscellaneousSink in imapurl. So when mailnews login unsuccessfully,
mailnews will use m_imapMiscellaneousSink to fire some event, but
m_imapMiscellaneousSinke is null pointer. I think the best way to fix this
problem is add a judgement  to aviod use null pointer, furthermore we should
popup a the login dialog to prompt user inputing the password, so I set
m_imapServerSink in imapurl. So mailnews will popup a dialog before mailnew get
email's body for quote.

Comment 3

[Antonio.Xu]

Attachment: patch version 1.1

please review

Comment 4

[Henry Jia]

Comment on attachment 109737 [details] [diff] [review]
patch version 1.1

Anto, thx for the patch. But it needs to be improved.

1. You use 'mailnewsUrl->SetFolder(folder);' before 'if (mailnewsUrl)'. It's
also a issue in previous code.

2. 'mailnewsUrl->SetFolder(folder);' is executed in SetImapUrlSink, so no need
to do it again. Also, no need to do 'if (mailnewsUrl)'.

Also, why 'NS_ENSURE_SUCCESS(rv, rv);'?

Comment 5

[David :Bienvenu]

I did check in the biff crash part of the fix (the check for the null
m_imapMiscellaneousSink) along with some other changes because I was trying to
reproduce this crash.

Comment 6

[Antonio.Xu]

Attachment: patch version 1.2

change some code according to Henry's advice

Comment 7

[Antonio.Xu]

Comment on attachment 109970 [details] [diff] [review]
patch version 1.2

Hi Henry,Could you review my new patch?
Thank you.

Comment 8

[Henry Jia]

Comment on attachment 109970 [details] [diff] [review]
patch version 1.2

Thx for the patch.

r=henry

Comment 9

[Antonio.Xu]

Comment on attachment 109970 [details] [diff] [review]
patch version 1.2

Hi Bienvenu, Could you super review my patch?
Thank you.

Comment 10

[David :Bienvenu]

antonio, can I get a -uw diff so I can see what you've really changed and not
just the whitespace changes? thx.

Comment 11

[Antonio.Xu]

Attachment: patch for super review

This patch was maked by "cvs -uw".

Comment 12

[David :Bienvenu]

Comment on attachment 110921 [details] [diff] [review]
patch for super review

sr=bienvenu, thx. I'm a little worried about removing the null checks for
folder and mailnewsurl but I guess they'll never be null...

Comment 13

[Henry Jia]

Yes. The pointer is not null when code goes here.

Comment 14

[Antonio.Xu]

fix checked in trunk