391100 - Http auth prompt from other tabs displays over current tab

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Bob Clary [:bc] (inactive)]

Bug 277574 has regressed on the trunk. I've confirmed with a recent trunk on windows and as far back as 1.9a1 on linux.

Steps to reproduce:

1. load a site in one tab.
2. load another site requiring basic authentication in a background tab.
3. switch back to the first tab.

Expected results:

When the authentication dialog appears, the active tab should be switched to the tab presenting the dialog.

Actual results:

The first tab remains active while the authentication dialog from the second tab appears.

Easy test on the command line:

firefox -P profilename http://cnn.com/ http://bclary.com/log/2007/08/06/

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Looks like this was regressed by bug 265780 - the nsPromptService::Prompt* methods don't have any nsAutoWindowStateHelper helpers that launch the DOMWillOpenModalDialog event.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: one option

this makes nsIPromptService2::PromptAuth also dispatch the necessary event. I'm not sure that that's correct behaviour though...

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

this moves the event dispatching stuff into the prompt service (as various people suggested in bug 277574 already)

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Oh, I should probably mention, the cause of this bug is that passwordmanager uses the prompt service method to show the password dialog. Previously, it used the nsIAuthPrompt method. So bug 265780 didn't really cause this directly.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 276858 [details] [diff] [review]
patch

Looks good if you get rid of the extraneous newlines after some of the blocks of code you added (some places you added two newlines after the block).

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 276858 [details] [diff] [review]
patch

a=bzbarsky

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in windowwatcher/src/nsPrompt.cpp;
/cvsroot/mozilla/embedding/components/windowwatcher/src/nsPrompt.cpp,v  <--  nsPrompt.cpp
new revision: 1.30; previous revision: 1.29
done
Checking in windowwatcher/src/nsPrompt.h;
/cvsroot/mozilla/embedding/components/windowwatcher/src/nsPrompt.h,v  <--  nsPrompt.h
new revision: 1.11; previous revision: 1.10
done
Checking in windowwatcher/src/nsPromptService.cpp;
/cvsroot/mozilla/embedding/components/windowwatcher/src/nsPromptService.cpp,v  <--  nsPromptService.cpp
new revision: 1.35; previous revision: 1.34
done
Checking in windowwatcher/src/nsPromptService.h;
/cvsroot/mozilla/embedding/components/windowwatcher/src/nsPromptService.h,v  <--  nsPromptService.h
new revision: 1.14; previous revision: 1.13
done
Checking in windowwatcher/src/nsWindowWatcher.cpp;
/cvsroot/mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp,v  <--  nsWindowWatcher.cpp
new revision: 1.135; previous revision: 1.134
done

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ah... so do embedding apps (like Camino, say) use this code?  Or did we just screw them over?  They _do_ use nsPrompt, right?

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

oh. um. I guess we may have, yeah... guess they'll have to add this code to their promptservice implementation if they want it

Comment 10

[Boris Zbarsky [:bzbarsky]]

We should at least relnote it then...

Comment 11

[Boris Zbarsky [:bzbarsky]]

Not really sure who the camino folks who should see this are, if any...

Comment 12

[Stuart Morgan]

Camino is still using nsIAuthPrompt for HTTP auth, so we're fine (unless I'm misunderstanding, but tests look fine in a fresh build).

Comment 13

[Boris Zbarsky [:bzbarsky]]

The question is whether you provide your own prompt service implementation or whether you're using the default one, complete with XUL dialogs.....  Sounds like the latter.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Although given the existence of camino/src/browser/CocoaPromptService.mm maybe not!

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

Well what this change potentially breaks isn't the dialogs as such, only the switching to that tab when a modal dialog/auth dialog is shown from it.

Comment 16

[Stuart Morgan]

Right, we have our own prompt service. It's been set up to force a switch to the tab the prompt came from since we fixed bug 262887 for Camino though, all handled in our code.

Comment 17

[Carsten Book [:Tomcat]]

verified fixed using  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8) Gecko/2007091216 GranParadiso/3.0a8 and the steps to reproduce from this bug