WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
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
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug