Last Comment Bug 724283 - Building with NSIS 2.33u restricts installation on all service packs of Windows XP
: Building with NSIS 2.33u restricts installation on all service packs of Windo...
Status: VERIFIED FIXED
[qa!]
: regression
Product: Firefox
Classification: Client Software
Component: Installer (show other bugs)
: Trunk
: x86 Windows XP
: -- blocker with 1 vote (vote)
: Firefox 13
Assigned To: Brian R. Bondy [:bbondy]
:
Mentors:
: 724332 724470 (view as bug list)
Depends on:
Blocks: 668574
  Show dependency treegraph
 
Reported: 2012-02-04 10:16 PST by p_mccarthy
Modified: 2012-05-11 08:04 PDT (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified


Attachments
About Windows, and error message (46.18 KB, image/png)
2012-02-04 23:34 PST, Alice0775 White
no flags Details
Patch v1. (1.05 KB, patch)
2012-02-05 20:16 PST, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
Patch v2. (1.01 KB, patch)
2012-02-06 05:43 PST, Brian R. Bondy [:bbondy]
robert.strong.bugs: review+
Details | Diff | Splinter Review
Patch v3. (1.26 KB, patch)
2012-02-06 08:51 PST, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Splinter Review

Description p_mccarthy 2012-02-04 10:16:58 PST
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:13.0a1) Gecko/20120204 Firefox/13.0a1
Build ID: 20120204031137

Steps to reproduce:

latest build (todays) 13.0a1 wont install says I need xp with sp2 installed.  XP is up to date with latest updates all installed.  


Actual results:

latest build (todays) 13.0a1 wont install says I need xp with sp2 installed.  XP is up to date with latest updates all installed.  


Expected results:

just install.  It already auto updated to 13.0a1.
Comment 1 Matthias Versen [:Matti] 2012-02-04 11:16:28 PST
Are you sure that you have SP2 or SP3 installed ?

From your Windows desktop, double-click 'My Computer'.
Select Help|About Windows
The 'About Windows' panel displays a variety of information, including which service pack is installed
Comment 2 Alice0775 White 2012-02-04 23:17:51 PST
*** Bug 724332 has been marked as a duplicate of this bug. ***
Comment 3 Alice0775 White 2012-02-04 23:20:06 PST
I can reproduce on Windows XP SP3.
See 724332
Comment 4 Alice0775 White 2012-02-04 23:34:14 PST
Created attachment 594515 [details]
About Windows, and error message
Comment 5 Brian R. Bondy [:bbondy] 2012-02-05 06:40:09 PST
Confirmed that I can reproduce it with a downloaded build.  I'm pretty sure I tried it with a locally built build though and it worked.  I'm investigating more.
Comment 6 Brian R. Bondy [:bbondy] 2012-02-05 06:57:06 PST
Tried making a small installer and the detection code from bug 668574 works on XP SP3.  Does anyone know which version of NSIS the build machines use to do a build?
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-02-05 07:45:03 PST
2.33-Unicode. Latest MozillaBuild includes 2.46-Unicode. I guess it was never deployed to the build slaves?
Comment 8 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-02-05 07:47:28 PST
NSIS versions are definitely questions for rs ;-)
Comment 9 Brian R. Bondy [:bbondy] 2012-02-05 17:34:30 PST
I confirmed by the way that building locally with the default MozillaBuild config produces a working binary.  

If I build a new NSIS installer with similar code with 2.46u it also works fine.
If I build a new NSIS installer with similar code with with 2.33u I can reproduce the problem on XP.  I should narrow down on the problem soon.
Comment 10 Brian R. Bondy [:bbondy] 2012-02-05 20:16:07 PST
Created attachment 594625 [details] [diff] [review]
Patch v1.

The problem is with the struct formatting string in NSIS v2.33unicode. 
Apparently with NSIS 2.33 you need to use the actual size of the wide string in bytes.

In later versions of NSIS you need to use the number of characters in the wide string.
So the change was to add special handling when dealing with nsis v2.33-Unicode to use a struct formatting string of &t256.s instead of &t128.s.

Tested with both 2.33 and 2.48 and it is working now.
Comment 11 Brian R. Bondy [:bbondy] 2012-02-05 20:22:20 PST
Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=4cef841beb7e
Comment 12 Alice0775 White 2012-02-05 23:28:28 PST
*** Bug 724470 has been marked as a duplicate of this bug. ***
Comment 13 Brian R. Bondy [:bbondy] 2012-02-06 05:43:07 PST
Created attachment 594684 [details] [diff] [review]
Patch v2.

Didn't want to introduce a dependency to logiclib for that file so just changed to StrCmp's ugly syntax :)
Comment 14 Brian R. Bondy [:bbondy] 2012-02-06 05:52:10 PST
Re-pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=9b3067afe49b
Comment 15 Robert Strong [:rstrong] (use needinfo to contact me) 2012-02-06 08:02:37 PST
Comment on attachment 594684 [details] [diff] [review]
Patch v2.

The only time this code will be used is with NSIS 2.33 (_WINVER_VERXBIT is defined in NSIS 2.46's WinVer.nsh) so you can just use the same system call without checking for the NSIS version.

Also, please add a short comment about this change.

r=me with that
Comment 16 Brian R. Bondy [:bbondy] 2012-02-06 08:11:15 PST
OK good point, I was thinking it might get renamed at some future release of NSIS, but if that was the case we'd get a build error.  New patch coming.
Comment 17 Brian R. Bondy [:bbondy] 2012-02-06 08:46:39 PST
Oh I remember I also was doing this to check for Unicode at the same time, do we ever build without Unicode for any product that uses this code?
Comment 18 Robert Strong [:rstrong] (use needinfo to contact me) 2012-02-06 08:49:42 PST
Never
Comment 19 Brian R. Bondy [:bbondy] 2012-02-06 08:51:28 PST
Created attachment 594727 [details] [diff] [review]
Patch v3.

Implemented review comments. Carried forward r+.
Comment 20 Brian R. Bondy [:bbondy] 2012-02-06 08:56:39 PST
Probably wasn't necessary but pushed to try one last time to be safe:
https://tbpl.mozilla.org/?tree=Try&rev=6aa769ca9cc9

Will push to inbound once that passes.
Comment 21 p_mccarthy 2012-02-06 10:54:18 PST
This bug is still in today's downloaded build (February 6, 2012.) Incidentally, SP2 and 3 were installed long ago.  I ran windows update after getting the message just to make sure and windows says sp3 is installed (Build 2600.xpsp_sp3_gdr.101209-1647: Service Pack 3, v.6055)
Comment 22 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-02-06 10:55:02 PST
Yes, the fix for this bug has not landed yet, so the bug is still present.
Comment 23 Brian R. Bondy [:bbondy] 2012-02-06 12:46:29 PST
Pushed to mozilla-central, fix will be in the next Nightly build.
http://hg.mozilla.org/mozilla-central/rev/b077059c575a
Comment 24 Simona B [:simonab] 2012-05-11 08:04:32 PDT
Verified that Firefox 13 beta 3 properly installs on Windows XP SP3 and that no error message dialog is thrown at installation.

Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20100101 Firefox/13.0

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