Closed Bug 668574 Opened 13 years ago Closed 12 years ago

Refuse to install on Windows XP SP 1 and below.

Categories

(Firefox :: Installer, defect)

Product:
Firefox
Component:
Installer
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 13
Tracking Flags:
Tracking Status
firefox7 - ---
firefox13 + verified

People

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

References

Details

(Whiteboard: [qa+])

Attachments

(1 file, 6 obsolete files)

patch updated to comments
10.72 KB, patch
robert.strong.bugs
: review+
Details | Diff | Splinter Review 
After we switch compiler versions the installer will need to refuse to install on the newly unsupported versions of Windows.
Individual apps get to decide this so moving over to Firefox
Component: NSIS Installer → Installer
Product: Toolkit → Firefox
QA Contact: nsis.installer → installer
Version: unspecified → Trunk
Not sure why this was nominated for tracking 7, since the compiler change isn't scheduled to land until the Firefox 8 cycle...
tracking-firefox7: ? → -
You're correct... sorry about that
cc'ing a couple of people for apps other than Firefox so they are in the loop.
I have a patch ready about Windows XP SP1. It aborts the process showing a message to the user if the target OS is Windows XP SP1 or earlier NT based. The same will happens with Win95/Win98/WinME.

What about Windows 2003 (server version)?
Assignee: nobody → leofigueres
Depends on: 488585
khuey: Will Fx not run at all, or is this just an unsupported config? If Fx will still run but is expected to be buggy, I don't think we should stop the install, and instead just offer a warning.

Re: this patch as is, I'm not a huge fan of the wording of this message:

"{BrandShortName} needs Windows XP Service Pack 2 or a later version of Windows. Installation will stop."

Maybe something along the lines of:

"{BrandShortName} is no longer support on this operating system. Windows XP Service Pack 2 or a later are required."

Also, there may be issues here with trade marked names. That's one of the reasons why we have an official branded build. (cc'ing Gerv on this.)

I would also suggest we add a URI here to a support page giving more detail as to the reasons why. But that could probably be in a follow up that should track whatever release we plan to send this out in.
:jimm: are you worried about the trademark on "Windows" or about our trademark? If the former, the legal team are the people to ask, but I doubt it's a big issue.

Windows XP SP1 is unsupported by Microsoft, insecure, and shouldn't be let within 10 feet of an Internet connection. I don't think it's in the best interests of users for us to give them buggy software so they can browse the internet, hate Firefox, and get pwned.

Gerv
Once we upgrade to VS 2010, Firefox will not even start up on these versions of Windows (it'll fail when the CRT DLLs try to find symbols that don't exist on the OS).
We could ship those dlls
(In reply to Gervase Markham [:gerv] from comment #8)
> :jimm: are you worried about the trademark on "Windows" or about our
> trademark? If the former, the legal team are the people to ask, but I doubt
> it's a big issue.
> 
> Windows XP SP1 is unsupported by Microsoft, insecure, and shouldn't be let
> within 10 feet of an Internet connection. I don't think it's in the best
> interests of users for us to give them buggy software so they can browse the
> internet, hate Firefox, and get pwned.
> 
> Gerv

I was wondering about the use of "Windows XP SP1". In the migrator for example, we have the name "Internet Explorer", but we ifdef that out for everything except branded builds. Why that is I am not sure, but seems like whatever restricts us from distributing with "Internet Explorer" in the UI might be the same for "Windows XP"?
(In reply to Brian R. Bondy [:bbondy] from comment #10)
> We could ship those dlls

Er, I think you misunderstand.  The CRT DLLs (which we do ship) require OS level functionality that is not available on versions of Windows prior to XP SP2.
I have no idea why that IFDEF might be there. CCing Liz on the trademark question.

Gerv
(In reply to Gervase Markham [:gerv] from comment #13)
> I have no idea why that IFDEF might be there. CCing Liz on the trademark
> question.
> 
> Gerv

Per discussion with Gavin, my mistake. The Internet Explorer string is available in all builds. Sorry for the confusion.
Just for edification, it's fine to use another company's trademark to refer to their product, as long as you use it correctly (e.g. with the (R) symbol if they request that, full name, as an adjective, etc.) See http://www.microsoft.com/About/Legal/EN/US/IntellectualProperty/Trademarks/Usage/General.aspx for Microsoft's general trademark guidelines and http://www.microsoft.com/about/legal/en/us/IntellectualProperty/Trademarks/Usage/Windows.aspx for Windows trademark guidelines.
I believe that the requirement of XP SP2 will apply to all Mozilla based applications as it has in the past. If so, then the change should be made to
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/windows/nsis/common.nsh#4626

and the MinSupportedVer value will need to be changed.
http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/defines.nsi.in#54
Comment on attachment 570742 [details] [diff] [review]
Aborting installation process load if OS version is not supported

based on rob's comments, it looks like this needs to be revamped.
Attachment #570742 - Flags: review?(jmathies) → review-
(In reply to Robert Strong [:rstrong] (do not email) from comment #16)
> I believe that the requirement of XP SP2 will apply to all Mozilla based
> applications as it has in the past. If so, then the change should be made to
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/
> windows/nsis/common.nsh#4626

In this piece of code there has been added some checkings to the registry because AtLeastWin2000 had been failing. Could it be switch to AtMostWin2000? My new patch fails because I am testing with the compatibility mode of Windows 7.

> 
> and the MinSupportedVer value will need to be changed.

Change to "Microsoft Windows XP SP2"?
(In reply to Javi Rueda from comment #18)
> My new patch fails because I am testing with the compatibility mode of Windows 7.

Fwiw, I can test (Try/other) builds on my Windows 2000.
(In reply to Serge Gautherie (:sgautherie) from comment #19)
> (In reply to Javi Rueda from comment #18)
> > My new patch fails because I am testing with the compatibility mode of Windows 7.
> 
> Fwiw, I can test (Try/other) builds on my Windows 2000.

Thank you Serge. I have managed myself to avoid testing for Win2000 in the patch I am going to send in a few seconds. Should abort installation with any Service Pack.

Anyone with a XP SP1 machine? Windows 7 only allows for WinXP SP2 for compatibility mode :-(
Status: NEW → ASSIGNED
Same as previous patch, but for all Mozilla applications.
Attachment #570742 - Attachment is obsolete: true
Attachment #571731 - Flags: review?(jmathies)
Assignee: leofigueres → nobody
Component: Installer → NSIS Installer
Product: Firefox → Toolkit
QA Contact: installer → nsis.installer
Assignee: nobody → leofigueres
Comment on attachment 571731 [details] [diff] [review]
Aborting installation of Mozilla apps if OS version is less than Windows XP SP2

We will still need the registry checks though they need to be updated for the new restrictions otherwise some systems that are compatible will not be able to install. From what I recall, some systems when they are upgraded return the incorrect system information when using the MS API calls to get this information.
Comment on attachment 571731 [details] [diff] [review]
Aborting installation of Mozilla apps if OS version is less than Windows XP SP2

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

Just to be explicit, check whether the following lines, which cause no harm, could/should be removed too:
{
5601       ${If} ${IsWin2000}
5602         ${LogMsg} "OS Name    : Windows 2000"
5603       ${ElseIf} ${IsWinXP}
}

::: toolkit/mozapps/installer/windows/nsis/common.nsh
@@ +4633,2 @@
>              MessageBox MB_OK|MB_ICONSTOP "$R9" IDOK
>              ; Nothing initialized so no need to call OnEndCommon

You should probably copy this comment too.
Tested using the compatibility mode on Win7: NT4 SP5, Win2000, WinXP SP2/SP3 and Windows Server 2003/2008 SP1.

When testing with Win9x comp-mode, the installation process showed the first page of the setup wizard.

Could you test this one on your Windows 2000 system, Serge? Thank you ^.^
Attachment #571731 - Attachment is obsolete: true
Attachment #571867 - Flags: review?(jmathies)
Attachment #571731 - Flags: review?(jmathies)
(In reply to Javi Rueda from comment #24)

> When testing with Win9x comp-mode, the installation process showed the first
> page of the setup wizard.

(Is that the expected behavior?
Or at least the same behavior as without your patch?)

> Could you test this one on your Windows 2000 system, Serge? Thank you ^.^

I can, if you provide an installer build ;->
(In reply to Serge Gautherie (:sgautherie) from comment #25)
> (In reply to Javi Rueda from comment #24)
> 
> > When testing with Win9x comp-mode, the installation process showed the first
> > page of the setup wizard.
> 
> (Is that the expected behavior?
> Or at least the same behavior as without your patch?)

I have tested it without the patch ant its behavior was the same (except not refusing to install with the compatibility mode set to Windows 2000). May be was the expected behavior, Robert?
 
> > Could you test this one on your Windows 2000 system, Serge? Thank you ^.^
> 
> I can, if you provide an installer build ;->

I have upload it here for testing purposes. It was built with attachment 581767 [details].

https://docs.google.com/open?id=0B5O1vkTpesx6N2ZjMjRhZDQtYjg0YS00ZTEwLTkyNjUtZWU1MjM4YjViODU3
(In reply to Javi Rueda from comment #26)
> https://docs.google.com/
> open?id=0B5O1vkTpesx6N2ZjMjRhZDQtYjg0YS00ZTEwLTkyNjUtZWU1MjM4YjViODU3

It has the expected behavior on my Win2000sp4 Pro:
100% extracted, then "requires WinXPsp2" error dialog :-)
(In reply to Serge Gautherie (:sgautherie) from comment #28)
> 
> It has the expected behavior on my Win2000sp4 Pro:
> 100% extracted, then "requires WinXPsp2" error dialog :-)

Thank you for testing it (and so quickly), Serge.

Also it would be great if somebody with a real Windows XP SP1 could do the same. Keeping the file public because of this.
(In reply to Javi Rueda from comment #26)
> (In reply to Serge Gautherie (:sgautherie) from comment #25)
> > (In reply to Javi Rueda from comment #24)
> > 
> > > When testing with Win9x comp-mode, the installation process showed the first
> > > page of the setup wizard.
> > 
> > (Is that the expected behavior?
> > Or at least the same behavior as without your patch?)
> 
> I have tested it without the patch ant its behavior was the same (except not
> refusing to install with the compatibility mode set to Windows 2000). May be
> was the expected behavior, Robert?
Not sure though when using compatibility mode there are idiosyncrasies that could cause that.
Removing dependence from bug 488585 as new patches here are using already existing strings. I changed it because I needed this string.

Setting back to Installer as of comment 1, although change will affect all Mozilla apps.
No longer depends on: 488585
Component: NSIS Installer → Installer
Product: Toolkit → Firefox
QA Contact: nsis.installer → installer
ping
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jmathies@mozilla.com-3f5ef24695fa

This looks ok to me. I pushed this to try so I can test on an xp image I have.
!insertmacro: macro "_And" requires 4 parameter(s), passed 3!
Error in macro InstallOnInitCommon on macroline 50
Error in script "installer.nsi" on line 135 -- aborting creation process
make[3]: Leaving directory `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer/windows'
make[2]: Leaving directory `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer/windows'
make[1]: Leaving directory `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer'
make[3]: *** [instgen/setup.exe] Error 1
make[2]: *** [installer] Error 2
make[1]: *** [installer] Error 2
make: *** [installer] Error 2
program finished with exit code 2


This doesn't build.
(In reply to Jim Mathies [:jimm] from comment #34)
> !insertmacro: macro "_And" requires 4 parameter(s), passed 3!
> Error in macro InstallOnInitCommon on macroline 50
> Error in script "installer.nsi" on line 135 -- aborting creation process
> make[3]: Leaving directory
> `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer/windows'
> make[2]: Leaving directory
> `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer/windows'
> make[1]: Leaving directory
> `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer'
> make[3]: *** [instgen/setup.exe] Error 1
> make[2]: *** [installer] Error 2
> make[1]: *** [installer] Error 2
> make: *** [installer] Error 2
> program finished with exit code 2
> 
> 
> This doesn't build.

Ups!
I don't receive any error message building from the obj-dir (tested with last repository tip). Now I am retesting building again all the source code and will build again the installer.
To make the installer and uninstaller cd into <obj-dir>/browser/installer/windows and then run:

make uninstaller

make installer
(In reply to Robert Strong [:rstrong] (do not email) from comment #37)
> To make the installer and uninstaller cd into
> <obj-dir>/browser/installer/windows and then run:
> 
> make uninstaller
> 
> make installer

Thank you, Robert. That is the way I create the installer, but from the browser/installer directory, not from the windows sub-directory.

The log from the try-build shows that it was run from the obj-dir, so I tried it from there again and returned no error (and installer worked and expected once it was built).
Jim, I'll try to find time to help out here... you have more important bugs to focus on.

Javi, the error below shows it was in browser/installer/windows

(In reply to Javi Rueda from comment #35)
> (In reply to Jim Mathies [:jimm] from comment #34)
> > !insertmacro: macro "_And" requires 4 parameter(s), passed 3!
> > Error in macro InstallOnInitCommon on macroline 50
> > Error in script "installer.nsi" on line 135 -- aborting creation process
> > make[3]: Leaving directory
> > `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer/windows'
> > make[2]: Leaving directory
> > `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer/windows'
> > make[1]: Leaving directory
> > `/e/builds/moz2_slave/try-w32/build/obj-firefox/browser/installer'
> > make[3]: *** [instgen/setup.exe] Error 1
> > make[2]: *** [installer] Error 2
> > make[1]: *** [installer] Error 2
> > make: *** [installer] Error 2
> > program finished with exit code 2

I'm in the middle of a build right now and will check the patch out sometime tomorrow.
Honestly I don't see where this _And macro is coming into play here. Here's the patch I submitted:

https://hg.mozilla.org/try/rev/c569f5963169
Thank you for your interest, Jim.

In LogicLib.nsh, AndIf macro is defined like this:

!define AndIf     `!insertmacro _And true`

My patch uses it in line 2.36 on the link you posted : https://hg.mozilla.org/try/rev/c569f5963169#l2.36
> To make the installer and uninstaller cd into
> <obj-dir>/browser/installer/windows and then run:
> 
> make uninstaller
> 
> make installer

Built this way. Returned no errors and installation package worked as expected in this bug.
It might have to do with the version of NSIS deployed to the build machines being 2.33. I'll check and report back.
Output during the building process showed it was using NSIS 2.46u.
Comment 44 was related to _my_ building process.
Attachment #571867 - Flags: review?(jmathies) → review-
Why? :-S
Comment on attachment 571867 [details] [diff] [review]
Refusing to install on unsupported versions patch v2

Likely due to the build failures Jim saw that are most likely caused by needing to support NSIS 2.33. I'll take a look after I've finished up some higher priority work I have to get done.
Attachment #571867 - Flags: review- → review?(robert.bugzilla)
Is there a way to force to use 2.33 on my system?
For an existing build you can
In the MozillaBuild directory
Rename nsis-2.46u to nsis-2.46u.bak
Rename nsis-2.33u to nsis-2.46u
Inside the new nsis-2.46u directory
Copy makensisu.exe and name this file makensisu-2.46.exe


For a new (e.g. clobber) build just rename nsis-2.46u to nsis-2.46u.bak
I've got the _And macro error. I will be working trying to rework the patch to avoid this, while waiting for any news about the patch already post to this bug.
(In reply to Javi Rueda from comment #50)
> I've got the _And macro error. I will be working trying to rework the patch
> to avoid this, while waiting for any news about the patch already post to
> this bug.

As soon as you post it I'll send it to try so we can take it for a spin on XP.
It was not possible to use AtLeastServicePack, because it was introduced in version 2.39 of NSIS. Then we manually check for ServicePack 1 or even no Service Pack installed.

To make it easy to read the code, it has been created a new function (ShowNoSupportedWinVersionMB) and it is called depending of the version detected.

Although it doesn't checks for Win9x OS, it is very possible that the code shows the error when trying to read the key that enables us to detect OS version, as "/Windows NT/" tree doesn't exist in those kind of systems.

As previous patches, I tested it using the compatibility mode and worked the way it was supposed to.

Requesting to Robert because of previous comments. If it is wrong, feel free to change the requestee. (you will do it anyway, as my permission is not needed, hehehe).
Attachment #571867 - Attachment is obsolete: true
Attachment #578374 - Flags: review?(robert.bugzilla)
Attachment #571867 - Flags: review?(robert.bugzilla)
Javi, just so you know a decision hasn't been made as to when we will drop support for Win XP SP1 and below so this isn't a high priority at this moment.
Bug 563318 has now landed, so we'll need this fairly soon if possible :-)

nthomas is working on bug 682182, which will at least stop the Nightly updates, but I imagine most people's reaction to not seeing an update after a couple of days, will be to download manually.
Comment on attachment 578374 [details] [diff] [review]
Quitting if OS version is Windows XP SP1 or earlier

We always try to use existing functionality provided by NSIS and the latest NSIS already has service pack detection capability. When we have to also support a version of NSIS that doesn't contain the functionality we need we use !ifdef, !ifndef, !ifmacrodef, or !ifmacrondef in overrides.nsh and add the newer code there. This way we continue support for the older version of NSIS, automatically start using the version in the newer release, and can just remove the code from overrides.nsh when we deprecate the older NSIS version. I'm r-'ing the patch due to the above. Since removal of Win2K / WinXP SP1 has landed I am going to take this bug (sorry about that) and I'll add a comment in the landing that you wrote the original patch. Thanks!
Attachment #578374 - Flags: review?(robert.bugzilla) → review-
Attached patch patch rev1 (obsolete) — — Splinter Review 
Assignee: leofigueres → robert.bugzilla

        Attachment #578374 -
        Attachment is obsolete: true

        Attachment #593640 -
        Flags: review?(netzen)

    
    

    
  
Note: I noticed that on Win7 the registry check returned the same version as the exe's compatibility so verifying that on Win7 won't work. Way back when we added Vista support (Firefox 2.0.0.2 I believe) we had a Vista user verify that they were unable to install without those additional registry checks so I would like to keep them since they haven't caused any harm since Firefox 2.0.0.2.

    
    

    
  
Comment on attachment 593640 [details] [diff] [review]
patch rev1

bah... I thought I had checked this with NSIS 2.33 but I haven't. I'll request review after I've done that.

        Attachment #593640 -
        Flags: review?(netzen)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch rev2 (obsolete)
        — 
          — Splinter Review
      

    


  
Added a couple of defines and macros needed for NSIS 2.33 support

        Attachment #593640 -
        Attachment is obsolete: true

        Attachment #593665 -
        Flags: review?(netzen)

    
    

    
  
Comment on attachment 593665 [details] [diff] [review]
patch rev2

Note: I changed the following locally
>+!endif # __WinVer_DeclareVars

to

!endif # _WINVER_VERXBIT

    
    

    
  
Comment on attachment 593665 [details] [diff] [review]
patch rev2

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

Looks good, also tested in the following scenarios:

- Windows 2000 gives the correct error message on install
- Windows XP SP1 or lower I couldn't test since I don't have access to a machine with that.
- Windows XP SP2, Vista and Win7 installed correctly.

::: toolkit/mozapps/installer/windows/nsis/common.nsh
@@ +4638,1 @@
>            ; XXX-rstrong - some systems fail the AtLeastWin2000 test for an

nit: Comment should be updated since we no longer use AtLeastWin2000

        Attachment #593665 -
        Flags: review?(netzen) → review+
Flags: in-litmus?

    
    

    
  


  

        Attachment #593665 -
        Attachment is obsolete: true

        Attachment #593742 -
        Flags: review+
Target Milestone: --- → Firefox 13

    
    

    
  
Thank you for your FAST patch, Robert. Also for mentioning me at the push summary :-)

    
    

    
  
Pushed to mozilla-central
https://hg.mozilla.org/mozilla-central/rev/e0a1c04765e3
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Blocks: 724283
No longer blocks: 724283
Depends on: 724283

    
  
Whiteboard: [qa+]

    
    

    
  
Verified that the install is refused on Win 2000 and on Win XP SP1 - the error message is displayed: "Sorry, Firefox can't be installed. This version of Firefox requires Microsoft XP SP 2 or newer".

Windows XP SP1 was on a virtual machine.

          status-firefox13:
          fixed → verified

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: