11470 - Can't bring up password dialogs from protocols responding to proxied necko events

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P1)

Description

[scottputterman]

Steps to reproduce:

1.  click Get Msg
2.  Get a login dialog and hit cancel
3.  The dialog goes away but then another one pops up.  This dialog is garbage.
4.  Hit the close box of this second dialog and crash:

NTDLL! 77f76148()
nsWindow::Create(nsWindow * const 0x0c5c1b34, nsIWidget * 0x00000000, const
nsRect & {x=0 y=0 width=0 height=0}, nsEventStatus (nsGUIEvent *)* 0x09fc3aa9
HandleEvent(nsGUIEvent *), nsIDeviceContext * 0x0c546c80, nsIAppShell *
0x00000000, nsIToolkit * 0x00000000, nsWidgetInitData * 0x00000000) line 769
nsView::CreateWidget(nsView * const 0x0c5c1c70, const nsID & {...},
nsWidgetInitData * 0x00000000, void * 0x00000000) line 1239
DocumentViewerImpl::MakeWindow(void * 0x00000000, const nsRect & {x=0 y=0
width=0 height=0}, nsScrollPreference nsScrollPreference_kAuto) line 887 + 32
bytes
DocumentViewerImpl::Init(DocumentViewerImpl * const 0x0c544120, void *
0x00000000, nsIDeviceContext * 0x0c546c80, nsIPref * 0x00ea7c60, const nsRect &
{x=0 y=0 width=0 height=0}, nsScrollPreference nsScrollPreference_kAuto) line
394
nsWebShell::Embed(nsWebShell * const 0x0c546e30, nsIContentViewer * 0x0c544120,
const char * 0x0c5466f0, nsISupports * 0x00000000) line 942 + 69 bytes
nsDocumentBindInfo::OnStartRequest(nsDocumentBindInfo * const 0x0c5460c0,
nsIChannel * 0x0c544ef0, nsISupports * 0x00000000) line 1953 + 36 bytes
nsOnStartRequestEvent::HandleEvent(nsOnStartRequestEvent * const 0x0c546690)
line 212
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x0c546694) line 149 + 12 bytes
PL_HandleEvent(PLEvent * 0x0c546694) line 509 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f1b730) line 470 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00b606f0, unsigned int 49312, unsigned int 0,
long 15841072) line 932 + 9 bytes
USER32! 77e71250()

Comment 1

[Scott MacGregor]

This is actually a bug against the dialog guys. I saw this problem on Friday
when I tried to bring up a password dialog (in the UI thread) as a result ofa
proxied event coming from necko.

It has nothing to do with canceling per say but more to do with the network and
UI thread interaction when bringing up a dialog.

I'll re-assign to danm or matiskella....

Comment 2

[Scott MacGregor]

There are two problems here:

1) I wasn't aborting the pop command if the user hit cancel
2) the dialog helper stuff seems to choke when I bring up the password dialog
(IN THE UI THREAD) but in response to a necko event. This is the stack trace
Scott P. posted in this bug report.

I've checked in a fix for (1) for pop. If you hit cancel, we abort and you don't
get prompted again.

I'm going to re-assing (2) to davidm or danm. Here's the context behind (2):
Bring up messenger, hit get new mail, enter in a bogus password. We'll take that
password, make a connection to the server, determine that the password is
incorrect and will bring up another prompt for password dialog. It's this second
password dialog that chokes on at least windows. (but I think it is all
platforms).

Down and Dirty:
The first time, I bring up the password dialog (see
local\src\nsPop3Protocol::GetPassword), I'm coming from the UI thread.

The second time, necko has posted an event on the UI thread saying there is data
available. The pop3 protocol lives in the UI thread. It processes the event,
reading the data and determines that the password is bogus. It then calls
GetPassword again which brings up the dialog and we see the crash with the stack
trace above.

Comment 3

[Scott MacGregor]

Lisa, can you verify that I fixed (1) desribed above in tomorrows M9 candidate
builds?

I'm re-assigning to davidm for (2).

Comment 4

[don]

David, is this fixable for M9 or is it dependent upon the changes coming from
Dan?

Comment 5

[lchiang]

Ok, we will verify for (1) that mscott lists.

Comment 6

[Paul MacQuiddy]

2) of this sounds like bug 8511

Comment 7

[sol]

This needs to be fixed by PR1 - changed Status Whiteboard to reflect this

Comment 8

[scottputterman]

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

Comment 9

[chris hofmann]

fix sounds too risky for m9 endgame.  should be ready to go for m10

Comment 10

[Scott MacGregor]

Changing the subject to more clearly represent the bug we are reporting.
Bringing up the password dialog in response to proxied events from netlib (so we
are on the ui thread when we go to handle the event) are failing.

Comment 11

[davidm]

I checked in my code to proxy the calls to nsINetSupportDialogService to the new
code. It appears to work in the couple cases I tested. Let me know of any
problems.

Comment 12

[David :Bienvenu]

this didn't work for me, when I deleted the changes from the patches you sent,
and tried the tip build. It was the same old problem, and I hit the
stack trace described at the top of the bug.

Comment 13

[David :Bienvenu]

My problem involved IMAP (just set up imap but don't put your password in the
prefs file; then open an imap folder, to recreate the bug). I did not try pop.

Comment 14

[davidm]

update nsNetSupportDialogs.cpp. My version didn't make it in due to merge
conflict.

Comment 15

[David :Bienvenu]

yep, much better now. Cool. And I'm not seeing the problem with always having a
username and password text field either.

Comment 16

[lchiang]

Esther - can you verify? Thanks.
Davidm - your fixes are in M10, not M9, right? Want to confirm.

Comment 17

[Scott MacGregor]

David's fixes are in M10.

Comment 18

[esther]

Will check in M10 as soon as I finish M9 bug verifications.

Comment 19

[esther]

Verified using 1999090109 build on win98, but haven't tested on mac and linux
yet.  This bug is not marked ALL for platforms nor [PP] for platform parity, so
I will test on all platforms before verifying.

Comment 20

[esther]

Using 19990908 builds on mac and linux this is fixed.  I tested this by hitting
Cancel, no other dialogs came up.  Did a Get Msg and then I closed the password
dialog using the X (on win and linux), no crash.  Verified