Closed
Bug 581319
Opened 14 years ago
Closed 14 years ago
Warn people on startup if they run an old build with updates disabled
Categories
(SeaMonkey :: Startup & Profiles, defect)
SeaMonkey
Startup & Profiles
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.1b2
People
(Reporter: kairo, Assigned: kairo)
References
(Blocks 3 open bugs)
Details
Attachments
(1 file)
(deleted),
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
Some people think they are being kept safe by turning off automatic updates, esp. as some people think our habit of silent security updates affects their privacy.
We know that we are usually issuing updates every 4-6 weeks, so when a build is old than 2 months or so and the user has updates disabled, we should bring up a warning on startup that the user should check for updates, IMHO.
The timer could also be for x days after the last update check, as we might keep that as a pref anyhow.
Assignee | ||
Comment 1•14 years ago
|
||
This patch tries to implement what I pointed out in comment #0. Unfortunately, we don't seem to leave any trace when checking manually, so we can't check if the user has done that in the interval.
I added yet another pref though that can be used both to test this (by setting it to false) or to permanently turn it off (Linux distributors may want to do that).
Assignee | ||
Comment 2•14 years ago
|
||
Rob, do we have any possibility to check if the user has manually checked for updates recently?
Comment 3•14 years ago
|
||
There is nothing in app update that keeps track of manual updates. Since the menuitem for checking for updates is provided by each application you could just keep track of when the code associated with the menuitem was last executed.
Comment 4•14 years ago
|
||
Comment on attachment 496494 [details] [diff] [review]
v1: add notification for outdated builds
> if (this._isPlacesDatabaseLocked) {
> this._showPlacesLockedNotificationBox(aWindow);
> }
>+ // Detect if updates are off and warn for outdated builds.
>+ if (this._shouldShowUpdateWarning())
>+ this._showUpdateWarning(aWindow);
[Remind me, am I supposed to be converting these to doorhangers at some point?]
>+ var maxAge = 90 * 24 * 3600;
[Interesting that you should choose to use seconds in an hour!]
>+ var now = Math.round(Date.now() / 1000);
Would it be easier to multiply the other times by 1000?
>+ if (lastUpdateTime + maxAge > now)
[Very minor nit: >= would be slightly more consistent with <]
>+ var buildDate = new Date(buildID.substr(0,4),
>+ buildID.substr(4,2)-1,
>+ buildID.substr(6,2));
Nit: spacing around operators etc.
>+ var notifyBox = aSubject.gBrowser.getNotificationBox();
Nit: getBrowser()
>+updatePrompt.text=Your copy of %S is old and probably has known security flaws, but you have disabled automated update checks. Please update to a newer version.
Should we include the age ("over 90 days old") in the message? I guess if you implement a pref for last manual update check you might want to rewrite the message anyway.
Attachment #496494 -
Flags: review?(neil) → review+
Assignee | ||
Comment 5•14 years ago
|
||
(In reply to comment #4)
> >+ // Detect if updates are off and warn for outdated builds.
> >+ if (this._shouldShowUpdateWarning())
> >+ this._showUpdateWarning(aWindow);
> [Remind me, am I supposed to be converting these to doorhangers at some point?]
Gavin told me that doorhangers are supposed to be bound to a tab (web page) and application-global notifications are not supposed to be doorhangers. I still think that notification bars are at least not better for those, but it's the only UI we have for this atm.
> >+ var maxAge = 90 * 24 * 3600;
> [Interesting that you should choose to use seconds in an hour!]
I thought it's the easiest variant to read. I could use 90 * 86400 if you like that better.
> >+ var now = Math.round(Date.now() / 1000);
> Would it be easier to multiply the other times by 1000?
I wondered about that, but there's always some calculation left to do, so I settled for what the pref uses.
> >+ if (lastUpdateTime + maxAge > now)
> [Very minor nit: >= would be slightly more consistent with <]
As we count is seconds, I guess it doesn't matter much ;-)
> >+ var notifyBox = aSubject.gBrowser.getNotificationBox();
> Nit: getBrowser()
Hrm, I really dislike getBrowser() but as you like.
> >+updatePrompt.text=Your copy of %S is old and probably has known security flaws, but you have disabled automated update checks. Please update to a newer version.
> Should we include the age ("over 90 days old") in the message? I guess if you
> implement a pref for last manual update check you might want to rewrite the
> message anyway.
IMHO, the message is already too long as it is, so even adding that "over 90 days" doesn't sound good to me, really.
Comment 6•14 years ago
|
||
(In reply to comment #5)
>(In reply to comment #4)
>>>+ // Detect if updates are off and warn for outdated builds.
>>>+ if (this._shouldShowUpdateWarning())
>>>+ this._showUpdateWarning(aWindow);
>>[Remind me, am I supposed to be converting these to doorhangers at some point?]
>Gavin told me that doorhangers are supposed to be bound to a tab (web page) and
>application-global notifications are not supposed to be doorhangers. I still
>think that notification bars are at least not better for those, but it's the
>only UI we have for this atm.
Odd. I wonder why I bothered to move showRightsNotification into notification.xml (although given that the bundles already exist there it reduces the amount of code needed.)
>>>+ var maxAge = 90 * 24 * 3600;
>>[Interesting that you should choose to use seconds in an hour!]
>I thought it's the easiest variant to read. I could use 90 * 86400 if you like
>that better.
24 * 60 * 60 seems to be pretty popular (46 hits in m-c; 3 for 24 * 3600)
Assignee | ||
Comment 7•14 years ago
|
||
Going for 90 * 86400 per IRC, just as a note. Fixed all other nits locally, expect the divide by 1000 vs. multiply by 1000 thing. Will check in tomorrow.
Assignee | ||
Comment 8•14 years ago
|
||
Pushed as http://hg.mozilla.org/comm-central/rev/609b77dd2e8a
I'll file a followup for some way to quit it when manually checking for updates, and I think I should file a bug for doing this on Firefox as well. :)
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b2
Comment 9•14 years ago
|
||
Stefan, should we open a Help follow-up for this? Probably just a sentence or two in case people wonder; maybe on the Software Installation prefpanel page.
Comment 10•14 years ago
|
||
(In reply to comment #9)
> Stefan, should we open a Help follow-up for this? Probably just a sentence or
> two in case people wonder; maybe on the Software Installation prefpanel page.
Sound like a good idea to me.
Comment 11•14 years ago
|
||
Filed bug 618734.
Comment 12•14 years ago
|
||
Pike, (and KaiRo) this is such a useful addition in my mind, I think we should allow this on stable branch even with string changes.
Thoughts on what we should/need to do to achieve that reasonably?
Comment 13•14 years ago
|
||
You would basically need to do the same thing that lorentz did, i.e., hard-code a back-up string for each localizable string into the code while backporting to branch.
Best is to have the strings in a new file, and to then make filter.py "report" those strings instead of making them missing. http://hg.mozilla.org/releases/mozilla-1.9.2/file/default/browser/locales/filter.py#l27 is the example.
You need to log in
before you can comment on or make changes to this bug.
Description
•