Http auth prompt from other tabs displays over current tab

VERIFIED FIXED

Status

()

Core
Embedding: APIs
--
major
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: bc, Assigned: Biesinger)

Tracking

({regression, relnote})

Trunk
regression, relnote
Points:
---
Bug Flags:
wanted1.8.1.x -
wanted1.8.0.x -
in-litmus ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:low] post 1.8-branch)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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/
Flags: blocking-firefox3?
Flags: in-litmus?
Looks like this was regressed by bug 265780 - the nsPromptService::Prompt* methods don't have any nsAutoWindowStateHelper helpers that launch the DOMWillOpenModalDialog event.
Severity: normal → major
Component: Tabbed Browser → Embedding: APIs
Flags: blocking-firefox3?
Product: Firefox → Core
QA Contact: tabbed.browser → apis
Hardware: PC → All
Flags: blocking1.9?
Blocks: 265780
Assignee: nobody → cbiesinger
Created attachment 276585 [details] [diff] [review]
one option

this makes nsIPromptService2::PromptAuth also dispatch the necessary event. I'm not sure that that's correct behaviour though...
Created attachment 276858 [details] [diff] [review]
patch

this moves the event dispatching stuff into the prompt service (as various people suggested in bug 277574 already)
Attachment #276585 - Attachment is obsolete: true
Attachment #276858 - Flags: superreview?(jst)
Attachment #276858 - Flags: review?(bzbarsky)
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 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).
Attachment #276858 - Flags: review?(bzbarsky) → review+

Updated

10 years ago
Attachment #276858 - Flags: superreview?(jst) → superreview+
Attachment #276858 - Flags: approval1.9?
Comment on attachment 276858 [details] [diff] [review]
patch

a=bzbarsky
Attachment #276858 - Flags: approval1.9? → approval1.9+
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
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: blocking1.9?
Resolution: --- → FIXED
Ah... so do embedding apps (like Camino, say) use this code?  Or did we just screw them over?  They _do_ use nsPrompt, right?
oh. um. I guess we may have, yeah... guess they'll have to add this code to their promptservice implementation if they want it
We should at least relnote it then...
Keywords: relnote
Not really sure who the camino folks who should see this are, if any...

Comment 12

10 years ago
Camino is still using nsIAuthPrompt for HTTP auth, so we're fine (unless I'm misunderstanding, but tests look fine in a fresh build).
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.
Although given the existence of camino/src/browser/CocoaPromptService.mm maybe not!
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

10 years ago
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.
Flags: wanted1.8.1.x-
Flags: wanted1.8.0.x-
Whiteboard: [sg:low] post 1.8-branch
Group: security
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.