Closed Bug 305187 Opened 19 years ago Closed 19 years ago

(Stub-)Installer: 'Browse' button to select 'Destination Directory' doesn't work anymore

Categories

(Core Graveyard :: Installer: XPInstall Engine, defect)

x86
Windows 98
defect
Not set
blocker

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.8beta4

People

(Reporter: sgautherie, Assigned: neil)

Details

(Keywords: regression, verified1.8)

Attachments

(1 file, 1 obsolete file)

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050816 SeaMonkey/1.0a]
(nightly) (W98SE)

Works.

[17 & 18 nighlies]

Keyboard and mouse accesses:
the button can be pressed, but it doesn't open the directory chooser.

Then I can't proceed with installation, as I don't want the default path to be used.
Flags: blocking-seamonkey1.0a?
We only can consider that as a 1.0 Alpha blocker if you can reproduce it on a
branch build (I know we have no current ones yet) or a trunk build that was made
before branching.
(In reply to comment #1)
> We only can consider that as a 1.0 Alpha blocker if you can reproduce it on a
> branch build

Right, my mistake.
Flags: blocking-seamonkey1.0a?
(In reply to comment #3)
> working:   2005-08-16-06-trunk BuildID: 1.0.0.2005081605 Build started 04:37
> regressed: 2005-08-16-14-trunk BuildID: 1.0.0.2005081612 Build started 12:30

The only xpinstall checkin !?
{{
2005-08-16 09:31	timeless%mozdev.org 	mozilla/ xpinstall/ src/
nsJSInstallTriggerGlobal.cpp 	1.48 	2/2  	Bug 303654
InstallTrigger.install(null) crashes [@ JS_Enumerate] patch by
shutdown@flashmail.com r=dveditz sr=dveditz
}}
I tried 2005-08-19-07: both Full and Stub Installers have this bug.
Summary: Stub-Installer: 'Browse' button to select 'Destination Directory' doesn't work anymore → (Stub-)Installer: 'Browse' button to select 'Destination Directory' doesn't work anymore
serge: please don't cc me if the checkin says "patch by ..." in it. instead cc 
*that* person.
Assignee: general → xpi-engine
Component: Installer → Installer: XPInstall Engine
Product: Mozilla Application Suite → Core
QA Contact: bugzilla
It is not my fault. It's build config problem.

http://lxr.mozilla.org/seamonkey/source/xpinstall/wizard/windows/setup/dialogs.c#646
646   of.lStructSize        = sizeof(OPENFILENAME);

2005-08-16-06-trunk: sizeof(OPENFILENAME) == 0x4C
2005-08-16-14-trunk: sizeof(OPENFILENAME) == 0x58

This indicates that 2005-08-16-14-trunk is compiled with 0x0500 <= _WIN32_WINNT.
Win9x does not support extended OPENFILENAME. So it does not work anymore.
The problem is caused by INCLUDE path change.

Build Log (Full) WINNT 5.0 creature Clobber on 08/16 09:46:00
INCLUDE=
C:\Program Files\Microsoft Visual Studio\VC98\atl\include;
C:\Program Files\Microsoft Visual Studio\VC98\mfc\include;
C:\Program Files\Microsoft Visual Studio\VC98\include

Build Log (Full) WINNT 5.0 creature Clobber on 08/16 12:30:00  
INCLUDE=
C:\Program Files\Microsoft SDK\include;
C:\Program Files\Microsoft Visual Studio\VC98\atl\include;
C:\Program Files\Microsoft Visual Studio\VC98\mfc\include;
C:\Program Files\Microsoft Visual Studio\VC98\include

dialog.c includes winresrc.h indirectly. and winresrc.h from PSDK defines
_WIN32_WINNT as 0x0500 if it is not defined, while MSVC's one does not do so.
The problem can be solved by defining _WIN32_WINNT as 0x0400 explicitly.
(In reply to comment #7)
> It is not my fault. It's build config problem.

Thanks for your explanation.

> The problem is caused by INCLUDE path change.

(This doesn't seem to be part of the checkins listed in comment 3.)

> The problem can be solved by defining _WIN32_WINNT as 0x0400 explicitly.

"WINNT 5.0 creature Clobber on 08/20 04:03:00
Build Administrator is dbaron@dbaron.org or bryner@brianryner.com":
helpwanted.
So, um, are you saying you don't want the builds to support Win9x anymore?  That
seems to be what comment 7 says.
Oh, never mind, you're saying that there was a change that needs to be undone,
not that there needs to be one that wasn't there before.
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050824 SeaMonkey/1.0a]
(nightly) (W98SE)

(bug still there)
As a workaround, I'm trying (successfully it would seem so far) to use the
nightly Zip file...
Attached patch Possible patch (obsolete) — Splinter Review
Assignee: xpi-engine → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Attachment #193798 - Flags: review?(cls)
Comment on attachment 193798 [details] [diff] [review]
Possible patch

It's a good start since NSPR also defines _WIN32_WINNT=0x400 but I don't think
it's sufficient.  There are other places in xpinstall which explicitly redefine
WINVER=0x500 .	If we're still targetting win98, then that code needs to be
removed.

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winprog/winpro
g/using_the_windows_headers.asp
Attachment #193798 - Flags: review?(cls) → review-
(In reply to comment #14)
> Sorry, I misread lxr.  gfx is redefining WINVER=0x500, not xpinstall. 
> 
> http://lxr.mozilla.org/seamonkey/search?string=_WIN32_WINNT
> http://lxr.mozilla.org/seamonkey/search?string=WINVER
> 
> Trying that link again:
>
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winprog/winprog/using_the_windows_headers.asp
> 


no, it isn't redefining, only checking:
http://lxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/control/HelperAppDlg.cpp#165
http://lxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/control/HelperAppDlg.cpp#483
 
163     OPENFILENAME ofn;
164     memset(&ofn, 0, sizeof(ofn));
165 #if _WIN32_WINNT >= 0x0500
166     ofn.lStructSize = OPENFILENAME_SIZE_VERSION_400;
167 #else
168     ofn.lStructSize = sizeof(ofn);
169 #endif
(In reply to comment #14)
> Sorry, I misread lxr.  gfx is redefining WINVER=0x500, not xpinstall. 
> 
> http://lxr.mozilla.org/seamonkey/search?string=_WIN32_WINNT

(_WIN32_WINNT = 0x0400 is fine there)

> http://lxr.mozilla.org/seamonkey/search?string=WINVER

To be checked WINVER locations:

/embedding/components/printingui/src/win/nsPrintDialogUtil.cpp, line 39 --
#undef WINVER
/embedding/components/printingui/src/win/nsPrintDialogUtil.cpp, line 40 --
#define WINVER 0x0500
/embedding/tests/mfcembed/components/nsPrintDialogUtil.cpp, line 32 -- #undef WINVER
/embedding/tests/mfcembed/components/nsPrintDialogUtil.cpp, line 33 -- #define
WINVER 0x0500

/gfx/src/windows/nsScreenManagerWin.cpp, line 40 -- // APIs that are only
defined when WINVER is >= 0x0500. Don't worry,
/gfx/src/windows/nsScreenManagerWin.cpp, line 43 -- #undef WINVER
/gfx/src/windows/nsScreenManagerWin.cpp, line 44 -- #define WINVER 0x0500
/gfx/src/windows/nsScreenWin.cpp, line 40 -- // APIs that are only defined when
WINVER is >= 0x0500. Don't worry,
/gfx/src/windows/nsScreenWin.cpp, line 43 -- #undef WINVER
/gfx/src/windows/nsScreenWin.cpp, line 44 -- #define WINVER 0x0500

/gfx/cairo/cairo/src/cairo-win32-private.h, line 43 -- #define WINVER 0x0500

> Trying that link again:
>
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winprog/winprog/using_the_windows_headers.asp

Relevant parts:

Windows 2000 	_WIN32_WINNT>=0x0500 WINVER>=0x0500
Windows NT 4.0 	_WIN32_WINNT>=0x0400 WINVER>=0x0400

Windows Me 	_WIN32_WINDOWS=0x0500 WINVER>=0x0500
Windows 95 	_WIN32_WINDOWS>=0x0400 WINVER>=0x0400
Like this? I LXR'd for #define WINVER 0x0500 (and variations but finding none);

vlad told me not to worry about cairo, as he'll be updating it anyway.
Attachment #193798 - Attachment is obsolete: true
Attachment #193964 - Flags: review?(cls)
Comment on attachment 193964 [details] [diff] [review]
Fix all #define WINVER 0x0500

Not quite.  I agree that we should set WINVER/_WIN32_WINNT to 0x400 globally. 
However, I don't understand why it's ok to reset those variables to 0x500 for
certain components.  Doesn't that mean that those components which use 0x500
will not work on older systems?  (I'm obviously not a windows programmer.) It
seems like we'd be back to where we started.
Attachment #193964 - Flags: review?(cls) → review?(benjamin)
(In reply to comment #18)
>I don't understand why it's ok to reset those variables to 0x500 for
>certain components.
Microsoft, in their wisdom, wrote their headers so you could target one and only
one platform. This means that you can target Windows 95, but then you can't
access any Windows XP features, or you can target Windows XP, but then code
won't run on Windows 95 even if it doesn't use those features.
In order to work around this problem certain headers redefine the WINVER to get
access to the definitions required for newer features, although they then take
care to detect the availability of the feature so that they will still run on
earlier versions.
Comment on attachment 193964 [details] [diff] [review]
Fix all #define WINVER 0x0500

bizarre
Attachment #193964 - Flags: review?(benjamin) → review+
Comment on attachment 193964 [details] [diff] [review]
Fix all #define WINVER 0x0500

Need this for working branch suite installer.
Attachment #193964 - Flags: approval1.8b4?
Fix checked in to the trunk.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
neil, can you tell us whether or not this could possibly have any impact on the
aviary apps?
Yes, it can affect the aviary apps, and it probably ought to land on the branch
to fix cases where the aviary apps use the directory picker and other similar
features.
Attachment #193964 - Flags: approval1.8b4? → approval1.8b4+
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050830 SeaMonkey/1.1a]
(nightly) (W98SE)

V.Fixed: I checked both full and stub installers.
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.8beta4
Has this been checked in to the 1.8 branch?
(with apologies to Kenneth Wolstenholme) Some people are commenting on the
patch... they think it's checked in to the branch... it is now!
Keywords: fixed1.8
also verified on Windows Deer Park branch with Mozilla/5.0 (Windows; U; Windows
NT 5.1; en-US; rv:1.8b4) Gecko/20050901 Firefox/1.0+
Keywords: fixed1.8verified1.8
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: