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

RESOLVED FIXED in seamonkey2.1b2

Status

SeaMonkey
Startup & Profiles
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Robert Kaiser, Assigned: Robert Kaiser)

Tracking

(Blocks: 3 bugs)

unspecified
seamonkey2.1b2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
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

7 years ago
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).
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #496494 - Flags: review?(neil)
(Assignee)

Comment 2

7 years ago
Rob, do we have any possibility to check if the user has manually checked for updates recently?
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

7 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

7 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

7 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

7 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

7 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
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b2
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.
(Assignee)

Updated

7 years ago
Blocks: 618687
(Assignee)

Updated

7 years ago
Blocks: 618688

Comment 10

7 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.
Blocks: 618734
Filed bug 618734.
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

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