Closed
Bug 657472
Opened 14 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)
Toolkit
Application Update
Tracking
()
VERIFIED
FIXED
mozilla10
People
(Reporter: robert.strong.bugs, Assigned: ttaubert)
References
Details
(Keywords: qawanted)
Attachments
(3 files, 3 obsolete files)
(deleted),
patch
|
robert.strong.bugs
:
review+
johnath
:
approval-mozilla-aurora+
johnath
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details |
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.
Comment 1•14 years ago
|
||
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).
Reporter | ||
Comment 2•14 years ago
|
||
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.
Reporter | ||
Comment 3•14 years ago
|
||
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
Reporter | ||
Comment 4•14 years ago
|
||
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
Comment 5•13 years ago
|
||
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.
Assignee | ||
Comment 6•13 years ago
|
||
This patch changes the time to wait before showing the update prompt to 36 hours.
Attachment #563578 -
Flags: review?(robert.bugzilla)
Reporter | ||
Comment 7•13 years ago
|
||
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.
Reporter | ||
Comment 8•13 years ago
|
||
Just found the email where Lucas agreed to increasing the time to 24 hours.
Assignee | ||
Comment 9•13 years ago
|
||
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)
Reporter | ||
Comment 10•13 years ago
|
||
Comment on attachment 563944 [details] [diff] [review]
patch v2
Thanks!
Attachment #563944 -
Flags: review?(robert.bugzilla) → review+
Comment 11•13 years ago
|
||
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.)
Reporter | ||
Comment 12•13 years ago
|
||
I'm fine with this going to Aurora and Beta and don't think we want this on nightly.
Reporter | ||
Comment 13•13 years ago
|
||
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+
Assignee | ||
Comment 14•13 years ago
|
||
(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)
Reporter | ||
Comment 15•13 years ago
|
||
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-
Assignee | ||
Comment 16•13 years ago
|
||
Attachment #564478 -
Attachment is obsolete: true
Attachment #564777 -
Flags: review?(robert.bugzilla)
Reporter | ||
Comment 17•13 years ago
|
||
Comment on attachment 564777 [details] [diff] [review]
patch v4
Thanks Tim!
Attachment #564777 -
Flags: review?(robert.bugzilla) → review+
Comment 18•13 years ago
|
||
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?
Reporter | ||
Comment 19•13 years ago
|
||
(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 20•13 years ago
|
||
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+
Comment 21•13 years ago
|
||
We'll want extra QA attention on this, adding QA wanted.
Keywords: qawanted
Assignee | ||
Comment 22•13 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/fb18dd9ee477
https://hg.mozilla.org/releases/mozilla-aurora/rev/108f75592a6e
https://hg.mozilla.org/releases/mozilla-beta/rev/71bfe7a76ddd
Status: NEW → ASSIGNED
Whiteboard: [fixed-in-fx-team]
Comment 23•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Target Milestone: --- → mozilla10
Comment 24•13 years ago
|
||
This was put on beta, so shouldn't the target milestone be "mozilla8" ?
status-firefox8:
--- → fixed
status-firefox9:
--- → fixed
tracking-firefox8:
--- → +
tracking-firefox9:
--- → +
Comment 25•13 years ago
|
||
Nope, it landed on central so it is mozilla10. I set the status flags for the branches appropriately.
Comment 26•13 years ago
|
||
Hi guys. How can I test this? The patch will land on beta as well?
thanks
Reporter | ||
Comment 27•13 years ago
|
||
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.
Comment 28•13 years ago
|
||
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
Reporter | ||
Comment 29•13 years ago
|
||
I'm fairly certain that should be enough.
Comment 30•13 years ago
|
||
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.
Comment 31•13 years ago
|
||
Reporter | ||
Comment 32•13 years ago
|
||
I'll test and get back with the results tomorrow.
Reporter | ||
Comment 33•13 years ago
|
||
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).
Comment 34•13 years ago
|
||
Rob, is there a count down timer or can we do some trickery like set our clock ahead 25 hours after the download completes?
Comment 35•13 years ago
|
||
(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?
Comment 36•13 years ago
|
||
(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.
Reporter | ||
Comment 37•13 years ago
|
||
(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.
Reporter | ||
Comment 38•13 years ago
|
||
I've verified that the ui is displayed properly after 24 hours as well.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•