Closed Bug 1642754 Opened 4 months ago Closed 4 months ago

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

Categories

(Toolkit :: Application Update, defect)

defect

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: bytesized, Assigned: mcs)

References

Details

(Whiteboard: [tor 28885] )

Attachments

(1 file)

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.

Severity: -- → S3

S3 particularly because of the megabar issue.

See Also: → 1642404

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.

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.

(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).

Whiteboard: [tor 28885]

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

Assignee: nobody → mcs
Status: NEW → ASSIGNED
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f4a6d407d52f
Update prompts should not depend on how update was initiated r=bytesized
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79
You need to log in before you can comment on or make changes to this bug.