1273545 - [Elevated Update] Loading Panel is displayed too early in the Elevate Update process

Status: NEW

(Toolkit :: Application Update, defect, P3)

Description

[Adrian Florinescu [:aflorinescu]]

Description:
This is a polish issue, related to the loading panel that is displayed during the elevated update. The problem with the elevated update loading panel is that is incorrectly instructing user that FF is updating, when in fact it is not yet. 

In the specific case that the user presses cancel(Step5) in the elevation OSX pop-up, the user will be left with a loading panel that shows Firefox is getting updated, followed with a FF restart + an update failed message.

[Steps To Reproduce]:
1. Install FF (http://archive.mozilla.org/pub/firefox/nightly/2016/05/2016-05-13-20-52-10-oak/firefox-49.0a1.en-US.mac.dmg) on a Admin user.
2. Switch to a standards user.
3. Check for updates / Restart to elevate.
4. The elevate OSX pop-up is shown instructing admin user to input his credentials.
5. Press Cancel.

[Actual Result]:
On Step 4, besides the OSX elevate pop-up a FF loading panel is displayed as well. 
In the case the admin authentication is canceled, the loading panel persists a few moments then is closed as expected.

[Expected Result]:
A loading panel is expected to be displayed starting with the moment after the credentials for admin user are validated: for the "empty" period in which FF is closed and the user is waiting for FF to get updated and to be restarted.

Comment 1

[Stephen A Pohl [:spohl] (OOO until 3/20)]

Robert, I believe we agreed to show the progress UI immediately after restart, even while waiting for the user to enter the admin username/password. Could you confirm? Thanks!

Comment 2

[Robert Strong (they/them - no direct email)]

Ideally it would only be shown after the user enters their credentials but I don't know of a way around this... so, I'm fine with this but you should run it by someone on the UX team.

Comment 3

[Stephen A Pohl [:spohl] (OOO until 3/20)]

Actually, you're right. Because of how we use the native OSX API, we seem to be limited to either showing the progress UI while the native elevation dialog is open or not showing it at all. I was going to ask Philipp for his opinion, but he seems to be out until mid-July. Stephen, would you mind giving us your opinion on this? Thanks!

Comment 4

[Markus Stange [:mstange]]

Doesn't the native dialog come up in response to some restricted API call you're making in the code? You could make that call before you show the window, couldn't you?

Comment 5

[Stephen A Pohl [:spohl] (OOO until 3/20)]

(In reply to Markus Stange [:mstange] from comment #4)
> Doesn't the native dialog come up in response to some restricted API call
> you're making in the code? You could make that call before you show the
> window, couldn't you?

Sorry, I realize my response was overly simplified. After reading the code again, we could display the progress UI after the user successfully enters username/password, but it would require quite a bit of work:

The progress UI is currently launched by the unelevated updater process while it's waiting for the elevated updater process to connect and get the cmdline args that it needs to run the elevated update. So, if we wanted to make this work, we'd have to add an additional IPC call from the elevated updater to the unelevated updater (or change |ObtainUpdaterArguments|, which seems weird) to tell it that elevation was successful and launch the progress UI. This would require that we add all the functionality to initialize and display the progress UI to launchchild_osx.mm (this functionality is currently self-contained in updater.cpp). There may also be an issue of doing this on the correct thread, not the thread that the IPC call comes in.

Showing the progress UI from the elevated updater after elevation was successful doesn't work, because we don't seem to have the ability to access the active window session to display UI from a privileged helper tool.

Comment 6

[Robert Strong (they/them - no direct email)]

I don't think that should delay landing... how about filing a followup bug for it?

Comment 7

[Stephen A Pohl [:spohl] (OOO until 3/20)]

I think this bug is meant to be the followup. :-) I agree that this shouldn't delay landing, but was hoping to get confirmation before doing so. We may be able to land as early as tomorrow.

Comment 8

[Robert Strong (they/them - no direct email)]

doh! ;)