RESOLVED FIXED 8251
navigator.platform incorrect in WebKit.app on Intel Macs
https://bugs.webkit.org/show_bug.cgi?id=8251
Summary navigator.platform incorrect in WebKit.app on Intel Macs
Justin Haygood
Reported 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".
Attachments
First attempt (1.33 KB, patch)
2006-07-04 12:57 PDT, Rob Buis
darin: review-
Improved patch (2.24 KB, patch)
2006-07-05 12:49 PDT, Rob Buis
darin: review-
Improved patch (1.65 KB, patch)
2006-07-08 11:01 PDT, Rob Buis
darin: review+
Rob Buis
Comment 1 2006-07-04 12:57:12 PDT
Created attachment 9194 [details] First attempt
Darin Adler
Comment 2 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?
Rob Buis
Comment 3 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.
Darin Adler
Comment 4 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.
Justin Haygood
Comment 5 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
Rob Buis
Comment 6 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.
Darin Adler
Comment 7 2006-07-08 19:56:10 PDT
Comment on attachment 9275 [details] Improved patch Great, lets land this. r=me
David Kilzer (:ddkilzer)
Comment 8 2006-07-09 04:04:24 PDT
Committed revision 15249.
Note You need to log in before you can comment on or make changes to this bug.