Closed
Bug 44145
Opened 24 years ago
Closed 24 years ago
improperly parented modal dialog in psm-glue
Categories
(Core :: Security, defect, P3)
Core
Security
Tracking
()
VERIFIED
FIXED
People
(Reporter: danm.moz, Assigned: thayes0993)
References
Details
(Keywords: embed, Whiteboard: [nsbeta2-][nsbeta3+])
nsNetSupportDialog should only be used as a backup plan if no other nsIPrompt interface is available. It has been used in dozens of places because of its seductive convenience. But it's flawed, creating modal windows that don't behave correctly; the cause of various blanket bugs like 25684 and 39439 (both currently considered nsbeta2+). This problem can only be fixed by trying much harder to find a proper window to be the modal dialog's parent. nsNetSupportDialog should be relegated to providing backup when herculean efforts to locate the actual parent window fail for some reason. As an example, the cookie service has been taught to use a proper parent window for its dialog by laboriously storing a reference to that window's nsIPrompt in nsHTTPChannel, from which it can be extracted and passed around while processing notification events, punting to nsNetSupportDialog only when no other choice is available. That same sort of thing needs to be done in many more places. There are three such places in nsFSDR.cpp: Wallet_Confirm, wallet_GetString and wallet_getDoubleString. These seem to all track back to nsFSecretDecoderRing functions, which of course take no nsIPrompt parameter and appear to be called from hundreds of places. Woohoo. There's one more use in nsPSMUICallbacks.cpp, PromptUserCallback(); another function that ends up effectively being called in a zillion places throughout psm-glue. Happy recipients of this bug would spread the happiness most widely if they would start using a good nsIPrompt window. The "modal windows don't behave nicely" bugs are being made dependent on this bug and its siblings, and will eventually be closed as "as fixed as they're going to get" once these have all been considered. I'm not quite sure who to assign this to; dougt, I think. But thayes gets first bat since Doug is on leave at the moment.
Comment 1•24 years ago
|
||
nsFSDR is a tempory module that was to be used until the real nsSDR module was ready for prime time (the F means False). Terry switched in the real module a couple of weeks ago. So if his new module stays in the build, we will not have to worry about the problems in nsFSDR. However, if for some reason we find that we can't release the beta with the real nsSDR, we might have to use nsFSDR as a fallback in the beta2 release. So, optimistically, the only case we need to be concerned about is PromptUserCallback in nsPSMUICallbacks.cpp.
Comment 2•24 years ago
|
||
This should be nsbeta2+ since it blocks bug 25684 which is already nsbeta2+.
Keywords: nsbeta2
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Comment 4•24 years ago
|
||
Because of the use of nsCommonDialogs, or the use of the nsIPrompt service, this component can not be used for embedding. Adding the embedding keyword. How To Bring Up A Modal Dialog: You need to know what window you want to have modality against. This will be a nsIDOMWindow. Using a magic "hidden" window breaks modality and embedding application may not have this hack. One you have the parent DOMWindow, you simply: nsCOMPtr<nsIPrompt> prompter; aDOMWinow->GetPrompt(getter_AddRefs(prompter)); if (prompter) prompter-> Any other way that you try to bring up a dialog may not work in an embedding application. To reiterate, do not use the nsICommonDialogs interface -or- a nsIPrompt service if you want your component in a non-seamonkey app. I don't have a DOMWindow?! If you don't have a DOMWindow, you need to get one. Just about every place I saw that used the hidden window, could have used the real parent window with some work. There really is no place in the code where we should be displaying a modal dialog without knowing what parent window we are modal against. If you find a case where you don't have a top level window, lets talk.
Keywords: embed
Assignee | ||
Updated•24 years ago
|
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3+]
Comment 5•24 years ago
|
||
The problem is more than just not being embedable. The use of the hidden window will lead to a crash if you close down the rest of the browser before closing these modal dialogs. I'd like to help out here. I just looked over the code in nsPSMUICallbacks.cpp. Specifically in the routine DisplayURI which is where the dialog gets put up. The code is: rv = appShell->GetHiddenWindowAndJSContext( getter_AddRefs( hiddenWindow ), &jsContext ); ... // open the window nsIDOMWindow *newWindow; hiddenWindow->Open(jsContext, argv, 3, &newWindow); I'm already experienced at finding parent windows, having fixed all the places in wallet.cpp and singsign.cpp where such was needed. But I don't see how to get a parent window in this case. OK, doug, I'm taking you up on your offer. "LET'S TALK."
Comment 7•24 years ago
|
||
I fixed this for beta3. (your welcome terry :-))
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•24 years ago
|
||
Thank you Doug. So where does the parent window come from (or how do you discover it?)
You need to log in
before you can comment on or make changes to this bug.
Description
•