Bug 8251 – navigator.platform incorrect in WebKit.app on Intel Macs

• Component: New Bugs
• Status: RESOLVED FIXED

[Justin Haygood]

reports "MacPPC", should say "MacIntel".

The WebKit that ships with OS X Tiger 10.4.6 reports it correctly as "MacIntel".

[Rob Buis]

Created attachment 9194 [details]
First attempt

[Darin Adler]

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?

[Rob Buis]

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.

[Darin Adler]

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.

[Justin Haygood]

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

[Rob Buis]

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.

[Darin Adler]

Comment on attachment 9275 [details]
Improved patch

Great, lets land this. r=me

[David Kilzer (:ddkilzer)]

Committed revision 15249.