Bug 8251 - navigator.platform incorrect in WebKit.app on Intel Macs
Summary: navigator.platform incorrect in WebKit.app on Intel Macs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 420+
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-04-07 13:02 PDT by Justin Haygood
Modified: 2006-07-09 04:14 PDT (History)
1 user (show)

See Also:


Attachments
First attempt (1.33 KB, patch)
2006-07-04 12:57 PDT, Rob Buis
darin: review-
Details | Formatted Diff | Diff
Improved patch (2.24 KB, patch)
2006-07-05 12:49 PDT, Rob Buis
darin: review-
Details | Formatted Diff | Diff
Improved patch (1.65 KB, patch)
2006-07-08 11:01 PDT, Rob Buis
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Haygood 2006-04-07 13:02:12 PDT
reports "MacPPC", should say "MacIntel".

The WebKit that ships with OS X Tiger 10.4.6 reports it correctly as "MacIntel".
Comment 1 Rob Buis 2006-07-04 12:57:12 PDT
Created attachment 9194 [details]
First attempt
Comment 2 Darin Adler 2006-07-04 14:41:47 PDT
Comment on attachment 9194 [details]
First attempt

Seems fine to do this, but:

    1) We should be taking advantage of the <wtf/Platform.h> header instead of adding still more one-off ifdefs.

    2) A name of format _PLATFORM is reserved for the standard C library. Don't use leading underscores for this sort of thing. How about WEBCORE_NAVIGATOR_PLATFORM or something like that?
Comment 3 Rob Buis 2006-07-05 12:49:30 PDT
Created attachment 9213 [details]
Improved patch

Renamed to WEBCORE_NAVIGATOR_PLATFORM, reused CPU macro's and moved to wtf. I am not sure
whether WEBCORE_NAVIGATOR_PLATFORM is useful anywhere outside kjs_navigator
though...
Cheers,

Rob.
Comment 4 Darin Adler 2006-07-08 06:28:25 PDT
Comment on attachment 9213 [details]
Improved patch

Sorry to be difficult, but there is at least one problem here. If the macro is going to be in Platform.h, then the prefix should not be WEBCORE_ but rather WTF_. But also, if it's going to be in Platform.h it has to be the whole macro, not just the PPC part, with the WIN32 bit etc.

I think we should go back to something more like the original patch that's in kjs_navigator.cpp, but not with something that starts with an underscore. I think that you should use PLATFORM(PPC) instead of a custom ifdef. And I think that all the platforms should use the macro instead of having ifdefs in the Platform case in the casae statement.
Comment 5 Justin Haygood 2006-07-08 09:35:11 PDT
I would use something like:

#if PLATFORM(MAC_OS) && PLATFORM(PPC)
return jsString("MacPPC");
#elif PLATFORM(MAC_OS)  && PLATFORM(X86)
return jsString("MacIntel");
#elif PLATFORM(WIN_OS)
return jsString("Win32");
#endif
Comment 6 Rob Buis 2006-07-08 11:01:48 PDT
Created attachment 9275 [details]
Improved patch

Hi Darin, Justin,

I interpreted the suggestions a bit wrong, I thought the macro should go into Platform.h, you
probably meant just using Platform.h macros. This time I tried to merge both your suggestions. Let
me know if a testcase is required (I tested using http://www.robinlionheart.com/stds/html4/objects)
and if so, how to go about the multi-platform aspect.
Cheers,

Rob.
Comment 7 Darin Adler 2006-07-08 19:56:10 PDT
Comment on attachment 9275 [details]
Improved patch

Great, lets land this. r=me
Comment 8 David Kilzer (:ddkilzer) 2006-07-09 04:04:24 PDT
Committed revision 15249.