44172 - improperly parented modal dialog in installer

Status: VERIFIED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect, P3)

Description

[Dan M]

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.
  Two such places are in nsInstall::Alert and nsInstall::Confirm.
  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.
  These two uses of the service are both using NS_WITH_PROXIED_SERVICE, which 
makes me think they're probably stuck using the service. Don't let my pessimism 
affect your opinion.

Comment 1

[Daniel Veditz [:dveditz]]

Reassigning to Don because he's done the fix already

Comment 2

[dbragg]

I did do the fix a while ago and it is in the shipping product and the trunk.  I
just didn't mark this bug fixed.  I think there was another bug about this on
our Confirm dialog and I fixed these at the same time.  Anyway, they have a
parent not (mParent) rather than the hidden window.

Comment 3

[Jimmy Lee]

Bug 52827 is similar to this.  This has been fixed.  Marking Verified.