581319 - Warn people on startup if they run an old build with updates disabled

Status: RESOLVED FIXED

(SeaMonkey :: Startup & Profiles, defect)

Description

[Robert Kaiser]

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]

Attachment: 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]

Rob, do we have any possibility to check if the user has manually checked for updates recently?

Comment 3

[Robert Strong (they/them - no direct email)]

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

[[e-mail]]

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]

(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

[[e-mail]]

(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]

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]

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)]

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]]

(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)]

Filed bug 618734.

Comment 12

[Justin Wood (:Callek)]

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]]

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.