Closed Bug 657472 Opened 13 years ago Closed 13 years ago

'Tune' the time to wait before displaying the update been downloaded / restart notification and provide ability to override in the update xml

Categories

(Toolkit :: Application Update, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla10
Tracking Status
firefox8 + fixed
firefox9 + fixed

People

(Reporter: robert.strong.bugs, Assigned: ttaubert)

References

Details

(Keywords: qawanted)

Attachments

(3 files, 3 obsolete files)

Per Johnath's and Jay's request I discussed with limi what we should do with the notification that there is an update downloaded / ready to install that occurs 12 hours after an update has been downloaded. Per limi, this notification should just be removed.

Though not part of this bug, if we want to make users restart we should create a new method that can be controlled by the update snippet.
Would this block on having that "new method" available? Seems like if we release a security update for a 0-day exploit, we'd want to make sure users update in a timely manner (which, at least in theory, is part of the purpose of this notification).
In discussing this with UX it wouldn't block though I personally think it might. I do hope that we get the design for the new notification system from UX in time for the two to land during the same cycle.
Discussed this with a few people and the consensus is that we should have a new notification system in place. I've discussed this briefly with limi and faaborg so they are aware that we need a UX design for this.

Tim, sorry for the scope creep. For now, a patch to remove the existing notification would help familiarize you with the existing code in app update and specifically the update prompt.
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#2853
Summary: Remove notification that an update has been downloaded... please restart → Replace notification that an update has been downloaded / please restart with new notification system TBD
Per a meeting today we are going to go with the existing notification, 'tune' when the notification is displayed by default after x amount of time as determined by bug 659425, and provide the ability for the update xml to override the amount of time to wait before displaying the notification.
Summary: Replace notification that an update has been downloaded / please restart with new notification system TBD → 'Tune' the time to wait before displaying the update been downloaded / restart notification and provide ability to override in the update xml
Depends on: 659425
We should set this at 24 or 36 hours (I lean towards 36 but if sec team requires 24, I'll go with that) and get it into Beta and Aurora ASAP. We can implement a better system later, but this seems like a big and relatively quick win for 'silent update'. Can you generate patches for trunk, aurora and beta that simply changes the interval from what it is today to 36 hours and after reviews request approval for landing on branches? This really could have made Firefox 7, I suspect, and I don't want us to miss 8 and 9 with the simple pref change.
Attached patch patch v1 (obsolete) — Splinter Review
This patch changes the time to wait before showing the update prompt to 36 hours.
Attachment #563578 - Flags: review?(robert.bugzilla)
Comment on attachment 563578 [details] [diff] [review]
patch v1

iirc the security team wanted to go with 24 hours... I'll double check when I have a minute and post what I find in this bug.

Let's not change the default used in app update and instead just change the preference.

Before this goes in I or someone else needs to review / verify there are no harmful side affects from this change.
Just found the email where Lucas agreed to increasing the time to 24 hours.
Attached patch patch v2 (obsolete) — Splinter Review
This patch changes the value of app.update.promptWaitTime to 24 hrs.
Attachment #563578 - Attachment is obsolete: true
Attachment #563578 - Flags: review?(robert.bugzilla)
Attachment #563944 - Flags: review?(robert.bugzilla)
Comment on attachment 563944 [details] [diff] [review]
patch v2

Thanks!
Attachment #563944 - Flags: review?(robert.bugzilla) → review+
Robert, I'd like to try to rush this into Aurora, and possibly Beta. It appears "really safe", well understood, and a potentially solid win on silencing updates that could come out in advance of the more difficult work. Do you think that's reasonable? (we could go straight to beta and not do this on aurora or trunk because we don't actually mind of those people get pushed to update sooner since builds are more frequent than the interval.)
I'm fine with this going to Aurora and Beta and don't think we want this on nightly.
Comment on attachment 563944 [details] [diff] [review]
patch v2

Tim, since we don't want this everywhere can you move the pref into the individual browser/branding preferences? Removing the approval since this is likely not wanted on trunk.
Attachment #563944 - Flags: review+
Attached patch patch v3 (obsolete) — Splinter Review
(In reply to Robert Strong [:rstrong] (do not email) from comment #13)
> Tim, since we don't want this everywhere can you move the pref into the
> individual browser/branding preferences? Removing the approval since this is
> likely not wanted on trunk.

Yeah, that totally makes sense to not have it in nightly. I added the pref changes to the brandings "aurora" and "official". I don't know exactly what the "unofficial" branding is used for but I figured we might not want to touch that.
Attachment #563944 - Attachment is obsolete: true
Attachment #564478 - Flags: review?(robert.bugzilla)
Comment on attachment 564478 [details] [diff] [review]
patch v3

Looks good but please add it to the following as well:
browser/branding/unofficial/pref/firefox-branding.js

though we don't use unofficial in any of our builds there is talk of making that the default for personal builds and I'd prefer it having the larger value of 24 hours.

The pref needs to be removed from:
browser/app/profile/firefox.js

The original value needs to be added to:
browser/branding/nightly/pref/firefox-branding.js

I *think* it would be a good thing to have a smaller value than 12 hours for nightly but I would prefer for that to be done in another bug if we decide we want to do that.

Should be a simple r+ with the above and thanks!
Attachment #564478 - Flags: review?(robert.bugzilla) → review-
Attached patch patch v4Splinter Review
Attachment #564478 - Attachment is obsolete: true
Attachment #564777 - Flags: review?(robert.bugzilla)
Comment on attachment 564777 [details] [diff] [review]
patch v4

Thanks Tim!
Attachment #564777 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 564777 [details] [diff] [review]
patch v4

We want to get this into Beta [8] so when they're updating from 8 to 9 they have this improved experience. We'll also want to get this into the next Beta [9].

I'm not sure if this is the right time increment for Aurora and I'm sure it's not right for m-c builds.
Attachment #564777 - Flags: approval-mozilla-beta?
(In reply to Asa Dotzler [:asa] from comment #18)
> Comment on attachment 564777 [details] [diff] [review] [diff] [details] [review]
> patch v4
> 
> We want to get this into Beta [8] so when they're updating from 8 to 9 they
> have this improved experience. We'll also want to get this into the next
> Beta [9].
> 
> I'm not sure if this is the right time increment for Aurora and I'm sure
> it's not right for m-c builds.
It is the same as it currently is on m-c and if this is to be changed on m-c it will be done in another bug. If you want to evaluate a different value for aurora it can be done in that bug as well or a different bug. I think the value is fine for aurora but if you prefer we can put it back to 12 hours in this bug.
Comment on attachment 564777 [details] [diff] [review]
patch v4

Discussed with Robert and Asa. Please land it across the board
Attachment #564777 - Flags: approval-mozilla-beta?
Attachment #564777 - Flags: approval-mozilla-beta+
Attachment #564777 - Flags: approval-mozilla-aurora+
We'll want extra QA attention on this, adding QA wanted.
Keywords: qawanted
https://hg.mozilla.org/mozilla-central/rev/fb18dd9ee477
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Keywords: uiwanted
Whiteboard: [fixed-in-fx-team]
This was put on beta, so shouldn't the target milestone be "mozilla8" ?
Nope, it landed on central so it is mozilla10. I set the status flags for the branches appropriately.
Hi guys. How can I test this? The patch will land on beta as well?
thanks
You can download an update, close the about dialog without restarting, and leave Firefox open for 24 hours to see if it brings up the update window asking to restart.
Hi Robert.
I have a question, what if I don't go to Help/About page and simply let the Firefox open, when will the download of the new version start ? In Tools/Options/Advanced is checked "Automatically download and install the update", so I guess it should automatically start

thanks
I'm fairly certain that should be enough.
Attached image Screenshot
I've verified this bug on Aurora 9.0a2 (2011-10-10) and Beta 8.0b3 on WinXP and Ubuntu executing following steps:
1. Install Firefox
2. Launch Firefox on a clean profile
3. Go to Help/About Firefox
4. After the new version is downloaded, simply close the About dialog and leave Firefox running for at least 24 hours.

Expected result:
After 24 hours a notification to restart Firefox in order to apply the update should be displayed

Actual results:
An extension update notification is displayed (please see screenshots attached). No notification to restart Firefox and apply the update occurs.
Attached image Screenshot2
I'll test and get back with the results tomorrow.
The problem is the notification is only applicable to background downloads.

The simplest way to test would be to launch a version of Firefox that has an update available with a new profile and then wait over 24 hours (25 should be enough).
Rob, is there a count down timer or can we do some trickery like set our clock ahead 25 hours after the download completes?
(In reply to Robert Strong [:rstrong] (do not email) from comment #32)
> I'll test and get back with the results tomorrow.

Is there any way to speed this test up? The sooner we know the state of this bug, the better. Can we change the system clock or fast forward the VM 25 hours?
(In reply to Lawrence Mandel [:lmandel] from comment #34)
> Rob, is there a count down timer or can we do some trickery like set our
> clock ahead 25 hours after the download completes?

Whoops my previous comment was obviously redundant. Let me just add that there's a desire to accelerate the testing in case the proposed plan doesn't succeed for some reason (and a re-test is necessary), or we need to re-spin for any regressions found here.
(In reply to Lawrence Mandel [:lmandel] from comment #34)
> Rob, is there a count down timer or can we do some trickery like set our
> clock ahead 25 hours after the download completes?
There is no way to do that that I know of and if there were it would negate the manual test since it wouldn't be the same as waiting 24 hours.

Besides the automated tests already in place I have verified the notification worked properly using a 1 hour notification and I am waiting on the 24 hour period to complete. I am quite certain everything is fine and the main reason I asked for the 24 hour period is that if we are going to perform a manual test it really should test the actual settings used by the client which we have not been good at in the past.
I've verified that the ui is displayed properly after 24 hours as well.
Status: RESOLVED → VERIFIED
Blocks: 706798
You need to log in before you can comment on or make changes to this bug.