1642754 - Update prompts should not depend on whether an update was initiated manually

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Robin Steuber (she/her) [:bytesized]]

We currently make some checks like this one where we decide whether to show prompts based on whether or not the update was initiated in the background (i.e. whether the update was started by a timer, or manually by the user). The assumption seems to be that if the user initiated the update via the UI, that they probably still have the UI open when the download finishes. I believe that this assumption is faulty for two reasons.

First, updates can now be triggered by the megabar. When updates are triggered this way, no UI is left open.

Second, especially if the user has a slow connection, it's pretty reasonable to suspect that the user may have closed the UI between starting the update download and finishing it.

I think that we could pretty easily make a change where we could suppress that prompt only when the UI is open, rather than based on how the update was initiated. We could do this by checking to see if the Downloader has any listeners.

Comment 1

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

S3 particularly because of the megabar issue.

Comment 2

[Mark Smith [:mcs]]

After thinking about this and experimenting while working on a revised patch for bug 1642404, Kathy Brade and I think Downloader.onStopRequest() should unconditionally send the update-downloaded observer notification. Some justification:

1. By default, the badge or doorhanger will not be displayed right away; its appearance is delayed by them mechanism that consults the app.update.badgeWaitTime and app.update.promptWaitTime preference values.
2. The other notifications that might cause a badge or doorhanger to be displayed on the app menu are made unconditionally, e.g., if an error occurs (update-error) or if an update is available but will not be automatically applied (update-available).
3. If we suppress the observer notification when the UI is open, then if someone opens the about dialog or about:preferences, downloads an update, and then closes the relevant UI without clicking the Restart button they will be left without any indication that the update is pending.

Comment 3

[Molly Howell (she/her) (mostly inactive)]

To be entirely clear, we do need one condition on sending update-downloaded, which is to skip it if staging is enabled (and succeeds), because then we don't want to start any prompting until that's done.

But the reasoning here for removing the background condition does seem right to me. It's occurred to me now that when that condition was written, we had prompts that were very invasive compared to what we have today, and I think that relieves us of the need to treat them so cautiously. I'm no longer even sure that we need to add the condition on whether the UI that initiated the foreground check is still open; it seems like a justifiable condition to have, but not a necessary or even really useful one.

Comment 4

[Mark Smith [:mcs]]

(In reply to Molly Howell (she/her) [:mhowell] from comment #3)

To be entirely clear, we do need one condition on sending update-downloaded, which is to skip it if staging is enabled (and succeeds), because then we don't want to start any prompting until that's done.

Ah, right. We need to preserve the shouldShowPrompt concept. I should have said that the if (this.background) checks can be removed from Downloader.onStopRequest().

But the reasoning here for removing the background condition does seem right to me. It's occurred to me now that when that condition was written, we had prompts that were very invasive compared to what we have today, and I think that relieves us of the need to treat them so cautiously.

Agreed!

I'm no longer even sure that we need to add the condition on whether the UI that initiated the foreground check is still open; it seems like a justifiable condition to have, but not a necessary or even really useful one.

Yes, that is what I am arguing for: replace the check that is made against the Downloader's .background value with... nothing. In fact, we can then remove the background property from Downloader (it is not used for anything else).

Comment 5

[Mark Smith [:mcs]]

Attachment: Bug 1642754 - Update prompts should not depend on how update was initiated r=bytesized

Show update badge and doorhanger when entering the "pending"
state for foreground updates.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4a6d407d52f
Update prompts should not depend on how update was initiated r=bytesized

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/f4a6d407d52f