Closed Bug 1596258 Opened 5 years ago Closed 5 years ago

[Interventions] The extension needs a way to tell whether the current Firefox version is the latest

Categories

(Firefox :: Address Bar, task, P2)

task
Points:
5

Tracking

()

RESOLVED FIXED
Firefox 73
Iteration:
72.2 - Nov 4 - 17
Tracking Status
firefox72 --- fixed
firefox73 --- fixed

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(1 file)

When the user is on the latest version and they type something that would normally trigger the update intervention, we should instead show the refresh intervention. See: https://docs.google.com/document/d/1pN0y8TkadgeQHF3k-t-sPDWHkfXTyno7tjAQf-IUtU0/edit#

So we need a way to tell whether the current version is the latest version. I'm not sure that's possible, but we should investigate.

We should respect policies for updates here, like DisableAppUpdate.
Also, if there's an update Firefox would already try to update to it, so it wouldn't be useful to send the user to the download page.
This seems more about "we know there's an update, but for some reason we failed to apply it, so we suggest where to find the latest version".
The ideal outcome, imo, would be to check how much time passed between the current version and the next one, if enough time passed it may be worth to notify, I think we do something similar already for old versions.

And indeed, we could just check the build time of the current version, if it's more than N weeks in the past and there's no update to apply, we suggest the download (respecting the will to disable updates).

Feel free to take this btw, I just assigned myself so it's assigned to someone, but I'm not planning on working on it soon.

I'm taking a closer look at this and aboutDialog-appUpdater.js, and it looks like there are a couple of other complications. The update also may be downloading, which isBrowserUpdateReadyToInstall doesn't capture. Also, auto update may be disabled, and in that case updates aren't automatically downloaded in the background.

So the ideal seems something like this:

if isBrowserUpdateReadyToInstall:
  prompt to restart
else if update is downloading:
  ??? show a progress bar?
else if auto update is not enabled:
  prompt to check for update
else:
  trigger an update check
  if there's an error:
    prompt to download
  else if no update available:
    prompt to refresh
  else:
    recurse (but skip the else branch and don't trigger an update check)

I don't think we should actually do this because it's more work than we should put into it since we're talking about an experiment and we want to ship soon, but if this were a feature in Firefox, this seems like what we would want to do.

I think we should do this:

if isBrowserUpdateReadyToInstall:
  prompt to restart
else:
  show a prompt that opens the update section of about:preferences

Or maybe:

if isBrowserUpdateReadyToInstall:
  prompt to restart
else if updates aren't allowed:
  prompt to download
else:
  show a prompt that opens the update section of about:preferences

i.e., we should delegate to the update logic/UI that we already have instead of writing a new UI for it.

What do you think?

btw there doesn't seem to be a way to open about:preferences and automatically show the updates section. That's because the updates section doesn't have the proper subcategory defined. (It needs a data-subcategory attribute.) So if we go with my suggestion, we need to fix that.

I think the question is better suited for Verdi/Connor, because I'm unsure what's our final intent for the update tip.
Do we just want to show a reminder?
Do we want to detect pathologic broken updates cases?
Do we want to regain users on too old versions?
... and so on...

(In reply to Drew Willcoxon :adw from comment #4)

else if update is downloading:
  ??? show a progress bar?

I'd show nothing, we can show a tip once it has been downloaded (next time)

else if auto update is not enabled:
  prompt to check for update

It sounds like going against the user's will, so this really depends on the intent of the tip.

  trigger an update check

This is something I'd avoid, we may be triggering it at the "wrong" time.

if isBrowserUpdateReadyToInstall:
  prompt to restart
else:
  show a prompt that opens the update section of about:preferences

My idea was more like:

if isBrowserUpdateReadyToInstall:
    prompt to restart

i.e., we should delegate to the update logic/UI that we already have instead of writing a new UI for it.

Absolutely, doing any new UI around update sounds like calling for trouble.

I spoke with Verdi today. He didn't like my idea of redirecting to about:preferences. His intention is to show the user their current update status and give them an immediate way to continue on their upgrade path, without going through some other UI.

He really wants us to detect whether the user's version is the latest. So I don't see a way around having to trigger an update check from the extension. The logic for determining the tip to show would be like:

if isReadyForRestart:
  prompt to restart
else if checking for update or downloading/applying:
  show some "please wait" message
else if updates are disabled by policy or there was an error checking:
  prompt to download
else if no update available:
  prompt to refresh
else:
  # in this case, the user is OK with background checks, but they want to
  # download and apply manually
  prompt to download and apply

We might be able to avoid the update check if we make an assumption: If there's no active update, then assume the user is on the latest version. That might be OK for the majority of users, because the default is to check for updates in the background every 12 hours and download and apply them automatically.

Marco, thoughts on that?

Flags: needinfo?(mak)

(In reply to Drew Willcoxon :adw from comment #7)

else if checking for update or downloading/applying:
  show some "please wait" message

I still think this should not show an intervention, it's a middle-state, uninsteresting to the user (what can he do once he knows an update is downloading?) and it's going away in a few seconds/minutes.

else if updates are disabled by policy or there was an error checking:
  prompt to download

I still think this goes against the user's will, and it's even worse if updates are disabled by a policy.

else if no update available:
  prompt to refresh

I don't understand this, we already suggest refreshing when an old profile is started, why should we do it here, it seems more dangerous than useful. At a minimum we should copy over all the refresh logic, that doesn't sound like a goal for this.

So, the user is searching for something related to firefox, like "download firefox" or "update firefox", the intervention wants to help him

  1. if there's already a pending update, ask to restart.
  2. if there's an available update, and it's not ready to be applied, ask if the user wants to download it (if it's in the process of being downloaded, we can cheat).
  3. if the user is on the latest version, tell him so.
    I think anything else is unnecessary, or even dangerous.
Flags: needinfo?(mak)

(In reply to Marco Bonardo [:mak] from comment #9)

else if no update available:
  prompt to refresh

I don't understand this, we already suggest refreshing when an old profile is started, why should we do it here, it seems more dangerous than useful. At a minimum we should copy over all the refresh logic, that doesn't sound like a goal for this.

That's in Verdi's design. I can ask him about it, but I presume it's there for a reason. Maybe he doesn't know about how we already suggest it for old profiles.

  1. if there's already a pending update, ask to restart.
  2. if there's an available update, and it's not ready to be applied, ask if the user wants to download it (if it's in the process of being downloaded, we can cheat).
  3. if the user is on the latest version, tell him so.

That sounds reasonable to me. But afaict -- and please tell me if you know different -- if there is no update downloaded or applied, then the only way to tell whether an update is available is to check for updates. There's no state maintained anywhere I can find that remembers whether an update is available but not downloaded. So the extension would need to be able to trigger a check.

I looked at UpdateService again while writing this comment. It looks like doing an update check is the same thing as actually starting an update. If auto updates are enabled, then UpdateService goes ahead and downloads the update. If auto updates aren't enabled, then it broadcasts update-available, and UpdateListener shows a prompt.

So I'm more confident this is how it should work:

if ready to restart:
  prompt to restart
else if we already triggered an update check and there's no update available:
  tell the user they're on the latest (pending more discussion with Verdi)
else if updates are prohibited:
  prompt to download
else:
  prompt to check for updates

The only thing is that if they check for updates and there isn't one, we should probably show some kind of feedback. I'm not sure that happens automatically. Otherwise they'll have to trigger the intervention again to see the "you're on the latest version" message.

(In reply to Marco Bonardo [:mak] from comment #9)

I don't understand this, we already suggest refreshing when an old profile is started, why should we do it here, it seems more dangerous than useful.

Reinstalling is a very common repair procedure. The problem with that is, of course, that reinstalling Firefox fixes almost zero things. The refresh feature is designed so that people don't have to understand why that is. So the reason to do the intervention here is because it's in the repair flow (as it's generally understood) exactly where the people who need it, can find it. And the message tries to explain that: "Firefox is up to date. Trying to fix a problem? Restore default settings and remove old add-ons for optimal performance. [Refresh Firefox]"

I think anything else is unnecessary, or even dangerous.

To me this sounds like you disagree with the premise or usefulness of the Refresh feature. Obviously, I disagree and we can certainly debate that, but to address dangerous:

  • The intervention explains that things will be lost.
  • The intervention includes a link to the support article with all of the details.
  • The Refresh action is not the default action.
  • Clicking Refresh opens the Refresh dialog that again explains what will be removed.
  • The default action of the Refresh dialog is Cancel.

So without reading anything that's a minimum of two user interactions that are not the default choice in order to invoke refresh.

Marco, Shane, could you please give me feedback on this? Not looking for review, just want to know whether you're opposed to this. We need to be able to check for app updates. There's no 5-line trivial way to do this. We already have a nice class in browser for it, but it's coupled to the About window and in-content preferences: https://searchfox.org/mozilla-central/source/browser/base/content/aboutDialog-appUpdater.js

This patch factors that out into an AppUpdater class. I rewrote the main check method and added a status getter that returns the current status without checking. I also replaced the UI-updating parts with a general listener system, although I don't actually use it here. (We could replace appUpdater in aboutDialog-appUpdater.js with this at some point.)

The extension uses both of the new checkForBrowserUpdate and getBrowserUpdateStatus functions on browser.urlbar. It checks for updates on startup and then gets the status when determining which update tip to show. That latter part is here for reference: https://github.com/0c0w3/urlbar-search-interventions-experiment/blob/2238a1cd355f4ea00ca662e095b8efe7eeb139a4/src/background.js#L141

Try run with the patch for this bug + tests + a browser.experiments.urlbar.installBrowserUpdateAndRestart function that the extension needs (when the user disables auto updates but an update is available):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=99733a2bf046ead6408d969a6fc0e736c9f599eb

Points: 3 → 5

(In reply to Drew Willcoxon :adw from comment #13)

a browser.experiments.urlbar.installBrowserUpdateAndRestart function that the extension needs (when the user disables auto updates but an update is available)

Bug 1600860

Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/20c0ab8e1a20 Add browser.experiments.urlbar.checkForBrowserUpdate and getBrowserUpdateStatus. r=mak,mixedpuppy
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 73

Comment on attachment 9111016 [details]
Bug 1596258 - Add browser.experiments.urlbar.checkForBrowserUpdate and getBrowserUpdateStatus.

Beta/Release Uplift Approval Request

  • User impact if declined: We need this in order to run the urlbar interventions experiment (bug 1564506) on 72 as planned.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a test-only change. There's no change to shipping code.
  • String changes made/needed:
Attachment #9111016 - Flags: approval-mozilla-beta?

Comment on attachment 9111016 [details]
Bug 1596258 - Add browser.experiments.urlbar.checkForBrowserUpdate and getBrowserUpdateStatus.

this can go in a=test-only

Attachment #9111016 - Flags: approval-mozilla-beta?
Whiteboard: [checkin-needed-beta]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: