1171852 - Hamburger menu doesn't display the green badge when staging is not possible

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

From bug 1170240 comment 5:

"[The code] doesn't appear to handle the case where staging is not possible or staging is disabled via pref. Try setting app.update.staging.enabled to false to disable it and you will get the same result as when staging is not possible."

I tried it and I don't see the badge in that case, although the about dialog displays the button to restart and update. I'm trying to figure out which notification is fired in that case.

Comment 1

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

There isn't an observer notification currently though I would be fine with adding one. The call to UpdatePrompt showUpdateDownloaded is what this should tie into.
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#4570

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Display the update badge on the hamburger menu when staging is not possible

This patch seems to work for me: with staging disabled I get an update badge if I (manually) trigger showUpdateDownloaded(). Is this what you had in mind Rob?

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e3bb1abc801

Comment 4

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

Comment on attachment 8621511 [details] [diff] [review]
Display the update badge on the hamburger menu when staging is not possible

To simplify things I think you can remove the following since staging also calls showUpdateDownloaded
Services.obs.addObserver(this, "update-staged", false);

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#3425

and move the following to before the call to this._getAltUpdateWindow()

if (background && getPref("getBoolPref", PREF_APP_UPDATE_SILENT, false))
  return;
}
Services.obs.notifyObservers(null, "update-downloaded", null);

if (this._getAltUpdateWindow())
  return;


Is there also a notification when the user has set the pref to "Check for updates, but let me choose whether to install them"? Does the notification also handle errors with the check or the download?

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Display the update badge on the hamburger menu when staging is not possible v2

I couldn't remove the update-staged observer, because we need it for the failure case (update.state == "failed"), where execution doesn't enter showUpdateDownloaded. I managed to simplify the code a little more however with your suggestions and fix a few minor errors that I spotted in the process.

About the "Check for updates, but let me choose whether to install them" option, I don't see any notifications firing when the user toggles it, just setting a couple of prefs:

https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content/advanced.js#732

It sets app.update.enabled = true, app.update.auto = false.

Comment 6

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

Comment on attachment 8622524 [details] [diff] [review]
Display the update badge on the hamburger menu when staging is not possible v2

This is an improvement so I'm ok with this but I am fairly certain this won't handle all error cases it should handle (non staging download failures for starters). I won't have time in the for at least several weeks to investigate so let's go with this and file a followup.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/391241169bec

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

Filed bug 1176501 for investigating the cases in comment 6.

Comment 9

[Mark Banner (:standard8)]

I had to back this out due to test-failures:

https://treeherder.mozilla.org/logviewer.html#?job_id=3536763&repo=fx-team
https://hg.mozilla.org/integration/fx-team/rev/e67d1044c439

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

I made a dumb typo that I've just fixed.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8800b4f3ccbc

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8800b4f3ccbc