Closed Bug 1171852 Opened 6 years ago Closed 6 years ago
Hamburger menu doesn't display the green badge when staging is not possible
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.
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
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?
Attachment #8621511 - Flags: review?(robert.strong.bugs)
Assignee: nobody → past
Status: NEW → ASSIGNED
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?
Attachment #8621511 - Flags: review?(robert.strong.bugs) → review-
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.
Attachment #8621511 - Attachment is obsolete: true
Attachment #8622524 - Flags: review?(robert.strong.bugs)
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.
Attachment #8622524 - Flags: review?(robert.strong.bugs) → review+
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
I made a dumb typo that I've just fixed.
You need to log in before you can comment on or make changes to this bug.