Mozilla doesn't start on Windows 95

RESOLVED FIXED in mozilla1.2beta

Status

()

--
critical
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: neil, Assigned: tetsuroy)

Tracking

Trunk
mozilla1.2beta
x86
Windows 95
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

16 years ago
Last known working build ID: 2002092304
Known non-working build ID: 2002092506

This bug was probably exposed by bug 104934.

SHBrowserForFolderW and SHGetPathFromIDListW don't exist on Windows 95.
(Reporter)

Comment 1

16 years ago
Oops, typo, meant SHBrowseForFolderW
(Assignee)

Comment 2

16 years ago
wow.  Win95 platform!! It's 7 yrs old OS.  
neil: I think I can make a patch to fix it; but none of us have Win95 for
      dev/testing.  Would you be able to test my patch once available?

Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2beta
(Reporter)

Comment 3

16 years ago
I can only download a .ZIP build for testing.
(Assignee)

Comment 4

16 years ago
neil: ok.  I found a qa Win95 machine.  I believe we can test the patch here. :)
      thanks. 
do we still support win95?  I don't even think that MSFT does.

the only reason I ask is there will probably be more changes down the road that 
aren't going to work for win95.

Comment 6

16 years ago
this one is easily fixable. unicode on w95 is supported by microsoft unicode for
windows, so i can't think of anything which would require us to break w95 compat.
(Assignee)

Comment 7

16 years ago
Created attachment 100786 [details] [diff] [review]
Create nsToolkit helper functions

Comment 8

16 years ago
We support what our development community is willing to maintain. If there are
no developers willing to keep us working on win95 then we die on win95. 
That being said, we work pretty well on win95 and I don't think we should let
any easily fixed bugs kill that platform. 
(Reporter)

Comment 9

16 years ago
> wow.  Win95 platform!! It's 7 yrs old OS

> do we still support win95?

Well, Unicode support on 9x is patchy - sometimes (as in this case) the function
doesn't exist (perhaps I need to install IE but that defeats the object :-) ),
while sometimes the function exists but doesn't work. Mozilla even almost works
on Windows NT 3.51 - the only unsupported call Mozilla uses is
SystemParametersInfo( SPI_GETNONCLIENTMETRICS, ... )

I really appreciate your taking the time to fix this.
(Reporter)

Comment 10

16 years ago
It it me or has "Attachments" > "Save All" stopped working in Windows 98?

Comment 11

16 years ago
FWIW build 2002093004 works on win95 since the patch to bug 104934 was backed out.
(Assignee)

Comment 12

16 years ago
Created attachment 102608 [details] [diff] [review]
implicit calls of SHFooW()

Need to add 
nsToolkit::mSHGetPathFromIDList = &nsSHGetPathFromIDList; 
nsToolkit::mSHBrowseForFolder = &nsSHBrowseForFolde;
to the 104934 attachment(already /r and /sr)

One unique thing about this is that we need to get pointer of
SHFooW() by implicitly calling GetProcAdress()
(NS_SHGetPathFromIDList)GetProcAddress(module, "SHGetPathFromIDListW"); 
(NS_SHBrowseForFolder)GetProcAddress(module, "SHBrowseForFolderW"); 

need review.
Attachment #100786 - Attachment is obsolete: true
(Assignee)

Comment 13

16 years ago
shanjian: can you review?

Comment 14

16 years ago
Comment on attachment 102608 [details] [diff] [review]
implicit calls of SHFooW()

Looks fine. r=shanjian
Attachment #102608 - Flags: review+
(Assignee)

Comment 15

16 years ago
kin: can you super review?  

This is yet another nsToolkit:: prefix stuff.
As much as you dislike the ifdef's,
http://bugzilla.mozilla.org/show_bug.cgi?id=171228#c9 , 
I would have to ask you for /sr.
#ifdef's will eventually be removed from the tree, soon. :) 

Comment 16

16 years ago
Comment on attachment 102608 [details] [diff] [review]
implicit calls of SHFooW()

==== Can this ever return null or fail? If so, what's our fallback so we don't
call through a null pointer in nsFilePicker.cpp?


+	 nsToolkit::mSHGetPathFromIDList =
(NS_SHGetPathFromIDList)GetProcAddress(module, "SHGetPathFromIDListW"); 
+	 nsToolkit::mSHBrowseForFolder =
(NS_SHBrowseForFolder)GetProcAddress(module, "SHBrowseForFolderW");
(Assignee)

Comment 17

16 years ago
>Can this ever return null or fail?
I thought of the same. It should never be null in NT base system. 
I have wrapped the call with (nsToolkit::mIsNT) && (module) 
just to be safe. If these conditions fail, then they fall 
back to A version.

I am not sure about non-Microsoft supported Windows
simulation though (eg. WINE in Linux, etc); but who would want
to run moz for Windows in Linux when we have the native
mozilla for Linux......

I can add ASSERTION as follow:
+  if (!nsToolkit::mDefWindowProc || 
+    !nsToolkit::mCallWindowProc ||
+    !nsToolkit::mSetWindowLong ||
+    !nsToolkit::mSendMessage ||
+    !nsToolkit::mDispatchMessage ||
+    !nsToolkit::mGetMessage ||
+    !nsToolkit::mPeekMessage ||
+    !nsToolkit::mGetOpenFileName ||
+    !nsToolkit::mGetSaveFileName ||
+    !nsToolkit::mGetClassName ||
+    !nsToolkit::mCreateWindowEx ||
+    !nsToolkit::mRegisterClass ||
+    !nsToolkit::mSHGetPathFromIDList ||
+    !nsToolkit::mSHBrowseForFolder) {
+      NS_ASSERTION(0, "Wide API doesn't exist");
+  }    
(Reporter)

Comment 18

16 years ago
Surely
+  NS_ASSERTION(
+    nsToolkit::mDefWindowProc &&
+    nsToolkit::mCallWindowProc &&
+    nsToolkit::mSetWindowLong &&
+    nsToolkit::mSendMessage &&
+    nsToolkit::mDispatchMessage &&
+    nsToolkit::mGetMessage &&
+    nsToolkit::mPeekMessage &&
+    nsToolkit::mGetOpenFileName &&
+    nsToolkit::mGetSaveFileName &&
+    nsToolkit::mGetClassName &&
+    nsToolkit::mCreateWindowEx &&
+    nsToolkit::mRegisterClass &&
+    nsToolkit::mSHGetPathFromIDList &&
+    nsToolkit::mSHBrowseForFolder,
+    "Wide API doesn't exist");
This saves code in release builds.
(Assignee)

Comment 19

16 years ago
I'll take neil's suggestion. :)
kin: good for /sr?

Comment 20

16 years ago
Comment on attachment 102608 [details] [diff] [review]
implicit calls of SHFooW()

==== I don't think it's necessary to check all the non mSH* pointers with your
assertion given the fact that the *W symbols have to be defined before the app
will even start up/link. It's the dynamic result of GetProcAddress() that
worries me and the fact that we assign the values directly into the mSH*
pointers, so we can't even fallback on the nsSH* versions we have. Would it be
better to assign into temporaries first? Or do something like this so we can
fallback on the nsSH* versions they are originally set to?

+	 nsToolkit::mSHGetPathFromIDList =
(NS_SHGetPathFromIDList)GetProcAddress(module, "SHGetPathFromIDListW"); 
+	 if (!nsToolkit::mSHGetPathFromIDList)
+	   nsToolkit::mSHGetPathFromIDList = &nsSHGetPathFromIDList;
+	   
+	 nsToolkit::mSHBrowseForFolder =
(NS_SHBrowseForFolder)GetProcAddress(module, "SHBrowseForFolderW");
+	 if (!nsToolkit::mSHBrowseForFolder)
+	   nsToolkit::mSHBrowseForFolder = &nsSHBrowseForFolder;

==== By the way, you might already know this, but I don't think it's necessary
to use '&' in front of the function names when getting their pointers.


sr=kin@netscape.com
Attachment #102608 - Flags: superreview+
(Assignee)

Comment 21

16 years ago
I'll add 
+	 nsToolkit::mSHGetPathFromIDList =
(NS_SHGetPathFromIDList)GetProcAddress(module, "SHGetPathFromIDListW"); 
+	 if (!nsToolkit::mSHGetPathFromIDList)
+	   nsToolkit::mSHGetPathFromIDList = &nsSHGetPathFromIDList;
+	   
+	 nsToolkit::mSHBrowseForFolder =
(NS_SHBrowseForFolder)GetProcAddress(module, "SHBrowseForFolderW");
+	 if (!nsToolkit::mSHBrowseForFolder)
+	   nsToolkit::mSHBrowseForFolder = &nsSHBrowseForFolder;

and remove & in front of the function names.

Comment 22

16 years ago
Comment on attachment 102608 [details] [diff] [review]
implicit calls of SHFooW()

a=asa for checkin to 1.2beta (on behalf of drivers)
Attachment #102608 - Flags: approval+
(Assignee)

Comment 23

16 years ago
fix checked into the trunk
Please verify once we turn the MOZ_UNICODE flag on by default
or provide a special unicode build (whichever comes first)
Making 104934 as a depend.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Depends on: 104934
Resolution: --- → FIXED

Comment 24

16 years ago
Tested with trunk 101815-unicode build on EN, JA and SC Win95OSR2, works fine.

Comment 25

16 years ago
Latest nightly (build 2002102510 probably, cannot check since it does not start)
shows exactly same behaviour as the builds hit with this bug.
(Assignee)

Comment 26

16 years ago
Torben: We haven't turn the Unicode flag for the regular trunk build just yet.  
        Our iQAs were testing against the special Unicode build 
        which is available from 
       
http://ftp.mozilla.org/pub/mozilla/nightly/2002-10-23-08-unicode/mozilla-win32.zip 
        Which build are you testing with?

Comment 27

16 years ago
Re #26:
This was a trunk build, probably 2002102508-trunk judging by the modification
time. If unicode is not turned on yet I guess it is another bug (sigh...). I
will test more on monday and fill a new bug if necessary.

Comment 28

16 years ago
Filled bug 177083 to adress the new crashes noted in comment 25. Unicode builds
2002102308 and 2002102408 works fine on win 95.
(Assignee)

Comment 29

16 years ago
Thanks, Torben
You need to log in before you can comment on or make changes to this bug.