Stub installer: Firefox is already dialog does not appear on front when it pops up, which makes the stub look like it's hung

VERIFIED FIXED in Firefox 18

Status

()

VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Yoric, Assigned: rstrong)

Tracking

unspecified
Firefox 19
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox18 verified)

Details

(Whiteboard: [stub+])

Attachments

(3 attachments, 1 obsolete attachment)

I just tested the stub installer. In appearance, the installation seemed to end with "Application not responding". In fact, a dialog, was informing me that Firefox was already launched, but for some reason this dialog was hidden.

This makes us look quite bad in front of users.
Created attachment 668880 [details]
What was hidden below
Affected me also. Problems with modal dialog boxes aside, UX review (Bug 798869) may suggest not using modal dialogs at all.

Updated

6 years ago
Blocks: 322206

Updated

6 years ago
Summary: Stub installer: Application not responding → Stub installer: Firefox is already dialog does not appear on front when it pops up, which makes the stub look like it's hung
Assignee: nobody → robert.bugzilla
Duplicate of this bug: 799175

Updated

6 years ago
Whiteboard: [stub+]
Status: NEW → ASSIGNED
Created attachment 669832 [details] [diff] [review]
patch rev1

The check for the Firefox Message Window can only be done reliably from the unelevated process which typically doesn't display UI. The options are to hide the existing installer window and not set MB_TOPMOST|MB_SETFOREGROUND on the messagebox which could be missed by the user or to set MB_TOPMOST|MB_SETFOREGROUND which will display the messagebox in front of other apps. I opted for the second.
Attachment #669832 - Flags: review?(netzen)
(In reply to Robert Strong [:rstrong] (do not email) from comment #5)
> Created attachment 669832 [details] [diff] [review]
> patch rev1
> 
> The check for the Firefox Message Window can only be done reliably from the
> unelevated process which typically doesn't display UI. 

Why can't it be done from the elevated process?
Even if you elevate to another user account it will be using the same session, window station and desktop.  I suspect the reason the message box shows underneath is because it sets its owner to the unelevated window which comes before in z-order.

> The options are to
> hide the existing installer window and not set MB_TOPMOST|MB_SETFOREGROUND
> on the messagebox which could be missed by the user or to set
> MB_TOPMOST|MB_SETFOREGROUND which will display the messagebox in front of
> other apps. I opted for the second.

If you set MB_TOPMOST the user will be unable to move anything on top of that message box unless it is also top most.  Even after the user sees the message.  Perhaps we can just set MB_SETFOREGROUND?

Also should we be doing this from the installer and elsewhere for other message boxes? I remember seeing a bug report at some point about this happening in the normal installer when the app is already running.
(In reply to Brian R. Bondy [:bbondy] from comment #6)
> (In reply to Robert Strong [:rstrong] (do not email) from comment #5)
> > Created attachment 669832 [details] [diff] [review]
> > patch rev1
> > 
> > The check for the Firefox Message Window can only be done reliably from the
> > unelevated process which typically doesn't display UI. 
> 
> Why can't it be done from the elevated process?
> Even if you elevate to another user account it will be using the same
> session, window station and desktop.  I suspect the reason the message box
> shows underneath is because it sets its owner to the unelevated window which
> comes before in z-order.
Because we need to find the Firefox Message Window which isn't available to the elevated process when launched as a different user.

> > The options are to
> > hide the existing installer window and not set MB_TOPMOST|MB_SETFOREGROUND
> > on the messagebox which could be missed by the user or to set
> > MB_TOPMOST|MB_SETFOREGROUND which will display the messagebox in front of
> > other apps. I opted for the second.
> 
> If you set MB_TOPMOST the user will be unable to move anything on top of
> that message box unless it is also top most.  Even after the user sees the
> message.  Perhaps we can just set MB_SETFOREGROUND?
Sure

> Also should we be doing this from the installer and elsewhere for other
> message boxes? I remember seeing a bug report at some point about this
> happening in the normal installer when the app is already running.
We don't do this elsewhere iirc and if we did that would need a new bug anyways.
(In reply to Robert Strong [:rstrong] (do not email) from comment #7)
> (In reply to Brian R. Bondy [:bbondy] from comment #6)
> > (In reply to Robert Strong [:rstrong] (do not email) from comment #5)
> > > Created attachment 669832 [details] [diff] [review]
> > > patch rev1
> > > 
> > > The check for the Firefox Message Window can only be done reliably from the
> > > unelevated process which typically doesn't display UI. 
> > 
> > Why can't it be done from the elevated process?
> > Even if you elevate to another user account it will be using the same
> > session, window station and desktop.  I suspect the reason the message box
> > shows underneath is because it sets its owner to the unelevated window which
> > comes before in z-order.
> Because we need to find the Firefox Message Window which isn't available to
> the elevated process when launched as a different user.

A process can find another HWND it's looking for, as long as it's in the same Desktop.  
A process can interact with an HWND it's looking for, as long as it's in the same Desktop and as long as it's equal or higher integrity. (Some exceptions exist that low priority processes can interact with higher ones).

On the Window's Session running in front of you right now, you have a Winlogon Desktop (which is the lock screen), Default Desktop (which is what you see now), ScreenSaver Desktop, and Secure Desktop.  The Secure Desktop is only used for UAC prompts.  

Both the elevated installer and unelevated installer run inside the Default Desktop.  Even if you are a limited user account and you elevate to another user, if you see it on your screen alongside other apps, then you are running in the same Desktop.  Any API involving an HWND will work within the same Desktop, even if it's from different users.  This is part of the reason Windows Vista introduced Session 0 isolation.  People were showing UIs from services and then the first logged on user could interact with that UI and get system level access.

I would prefer to use the FindWindow call inside the elevated process and only use MB_SETFOREGROUND.
Quite possibly... the last time I checked that it was a very long time ago and I was under a tight deadline (kind of like now). ;)
Created attachment 669854 [details] [diff] [review]
patch rev2
Attachment #669832 - Attachment is obsolete: true
Attachment #669832 - Flags: review?(netzen)
Attachment #669854 - Flags: review?(netzen)
Attachment #669854 - Flags: review?(netzen) → review+

Updated

6 years ago
Keywords: verifyme
QA Contact: jsmith
https://hg.mozilla.org/mozilla-central/rev/f2fae799ff95
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Attachment #669854 - Flags: approval-mozilla-aurora?

Updated

6 years ago
Attachment #669854 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Pushed to mozilla-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc83b62af3ec
status-firefox18: --- → fixed
Verified on 10/12 build.
Status: RESOLVED → VERIFIED
Keywords: verifyme

Updated

6 years ago
Keywords: verifyme

Updated

6 years ago
status-firefox18: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.