Building with NSIS 2.33u restricts installation on all service packs of Windows XP

VERIFIED FIXED in Firefox 13

Status

()

Firefox
Installer
--
blocker
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: p_mccarthy, Assigned: bbondy)

Tracking

({regression})

Trunk
Firefox 13
x86
Windows XP
regression
Points:
---

Firefox Tracking Flags

(firefox13 verified)

Details

(Whiteboard: [qa!])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
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.
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
Depends on: 668574
Blocks: 668574
Severity: normal → blocker
Component: Untriaged → Installer
No longer depends on: 668574
Keywords: regression
QA Contact: untriaged → installer
Version: 13 Branch → Trunk

Updated

5 years ago
Duplicate of this bug: 724332

Comment 3

5 years ago
I can reproduce on Windows XP SP3.
See 724332
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 4

5 years ago
Created attachment 594515 [details]
About Windows, and error message
(Assignee)

Comment 5

5 years ago
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.
(Assignee)

Comment 6

5 years ago
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?
2.33-Unicode. Latest MozillaBuild includes 2.46-Unicode. I guess it was never deployed to the build slaves?
NSIS versions are definitely questions for rs ;-)
(Assignee)

Comment 9

5 years ago
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.
(Assignee)

Comment 10

5 years ago
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.
Assignee: nobody → netzen
Attachment #594625 - Flags: review?(robert.bugzilla)
(Assignee)

Updated

5 years ago
Summary: windows sp2 message → Building with NSIS 2.33u restricts installation on all service packs of Windows XP
(Assignee)

Comment 11

5 years ago
Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=4cef841beb7e

Updated

5 years ago
Duplicate of this bug: 724470
(Assignee)

Comment 13

5 years ago
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 :)
Attachment #594625 - Attachment is obsolete: true
Attachment #594625 - Flags: review?(robert.bugzilla)
Attachment #594684 - Flags: review?(robert.bugzilla)
(Assignee)

Comment 14

5 years ago
Re-pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=9b3067afe49b
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
Attachment #594684 - Flags: review?(robert.bugzilla) → review+
(Assignee)

Comment 16

5 years ago
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.
(Assignee)

Comment 17

5 years ago
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?
Never
(Assignee)

Comment 19

5 years ago
Created attachment 594727 [details] [diff] [review]
Patch v3.

Implemented review comments. Carried forward r+.
Attachment #594684 - Attachment is obsolete: true
Attachment #594727 - Flags: review+
(Assignee)

Comment 20

5 years ago
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.
(Reporter)

Comment 21

5 years ago
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)
Yes, the fix for this bug has not landed yet, so the bug is still present.
(Assignee)

Comment 23

5 years ago
Pushed to mozilla-central, fix will be in the next Nightly build.
http://hg.mozilla.org/mozilla-central/rev/b077059c575a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
status-firefox13: --- → fixed
Whiteboard: [qa+]
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
Status: RESOLVED → VERIFIED
status-firefox13: fixed → verified
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.