Closed Bug 843497 Opened 7 years ago Closed 7 years ago

Update check says "up to date" for out of date versions on unsupported OS versions

Categories

(Toolkit :: Application Update, defect)

x86
Windows 2000
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- wontfix
firefox24 + fixed
firefox-esr17 --- wontfix
b2g18 --- wontfix

People

(Reporter: hmdmhdfmhdjmzdtjmzdtzktdkztdjz, Assigned: robert.strong.bugs)

References

Details

(Keywords: qawanted, verifyme)

Attachments

(7 files, 6 obsolete files)

25.66 KB, image/png
Details
204.84 KB, image/png
Details
4.25 KB, patch
bbondy
: review+
Details | Diff | Splinter Review
24.30 KB, patch
robert.strong.bugs
: review+
Details | Diff | Splinter Review
9.22 KB, patch
robert.strong.bugs
: review+
Details | Diff | Splinter Review
7.12 KB, patch
bbondy
: review+
Details | Diff | Splinter Review
8.75 KB, patch
robert.strong.bugs
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Windows NT 5.0; rv:10.0.12) Gecko/20100101 Firefox/10.0.12
Build ID: 20130103094221

Steps to reproduce:

Click on help : about firefox (IOW, check for updates) 


Actual results:

Windows 2000: "firefox is up to date"

Windows 7: "new update available" (version number NOT shown, I just happen to know that there was no final 10.0.13)


Expected results:

Windows 2000: "firefox 10.0.12 is insecure and won't be fixed"

Windows 7: "you cannot upgrade to ESR 17 if you use this installation also with Windows 2000"

Cf. last line on <https://wiki.mozilla.org/QA/SoftVision/Meetings/Desktop:2013-02-13#Notes_and_Actions>
We're not going to change these strings in ESR-10 at this point. Hopefully the people using ESR-10 are in an actual Enterprise that manages their releases and can catch these issues.

We're unlikely to get strings changed in ESR-17, but it might be worth trying (we've got 6 months before ESR-17 users hit this case).

We definitely should fix this before the next ESR or the next round of OS de-support.

I don't think it's worth worrying about multiple-OS situations, we can just tell people about the version they're currently using.
Status: UNCONFIRMED → NEW
Component: Untriaged → Application Update
Ever confirmed: true
Product: Firefox → Toolkit
Version: 10 Branch → 17 Branch
Summary: ESR 10.0.12 update observation → Update check says "up to date" for out of date versions on unsupported OS versions
Just in case, "multiple-OS" include the portable.apps.com ESR versions if used with different NT versions (5: W2K, 5.1: XP, 6: Vista, 6.1: 7, if I got that right), and ordinary installations on virtual hard disks mounted in newer NT hosts, or in older NT guests.  

Presumably in two years (ESR 24?) when you drop to support XP clearer messages than for ESR 10 would be nice... ;-)
Gavin - would it be possible to simply change the string to say "No update available", or more complexly allow for a serverside key to choose between "Firefox is up to date" or "Your OS is no longer supported"? We don't have any plans to EOL another OS right now, but I don't want this to fall off our radar for when that day comes.
Note - this would be for mozilla-central and to ride the train, not m-e17
Flags: needinfo?(gavin.sharp)
(In reply to Alex Keybl [:akeybl] from comment #3)
> Gavin - would it be possible to simply change the string to say "No update
> available"

Easy! Though this isn't a great solution because for the majority of users who are on supported operating systems, "you're up to date" is a better message than "no update available".

>, or more complexly allow for a serverside key to choose between
> "Firefox is up to date" or "Your OS is no longer supported"? We don't have
> any plans to EOL another OS right now, but I don't want this to fall off our
> radar for when that day comes.

More complicated, certainly. rstrong may have better ideas for how to do this.
Flags: needinfo?(gavin.sharp)
good point - ni? on rstrong to evaluate the level of effort on the second part
Flags: needinfo?(robert.bugzilla)
Other work has kept me from commenting but I really want to do this one right. It won't be that difficult to add this capability to the client side but changes to AUS will be required as well.
Assignee: nobody → robert.bugzilla
Flags: needinfo?(robert.bugzilla)
Update, after some months of not using it I used my real (no VM) windows 2000 box again, it was still at ESR 10.0.11.  Clicking on "help" "about firefox" I got the same "firefox is up to date" message as on 10.0.12.  Whatever ESR 10.0.11 might be, "up to date" can't be right... ;-)
Rob - any updates or eta here? Not tracking for ESR branch since, as Alex mentions in comment 6, this will land to m-c and then ride the trains into ESR24. Will track this for FF23 to stay in the loop about work underway in preparation for FF24.
Flags: needinfo?(robert.bugzilla)
Not as of yet... other work has priority atm.
Flags: needinfo?(robert.bugzilla)
I talked with Madhava and Matej about the UX and copy for this bug.

cc'ing Matej for the copy.

We'll need strings for both the app update UI notification and the about dialog (note: there is limited space in the about dialog). There will also be a link for additional information.
Can someone recap exactly the information we need to communicate, including if there's anything we don't want to or can't say? Thanks.
The existing about dialog strings are here:
http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/aboutDialog.dtd#46

We essentially need a concise way to convey:
- your Firefox is out of date
- your system (OS, or hardware, or something else) is not supported by the latest Firefox version, and so we can't offer you an update

I think the only limitations are that it must fit within the about dialog, and so there's not much room for more than one line of text (screenshots at attachment 477857 [details]).

Robert: I'm not sure what strings would be needed for the update notification - is the idea that we'd prompt the user using the update prompt that they're out of date but we can't update them? That seems to widen the scope considerably from just "change the about dialog string".
Thanks, Gavin. Robert also mentioned that security may be a part of this, i.e. letting people know that their version of Firefox will be less secure going forward. Is that something mandatory and/or worth considering?
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #13)
> The existing about dialog strings are here:
> http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/
> browser/aboutDialog.dtd#46
> 
> We essentially need a concise way to convey:
> - your Firefox is out of date
> - your system (OS, or hardware, or something else) is not supported by the
> latest Firefox version, and so we can't offer you an update
> 
> I think the only limitations are that it must fit within the about dialog,
> and so there's not much room for more than one line of text (screenshots at
> attachment 477857 [details]).
> 
> Robert: I'm not sure what strings would be needed for the update
> notification - is the idea that we'd prompt the user using the update prompt
> that they're out of date but we can't update them? That seems to widen the
> scope considerably from just "change the about dialog string".
It is the same information except there is significantly more UI real estate to convey this information. In both cases there should also be a link for further information.
cc'ing dveditz

Daniel, can you or can you find someone from the security side to provide input on whether the text for this bug should convey anything about being "less secure", etc.? Thanks!
Will do.
Flags: sec-review?
Flags: needinfo?(dveditz)
Since the update dialog only comes up once in the "unsupported" case is it worth much effort to invent--and then translate--a different (longer, more descriptive) string for that case?

I agree w/Gavin that we should distinguish the "You're up to date" case from the "Firefox is out of date because your OS is no longer supported" case--I dislike the generic "No update available" option. Beyond that distinction we don't need to add any scary security warnings. I /would/ prefer the message to include some sense of Firefox specifically being out of date and not just be a statement about their machine: I don't think "Your OS is no longer supported" is quite enough. "Your OS is no longer secure, please upgrade" perhaps? "The latest version of Firefox is not available for your OS"?

If someone can think of a word or two that fits and is more explicit about missing security fixes that would be great, but it's not a requirement. "The latest security fixes are not available for your OS"?
Flags: needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #18)
> Since the update dialog only comes up once in the "unsupported" case is it
> worth much effort to invent--and then translate--a different (longer, more
> descriptive) string for that case?
I think it is. The about dialog is not a notification and surely some people never bother to open it whereas the update dialog is a notification and all users will see it.

When I discussed this with UX it was decided that this message should only be shown once. With that in mind it should probably let the user know that the message would only be shown once.
If you have the space and you believe users do read it then it would be great to word the text to let them know that being unsupported means they are at risk from known security bugs. But that kind of sucks for them because they're probably on an old OS because they don't have the resources to get a new computer so there's not much they can do in practice.

Letting them know it's a one-time notice is a fantastic idea, because people these days are trained that they can get this type of dialog out of their face quickly and do something about it "next time/eventually".
Flags: sec-review? → sec-review?(dveditz)
Matej, is there anything else you need to create the text? Thanks!
Flags: needinfo?(Mnovak)
Hi Robert. I need to take a closer look at this again, but I likely won't be able to do that until the early part of next week. At a quick glance, however, I think I have what I need to proceed. Thanks.
Flags: needinfo?(Mnovak)
The about dialog is definitely trickier because of the limited space, but here are several options. Please let me know if they don't fit and I can try to make them even shorter:

No updates available for your system. Learn more.

No more updates are available for your system. Learn more.

No more updates will be available for your system. Learn more.

The latest version is not available for your system. Learn more.

You can not perform further updates on this system. Learn more.

Your system is not secure and no longer supported. Learn more.


And here's what I was thinking for the update notification:

Your Firefox is out of date, but the latest version is not available for your system. Please upgrade to keep your system secure, then try again. You will not see this notice again, but you can <a>learn more.</a>
(In reply to Matej Novak [:matej] from comment #23)
> The about dialog is definitely trickier because of the limited space, but
> here are several options. Please let me know if they don't fit and I can try
> to make them even shorter:
I'll try to check whether they fit tomorrow.

> No updates available for your system. Learn more.
> 
> No more updates are available for your system. Learn more.
> 
> No more updates will be available for your system. Learn more.
> 
> The latest version is not available for your system. Learn more.
> 
> You can not perform further updates on this system. Learn more.
> 
> Your system is not secure and no longer supported. Learn more.
I'm very hesitant in stating that a system is not secure. For example, there are Firewalls available on Windows that can secure the hell out of it.

> 
> 
> And here's what I was thinking for the update notification:
> 
> Your Firefox is out of date, but the latest version is not available for
> your system.
What do you think of:
Your Firefox is out of date, but the latest version is not supported on your system.

> Please upgrade to keep your system secure, then try again. You
> will not see this notice again, but you can <a>learn more.</a>
I'd like to have upgrade closely attached to system since some people might confuse it with "upgrade your Firefox". Perhaps something like:
"Please upgrade your system, then try again."
or similar.

BTW: I'm not pushing hard for the suggested changes to the strings.
Both those changes look good to me:

Your Firefox is out of date, but the latest version is not supported on your system. Please upgrade your system, then try again. You will not see this notice again, but you can <a>learn more.</a>
Attachment #765799 - Flags: ui-review?(Mnovak)
Attached image About Dialog screenshot
Attachment #765812 - Flags: ui-review?(Mnovak)
Attached patch 1. main patch rev1 (obsolete) — Splinter Review
Attachment #765824 - Flags: review?(netzen)
Attached patch 2. browser patchSplinter Review
Attachment #765826 - Flags: review?(netzen)
Attached patch 3. tests rev1 (obsolete) — Splinter Review
Attachment #765828 - Flags: review?(netzen)
Hey Brian, a couple of review notes:

I reduced the minimum interval for the timer manager from 60 seconds to 30 seconds. This is to facilitate checking for updates early during startup.

Tests for the about dialog are non-existent. The only reasonable way to do this that I can see is via an app update widget that I haven't had time to write. Bug 599574

One test that I plan to write is not notifying a second time. I'll get to that as soon as I am able.

I'll file an AUS bug for support of this feature after the reviews are done.

Thanks!
Attached patch 1. main patch rev2 (obsolete) — Splinter Review
Fixed a bug caused by a last minute change... the test caught it at least
Attachment #765824 - Attachment is obsolete: true
Attachment #765824 - Flags: review?(netzen)
Attachment #765849 - Flags: review?(netzen)
Comment on attachment 765849 [details] [diff] [review]
1. main patch rev2

Review of attachment 765849 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r+'ing given that I'm sure the question below is just due to my not understanding everything.

::: toolkit/mozapps/update/nsUpdateService.js
@@ +2379,5 @@
>      if (updates.length == 0)
>        return null;
>  
> +    if (updates.length == 1 && updates[0].unsupported)
> +      return updates[0];

I don't fully understand this change, could you explain? Thanks!

@@ +2474,5 @@
> +    if (!update) {
> +      return;
> +    }
> +
> +    if (update.unsupported) {    

nit: Trailing whitespace
Attachment #765849 - Flags: review?(netzen) → review+
Blocks: 885721
Comment on attachment 765826 [details] [diff] [review]
2. browser patch

Review of attachment 765826 [details] [diff] [review]:
-----------------------------------------------------------------

Posted bug 885721 to sync this up to Metro.
Attachment #765826 - Flags: review?(netzen) → review+
Attachment #765828 - Flags: review?(netzen) → review+
(In reply to Brian R. Bondy [:bbondy] from comment #33)
> Comment on attachment 765849 [details] [diff] [review]
> 1. main patch rev2
> 
> Review of attachment 765849 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, r+'ing given that I'm sure the question below is just due to my
> not understanding everything.
> 
> ::: toolkit/mozapps/update/nsUpdateService.js
> @@ +2379,5 @@
> >      if (updates.length == 0)
> >        return null;
> >  
> > +    if (updates.length == 1 && updates[0].unsupported)
> > +      return updates[0];
> 
> I don't fully understand this change, could you explain? Thanks!
This skips the version check for update which will cause the update to be incompatible and returns the first update if there is only 1 update and it has unsupported="true" in the update.xml.

> 
> @@ +2474,5 @@
> > +    if (!update) {
> > +      return;
> > +    }
> > +
> > +    if (update.unsupported) {    
> 
> nit: Trailing whitespace
fixed
standard8 and Callek, this will be landing before the next merge and you might want to port this to Thunderbird and Seamonkey if you want to support notifying the user that their system is no longer supported and you have your own update check code
Attached patch 1. main patch rev3 (obsolete) — Splinter Review
Adds the following so the pref is cleared if an update becomes available

diff --git a/toolkit/mozapps/update/content/updates.js b/toolkit/mozapps/update/content/updates.js
--- a/toolkit/mozapps/update/content/updates.js
+++ b/toolkit/mozapps/update/content/updates.js
@@ -595,16 +595,21 @@ var gCheckingPage = {
     // notifications will never happen.
     Services.prefs.deleteBranch(PREF_APP_UPDATE_NEVER_BRANCH);
 
     // The user will be notified if there is an error so clear the background
     // check error count.
     if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_BACKGROUNDERRORS))
       Services.prefs.clearUserPref(PREF_APP_UPDATE_BACKGROUNDERRORS);
 
+    // The preference will be set back to true if the system is still
+    // unsupported.
+    if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_NOTIFIEDUNSUPPORTED))
+      Services.prefs.clearUserPref(PREF_APP_UPDATE_NOTIFIEDUNSUPPORTED);
+
     this._checker = CoC["@mozilla.org/updates/update-checker;1"].
                     createInstance(CoI.nsIUpdateChecker);
     this._checker.checkForUpdates(this.updateListener, true);
   },
Attachment #765849 - Attachment is obsolete: true
Attachment #766054 - Flags: review?(netzen)
Attachment #765812 - Flags: ui-review?(Mnovak)
Comment on attachment 765799 [details]
App update notification window screenshot

Cancelling ui-review... any changes to the UI can happen in the next cycle.
Attachment #765799 - Flags: ui-review?(Mnovak)
Attachment #766054 - Flags: review?(netzen) → review+
Posted the code bug forgot to referesh before resubmitting
Attachment #766054 - Attachment is obsolete: true
Attachment #766058 - Flags: review+
Attached patch 3. tests rev2 (obsolete) — Splinter Review
minor updates to tests
Attachment #765828 - Attachment is obsolete: true
Attachment #766059 - Flags: review+
Attached patch 3. tests rev3Splinter Review
Missed a chunk
Attachment #766059 - Attachment is obsolete: true
Attachment #766062 - Flags: review+
New xpcshell test for checking that the update doesn't open the UI if the notification has already been displayed.
Attachment #766195 - Flags: review?(netzen)
Requesting tracking Firefox 24. The patches required new strings and a change to an interface so this should ride the trains. I don't think this should be tracked for Firefox 23 due to the above which shouldn't be a problem since the next esr release should be based off of Firefox 24 per
http://www.mozilla.org/en-US/firefox/organizations/faq/
Attachment #766195 - Flags: review?(netzen) → review+
Additional test patch pushed to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c8506b0729d
Flags: in-testsuite+
Target Milestone: --- → mozilla24
Version: 17 Branch → Trunk
Backed out (in a fumbling way since I didn't notice the second push in time) in https://hg.mozilla.org/integration/mozilla-inbound/rev/089f6bad69de and https://hg.mozilla.org/integration/mozilla-inbound/rev/0fd721c4a35f for xpcshell failures in test_0020_general.js and test_0060_manager.js like https://tbpl.mozilla.org/php/getParsedLog.php?id=24451299&tree=Mozilla-Inbound
Attached patch xpcshell test bustage fix (obsolete) — Splinter Review
Missed removing the check in xpcshell tests for the removed nsIUpdate property
Attachment #766216 - Flags: review+
Sending to try (like I should have in the first place :( )
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c8506b0729d
(In reply to Robert Strong [:rstrong] (do not email) from comment #48)
> Sending to try (like I should have in the first place :( )
> https://hg.mozilla.org/integration/mozilla-inbound/rev/7c8506b0729d
meh... I'll get it right yet!
https://tbpl.mozilla.org/?tree=Try&rev=522be8a66c89
Missed a couple of other cases where the showSurvey property was checked. New try build looks good. The mochitest-browser-chrome failures are unrelated
https://tbpl.mozilla.org/?tree=Try&rev=2d439874e17a
Blocks: 886454
Drivers, since there are string and interface changes I set the tracking flags to wontfix for version prior to Firefox 24.
Duplicate of this bug: 684227
I was trying to verify this bug and tested two cases:

Case 1: Update to Firefox 24
For instance, if I install Firefox 10 RC on Windows 2000 and I open that version in Windows 7, the update is done to Firefox 23 (same thing happened with a supported Nightly version installed on Windows 2000 and opened in Windows 7 - the update was done all the way to Nightly 26). If I install Firefox 10 on Windows 7 and I open it in Windows 2000 I get the message that Firefox is up to date(same results if I install Nightly 10.0a1 on Windows 7 and I open it in Win 2000).

Is this a case that should have been fixed?

Case 2: Update from Firefox 24
Firefox 24.0 can't be opened on Win 2k because is unsupported. If installed on Windows 7, you still can't open it on Windows 2000 for the same reason. There are no OSs on which versions newer than 24 are not supported. Considering this, is there any way to test this fix?
Flags: needinfo?(robert.bugzilla)
(In reply to Simona B [QA] from comment #56)
This requires work on the server side, see bug 886543
Flags: needinfo?(robert.bugzilla)
Duplicate of this bug: 393782
rs, thanks for fixing this. This was important for security.
Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/a1f7f656e5b34ce081a2e9fdcd7cce9d4b9f176b
Remove support for showSurvey, which bug 843497 also did on the clientside
Flags: sec-review?(dveditz)
You need to log in before you can comment on or make changes to this bug.