Closed Bug 1171852 Opened 7 years ago Closed 7 years ago

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


(Firefox :: Toolbars and Customization, defect)

Not set



Firefox 41
Tracking Status
firefox41 --- fixed


(Reporter: past, Assigned: past)



(1 file, 1 obsolete file)

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

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

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

if (this._getAltUpdateWindow())

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:

It sets app.update.enabled = true, = 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+
Filed bug 1176501 for investigating the cases in comment 6.
I made a dumb typo that I've just fixed.
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.