Last Comment Bug 581319 - Warn people on startup if they run an old build with updates disabled
: Warn people on startup if they run an old build with updates disabled
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Startup & Profiles (show other bugs)
: unspecified
: All All
: -- normal (vote)
: seamonkey2.1b2
Assigned To: Robert Kaiser (not working on stability any more)
:
Mentors:
Depends on:
Blocks: 618687 618688 618734
  Show dependency treegraph
 
Reported: 2010-07-23 04:36 PDT by Robert Kaiser (not working on stability any more)
Modified: 2010-12-17 10:05 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1: add notification for outdated builds (5.34 KB, patch)
2010-12-09 07:40 PST, Robert Kaiser (not working on stability any more)
neil: review+
Details | Diff | Review

Description Robert Kaiser (not working on stability any more) 2010-07-23 04:36:29 PDT
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.
Comment 1 Robert Kaiser (not working on stability any more) 2010-12-09 07:40:14 PST
Created attachment 496494 [details] [diff] [review]
v1: add notification for outdated builds

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).
Comment 2 Robert Kaiser (not working on stability any more) 2010-12-09 07:40:57 PST
Rob, do we have any possibility to check if the user has manually checked for updates recently?
Comment 3 Robert Strong [:rstrong] (use needinfo to contact me) 2010-12-09 15:25:46 PST
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 neil@parkwaycc.co.uk 2010-12-10 04:10:16 PST
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.
Comment 5 Robert Kaiser (not working on stability any more) 2010-12-10 07:28:21 PST
(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 neil@parkwaycc.co.uk 2010-12-10 16:07:19 PST
(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)
Comment 7 Robert Kaiser (not working on stability any more) 2010-12-10 16:31:05 PST
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.
Comment 8 Robert Kaiser (not working on stability any more) 2010-12-11 16:12:04 PST
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. :)
Comment 9 Jens Hatlak (:InvisibleSmiley) 2010-12-12 02:59:04 PST
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 Stefan [:stefanh] 2010-12-12 13:32:33 PST
(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 Jens Hatlak (:InvisibleSmiley) 2010-12-12 13:39:18 PST
Filed bug 618734.
Comment 12 Justin Wood (:Callek) 2010-12-17 00:34:51 PST
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 Axel Hecht [:Pike] 2010-12-17 10:05:02 PST
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.

Note You need to log in before you can comment on or make changes to this bug.