Closed Bug 1305453 Opened 8 years ago Closed 8 years ago

Prevent non-esr stand alone installers from installing on XP/Vista (Fx53 and up)

Categories

(Firefox :: Installer, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 53
Tracking Status
firefox53 --- verified

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(4 files, 5 obsolete files)

We plan to eol XP/Vista by first moving those users out to ESR 52. Once 52 merges to aurora, we should land changes to the stand alone installer to prevent install by XP and Vista users. Initially there shouldn't be an issue with running but eventually we'll import a system dependency that will break browser startup.
Note that the above plan is tentative until fully approved by our leadership team.
Globally the firefox-eol in windows xp will not affect many users(only 5.45%)

http://gs.statcounter.com/#desktop-os-ww-monthly-201509-201609

but in a large market as China will affect 23%

http://gs.statcounter.com/#desktop-os-CN-monthly-201509-201609
(In reply to ideato from comment #2)
> Globally the firefox-eol in windows xp will not affect many users(only 5.45%)
> 
> http://gs.statcounter.com/#desktop-os-ww-monthly-201509-201609
> 
> but in a large market as China will affect 23%
> 
> http://gs.statcounter.com/#desktop-os-CN-monthly-201509-201609

If this issue is really affect so many PR China users, I think maybe you can ask Beijing Mozilla Online Ltd. (https://www.firefox.com.cn) for help.
(In reply to Krasnaya Ploshchad from comment #3)
> (In reply to ideato from comment #2)
> > Globally the firefox-eol in windows xp will not affect many users(only 5.45%)
> > 
> > http://gs.statcounter.com/#desktop-os-ww-monthly-201509-201609
> > 
> > but in a large market as China will affect 23%
> > 
> > http://gs.statcounter.com/#desktop-os-CN-monthly-201509-201609
> 
> If this issue is really affect so many PR China users, I think maybe you can
> ask Beijing Mozilla Online Ltd. (https://www.firefox.com.cn) for help.

On this page you can find the way to contact them: http://www.firefox.com.cn/about/contact/
(In reply to Krasnaya Ploshchad from comment #3)
> (In reply to ideato from comment #2)
> > Globally the firefox-eol in windows xp will not affect many users(only 5.45%)
> > 
> > http://gs.statcounter.com/#desktop-os-ww-monthly-201509-201609
> > 
> > but in a large market as China will affect 23%
> > 
> > http://gs.statcounter.com/#desktop-os-CN-monthly-201509-201609
> 
> If this issue is really affect so many PR China users, I think maybe you can
> ask Beijing Mozilla Online Ltd. (https://www.firefox.com.cn) for help.

Ιt is not about help, it's just that we will lose many users, or those users will continue to use a not-secure browser anymore.
See Also: → 1315327
Attached patch patch (obsolete) — Splinter Review
This takes care of the installer checks for Vista (Server 2008) or lower. We'll land this in 53.
Assignee: nobody → jmathies
Attachment #8810966 - Flags: review?(robert.strong.bugs)
Comment on attachment 8810966 [details] [diff] [review]
patch

Clearing review since there are a few obvious issues that I went over with Jim on irc.

Jim, please file a release engineering bug to no longer update systems less than Win 7 and to send the unsupported update xml for these clients.
Attachment #8810966 - Flags: review?(robert.strong.bugs)
Attached patch patch (obsolete) — Splinter Review
Attachment #8810966 - Attachment is obsolete: true
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #7)
> Jim, please file a release engineering bug to no longer update systems less
> than Win 7 and to send the unsupported update xml for these clients.

The releng side of moving xp/vista out to ESR is already on file under bug 1303827.
Attachment #8810991 - Flags: review?(robert.strong.bugs)
(In reply to Jim Mathies [:jimm] from comment #9)
> (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from
> comment #7)
> > Jim, please file a release engineering bug to no longer update systems less
> > than Win 7 and to send the unsupported update xml for these clients.
> 
> The releng side of moving xp/vista out to ESR is already on file under bug
> 1303827.
As long as that covers nightly, etc. that is fine. If it doesn't then we'll be busting nightly for xp users at some point.
Comment on attachment 8810991 [details] [diff] [review]
patch

Note, I've updated the common os checks here, which I guess will impact thunberbird and other non-browser installers. I'll look for a why to use different checks. The simplest might be to just skip calling InstallOnInitCommon from onInit.
Attachment #8810991 - Flags: review?(robert.strong.bugs)
Comment on attachment 8810991 [details] [diff] [review]
patch

resetting, tb installs share our os checks since we share the same codebase.
Attachment #8810991 - Flags: review?(robert.strong.bugs)
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #10)
> (In reply to Jim Mathies [:jimm] from comment #9)
> > (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from
> > comment #7)
> > > Jim, please file a release engineering bug to no longer update systems less
> > > than Win 7 and to send the unsupported update xml for these clients.
> > 
> > The releng side of moving xp/vista out to ESR is already on file under bug
> > 1303827.
> As long as that covers nightly, etc. that is fine. If it doesn't then we'll
> be busting nightly for xp users at some point.
For reference, bug 1317780
Comment on attachment 8810991 [details] [diff] [review]
patch

>diff --git a/browser/installer/windows/nsis/installer.nsi b/browser/installer/windows/nsis/installer.nsi
>--- a/browser/installer/windows/nsis/installer.nsi
>+++ b/browser/installer/windows/nsis/installer.nsi
>@@ -1133,88 +1133,53 @@ Function .onInit
>   System::Call 'kernel32::SetDllDirectoryW(w "")'
> 
>   StrCpy $PageName ""
>   StrCpy $LANGUAGE 0
>   ${SetBrandNameVars} "$EXEDIR\core\distribution\setup.ini"
> 
>   ; Don't install on systems that don't support SSE2. The parameter value of
>   ; 10 is for PF_XMMI64_INSTRUCTIONS_AVAILABLE which will check whether the
>-  ; SSE2 instruction set is available.
>+  ; SSE2 instruction set is available. Result returned in $R7.
>   System::Call "kernel32::IsProcessorFeaturePresent(i 10)i .R7"
> 
>-!ifdef HAVE_64BIT_BUILD
>-  ; Restrict x64 builds from being installed on x86 and pre Win7
>-  ${Unless} ${RunningX64}
>-  ${OrUnless} ${AtLeastWin7}
>+  ; Operating system checks: NT 6.0 (Vista/Server 2008) or lower no
>+  ; longer supported.
Just make this along the lines of:
Windows NT 6.0 (Vista/Server 2008) and lower are not supported.

>+  ${If} ${AtMostWin2008}
>     ${If} "$R7" == "0"
>       strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_CPU_MSG)"
>     ${Else}
>       strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_MSG)"
>     ${EndIf}
>     MessageBox MB_OKCANCEL|MB_ICONSTOP "$R7" IDCANCEL +2
>     ExecShell "open" "${URLSystemRequirements}"
>     Quit
>-  ${EndUnless}
>-
>-  SetRegView 64
>-!else
>-  StrCpy $R8 "0"
>-  ${If} ${AtMostWin2000}
>-    StrCpy $R8 "1"
>   ${EndIf}
> 
>-  ${If} ${IsWinXP}
>-  ${AndIf} ${AtMostServicePack} 1
>-    StrCpy $R8 "1"
>-  ${EndIf}
>-
>-  ${If} $R8 == "1"
>-    ; XXX-rstrong - some systems failed the AtLeastWin2000 test that we
>-    ; used to use for an unknown reason and likely fail the AtMostWin2000
>-    ; and possibly the IsWinXP test as well. To work around this also
>-    ; check if the Windows NT registry Key exists and if it does if the
>-    ; first char in CurrentVersion is equal to 3 (Windows NT 3.5 and
>-    ; 3.5.1), 4 (Windows NT 4), or 5 (Windows 2000 and Windows XP).
>-    StrCpy $R8 ""
>-    ClearErrors
>-    ReadRegStr $R8 HKLM "SOFTWARE\Microsoft\Windows NT\CurrentVersion" "CurrentVersion"
>-    StrCpy $R8 "$R8" 1
>-    ${If} ${Errors}
>-    ${OrIf} "$R8" == "3"
>-    ${OrIf} "$R8" == "4"
>-    ${OrIf} "$R8" == "5"
>-      ${If} "$R7" == "0"
>-        strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_CPU_MSG)"
>-      ${Else}
>-        strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_MSG)"
>-      ${EndIf}
>-      MessageBox MB_OKCANCEL|MB_ICONSTOP "$R7" IDCANCEL +2
>-      ExecShell "open" "${URLSystemRequirements}"
>-      Quit
>-    ${EndIf}
>-  ${EndUnless}
>-!endif
>-
>+  ; SSE CPU support
It is actually SSE2

>   ${If} "$R7" == "0"
>     MessageBox MB_OKCANCEL|MB_ICONSTOP "$(WARN_MIN_SUPPORTED_CPU_MSG)" IDCANCEL +2
>     ExecShell "open" "${URLSystemRequirements}"
>     Quit
>   ${EndIf}
> 
>   ${InstallOnInitCommon} "$(WARN_MIN_SUPPORTED_OSVER_CPU_MSG)"
> 
> ; The commands inside this ifndef are needed prior to NSIS 3.0a2 and can be
> ; removed after we require NSIS 3.0a2 or greater.
> !ifndef NSIS_PACKEDVERSION
>   ${If} ${AtLeastWinVista}
>     System::Call 'user32::SetProcessDPIAware()'
>   ${EndIf}
> !endif
> 
>+!ifdef HAVE_64BIT_BUILD
>+  SetRegView 64
>+!endif
Move this to just before ${InstallOnInitCommon} "$(WARN_MIN_SUPPORTED_OSVER_CPU_MSG)"

>diff --git a/browser/installer/windows/nsis/stub.nsi b/browser/installer/windows/nsis/stub.nsi
>--- a/browser/installer/windows/nsis/stub.nsi
>+++ b/browser/installer/windows/nsis/stub.nsi
>@@ -316,69 +316,30 @@ Function .onInit
>   ; isn't supported for the stub installer.
>   ${SetBrandNameVars} "$PLUGINSDIR\ignored.ini"
> 
>   ; Don't install on systems that don't support SSE2. The parameter value of
>   ; 10 is for PF_XMMI64_INSTRUCTIONS_AVAILABLE which will check whether the
>   ; SSE2 instruction set is available.
>   System::Call "kernel32::IsProcessorFeaturePresent(i 10)i .R7"
> 
>-!ifdef HAVE_64BIT_BUILD
>-  ; Restrict x64 builds from being installed on x86 and pre Win7
>-  ${Unless} ${RunningX64}
>-  ${OrUnless} ${AtLeastWin7}
>+  ; Operating system checks: NT 6.0 (Vista/Server 2008) or lower no
>+  ; longer supported.
Just make this along the lines of:
Windows NT 6.0 (Vista/Server 2008) and lower are not supported.

>+  ${If} ${AtMostWin2008}
This should be fine but out of curiousity why didn't you just go with the following?
${Unless} ${AtLeastWin7}

>     ${If} "$R7" == "0"
>       strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_CPU_MSG)"
>     ${Else}
>       strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_MSG)"
>     ${EndIf}
>     MessageBox MB_OKCANCEL|MB_ICONSTOP "$R7" IDCANCEL +2
>     ExecShell "open" "${URLSystemRequirements}"
>     Quit
>-  ${EndUnless}
>-
>-  SetRegView 64
Though this will change soon please do the same thing here as you did in installer.nsi so it isn't overlooked.

>-!else
>-  StrCpy $R8 "0"
>-  ${If} ${AtMostWin2000}
>-    StrCpy $R8 "1"
>   ${EndIf}
<snip>
>-      ExecShell "open" "${URLSystemRequirements}"
>-      Quit
>-    ${EndIf}
>-  ${EndUnless}
>-!endif
>-
>+  ; SSE CPU support
It is actually SSE2

>diff --git a/toolkit/mozapps/installer/windows/nsis/common.nsh b/toolkit/mozapps/installer/windows/nsis/common.nsh
>--- a/toolkit/mozapps/installer/windows/nsis/common.nsh
>+++ b/toolkit/mozapps/installer/windows/nsis/common.nsh
>@@ -5102,56 +5102,26 @@
>       ; SSE2 instruction set is available.
>       System::Call "kernel32::IsProcessorFeaturePresent(i 10)i .R8"
>       ${If} "$R8" == "0"
>         MessageBox MB_OK|MB_ICONSTOP "$R9"
>         ; Nothing initialized so no need to call OnEndCommon
>         Quit
>       ${EndIf}
> 
>+      ; Operating system checks: NT 6.0 (Vista/Server 2008) or lower no
>+      ; longer supported.
Just make this along the lines of:
Windows NT 6.0 (Vista/Server 2008) and lower are not supported.

>+      ${If} ${AtMostWin2008}
>+        MessageBox MB_OK|MB_ICONSTOP "$R9"
>+        ; Nothing initialized so no need to call OnEndCommon
>+        Quit
>+      ${EndIf}

I want one more look at this before r+ing
Attachment #8810991 - Flags: review?(robert.strong.bugs) → review-
Attached image dialog
Testing off a try build of the original patches just to confirm this is working as expected.

https://archive.mozilla.org/pub/firefox/try-builds/jmathies@mozilla.com-4ce0805561b157428537d065296bb3fb6461c72e/try-win32/
Attached image tab load
Hey rstrong, do you know if we can be more specific here in terms of release version? Looks like we navigate to a generic url [1]. In 53, this takes me to 50's requirements. I'd like to modify this in some way such that the 53 installer sends users to the 53 requirements regardless of what they default browser is.

[1] http://searchfox.org/mozilla-central/rev/efcb1644e49c36445e75d89b434fdf4c84832c84/browser/branding/aurora/branding.nsi#19
Flags: needinfo?(robert.strong.bugs)
It can be controlled by channel as seen by the url you posted in the channel specific file. You can coordinate that change with the parties that manage the system requirements page. So, if there is a system requirements page for nightly you can change the url for nightly here and add that only Win 7 and above is supported to that page.
https://dxr.mozilla.org/mozilla-central/source/browser/branding/nightly/branding.nsi#18

Do the same for the other channels and you can have a page for each channel. When XP is no longer supported for the channel (e.g. after merge) the system requirements page can be updated.

Last I checked there was little to no interest in maintaining a system requirements page for all of the channels and I was told to just use the link as shown in all of the branding.nsi files. I left those in the branding.nsi files instead of one central location just in case something like this came up.
Flags: needinfo?(robert.strong.bugs)
This may would affect ReactOS, if ReactOS got affected, you can try to add an exception.
fyi, I filed bug 1318671 on updating the system requirements page.
Attached patch patch (obsolete) — Splinter Review
Attachment #8812205 - Attachment is obsolete: true
Attachment #8812206 - Flags: review?(robert.strong.bugs)
Comment on attachment 8812206 [details] [diff] [review]
patch

>diff --git a/browser/installer/windows/nsis/installer.nsi b/browser/installer/windows/nsis/installer.nsi
>--- a/browser/installer/windows/nsis/installer.nsi
>+++ b/browser/installer/windows/nsis/installer.nsi
>@@ -1133,78 +1133,42 @@ Function .onInit
>   System::Call 'kernel32::SetDllDirectoryW(w "")'
> 
>   StrCpy $PageName ""
>   StrCpy $LANGUAGE 0
>   ${SetBrandNameVars} "$EXEDIR\core\distribution\setup.ini"
> 
>   ; Don't install on systems that don't support SSE2. The parameter value of
>   ; 10 is for PF_XMMI64_INSTRUCTIONS_AVAILABLE which will check whether the
>-  ; SSE2 instruction set is available.
>+  ; SSE2 instruction set is available. Result returned in $R7.
>   System::Call "kernel32::IsProcessorFeaturePresent(i 10)i .R7"
> 
>-!ifdef HAVE_64BIT_BUILD
>-  ; Restrict x64 builds from being installed on x86 and pre Win7
>-  ${Unless} ${RunningX64}
>-  ${OrUnless} ${AtLeastWin7}
>+  ; Windows NT 6.0 (Vista/Server 2008) and lower are not supported.
>+  ${Unless} ${AtLeastWin7}
>     ${If} "$R7" == "0"
>       strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_CPU_MSG)"
>     ${Else}
>       strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_MSG)"
>     ${EndIf}
>     MessageBox MB_OKCANCEL|MB_ICONSTOP "$R7" IDCANCEL +2
>     ExecShell "open" "${URLSystemRequirements}"
>     Quit
>-  ${EndUnless}
You removed the matching EndUnless

>diff --git a/browser/installer/windows/nsis/stub.nsi b/browser/installer/windows/nsis/stub.nsi
>--- a/browser/installer/windows/nsis/stub.nsi
>+++ b/browser/installer/windows/nsis/stub.nsi
>@@ -316,75 +316,39 @@ Function .onInit
>   ; isn't supported for the stub installer.
>   ${SetBrandNameVars} "$PLUGINSDIR\ignored.ini"
> 
>   ; Don't install on systems that don't support SSE2. The parameter value of
>   ; 10 is for PF_XMMI64_INSTRUCTIONS_AVAILABLE which will check whether the
>   ; SSE2 instruction set is available.
>   System::Call "kernel32::IsProcessorFeaturePresent(i 10)i .R7"
> 
>-!ifdef HAVE_64BIT_BUILD
>-  ; Restrict x64 builds from being installed on x86 and pre Win7
>-  ${Unless} ${RunningX64}
>-  ${OrUnless} ${AtLeastWin7}
>+  ; Windows NT 6.0 (Vista/Server 2008) and lower are not supported.
>+  ${Unless} ${AtLeastWin7}
>     ${If} "$R7" == "0"
>       strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_CPU_MSG)"
>     ${Else}
>       strCpy $R7 "$(WARN_MIN_SUPPORTED_OSVER_MSG)"
>     ${EndIf}
>     MessageBox MB_OKCANCEL|MB_ICONSTOP "$R7" IDCANCEL +2
>     ExecShell "open" "${URLSystemRequirements}"
>     Quit
>-  ${EndUnless}
You removed the matching EndUnless

The following need to be changed as well to ${Unless} ${AtLeastWin7}
https://dxr.mozilla.org/mozilla-central/source/toolkit/components/maintenanceservice/bootstrapinstaller/maintenanceservice_installer.nsi#123
https://dxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/maintenanceservice_installer.nsi#126

After these changes all should be good
Attachment #8812206 - Flags: review?(robert.strong.bugs) → review-
Comment on attachment 8812206 [details] [diff] [review]
patch

and remove the following EndIf from installer.nsi and stub.nsi
>-!else
>-  StrCpy $R8 "0"
>-  ${If} ${AtMostWin2000}
>-    StrCpy $R8 "1"
>   ${EndIf}
BTW: I would be fine with just removed the comments about Win2K in the two maintenanceservice_installer.nsi files
Attached patch patch (obsolete) — Splinter Review
Attachment #8812206 - Attachment is obsolete: true
Looks good and thanks. I'll r+ it after you request review.
Attached patch patchSplinter Review
Attachment #8812303 - Attachment is obsolete: true
Attachment #8812304 - Flags: review?(robert.strong.bugs)
Comment on attachment 8812304 [details] [diff] [review]
patch

Hmmm... just realized that this doesn't prevent 32 bit systems from installing 64 bit Firefox. Please file a new bug for that and I'll take care of it unless you want to do that in this bug as well.
Attachment #8812304 - Flags: review?(robert.strong.bugs) → review+
Attached patch followupSplinter Review
Jim, this should take care of the issue I found in comment #30
Attachment #8812537 - Flags: review?(jmathies)
Attachment #8812537 - Flags: review?(jmathies) → review+
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/af88a04128f6
Main patch for Bug 1305453 - Prevent non-esr stand alone installers from installing on XP/Vista. r=rstrong
https://hg.mozilla.org/integration/mozilla-inbound/rev/86a19f0dfa67
Followup to handle not installing x64 on x86 systems for Bug 1305453 - Prevent non-esr stand alone installers from installing on XP/Vista. r=mhowell
Since bug 1318619 landed a change that broke Firefox on XP and Vista I landed this on inbound so people will receive an error message instead of installing a build that is broken on their system. If a requirements page for Nightly is wanted that can be done in a new bug after the requirements page is created.
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #33)
> Since bug 1318619 landed a change that broke Firefox on XP and Vista

The offending change has been reverted:
https://hg.mozilla.org/mozilla-central/rev/c788e6d12d93

That said, given that we disabled all tests on XP, we should expect Nightly will soon break again on XP.
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #33)
> If a requirements page
> for Nightly is wanted that can be done in a new bug after the requirements
> page is created.

This is covered by bug 1318671.
https://hg.mozilla.org/mozilla-central/rev/af88a04128f6
https://hg.mozilla.org/mozilla-central/rev/86a19f0dfa67
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Depends on: 1319469
Depends on: 1319921
Flags: qe-verify+
I verified that Firefox 53 beta 2, latest Developer Edition 54.0a1 and latest Nightly 55.0a1 can't be installed on Windows XP x86 and Windows Vista x64. Clicking OK on the error prompt for each build from above will open https://www.mozilla.org/en-US/firefox/52.0/system-requirements/ in the default browser. I checked stub installers and normal installers for 32bit and 64bit builds.

The following error messages appear when trying to install Fx:

Fx 53 beta 2:
Sorry, Firefox can't be installed. The version of Firefox requires Microsoft Windows 7 or 
newer. Please click the OK button for additional information.

Fx 54.0a2:
Sorry, Firefox Developer Edition can't be installed. The version of Firefox Developer 
Edition requires Microsoft Windows 7 or newer. Please click the OK button for additional 
information.

Fx 55.0a1:
Sorry, Nightly can't be installed. The version of Nightly requires Microsoft Windows 7 or 
newer. Please click the OK button for additional information.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: