RESOLVED FIXED 27351
[Qt] navigator.platform returns an empty string in Symbian and in Linux platforms 
https://bugs.webkit.org/show_bug.cgi?id=27351
Summary [Qt] navigator.platform returns an empty string in Symbian and in Linux platf...
Yael
Reported 2009-07-16 12:24:49 PDT
WEBCORE_NAVIGATOR_PLATFORM is not defined for Symbian and for Linux platforms. the following javascript would fail <html> <body > The name of the operating system of the current browser should be displayed below:<br> <script type="text/javascript"> document.write(navigator.platform+"<br/>"); </script> </body> </html>
Attachments
Patch (1.05 KB, patch)
2009-07-16 12:27 PDT, Yael 		staikos: review-
DetailsFormatted DiffDiff
Patch (1.72 KB, patch)
2009-07-16 19:43 PDT, Yael 		zecke: review-
DetailsFormatted DiffDiff
Patch (1.79 KB, patch)
2009-07-17 18:23 PDT, Yael 		staikos: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Yael
Comment 1 2009-07-16 12:27:27 PDT
Created attachment 32887 [details] Patch
George Staikos
Comment 2 2009-07-16 12:43:11 PDT
Comment on attachment 32887 [details] Patch No reason to believe that PLATFORM(LINUX) is "Linux i686"
Yael
Comment 3 2009-07-16 13:20:08 PDT
I think navigator.platform is very similar to what is in PluginViewQt.cpp line 275: static const char* MozillaUserAgent = "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0";
Yael
Comment 4 2009-07-16 19:43:23 PDT
Created attachment 32911 [details] Patch Add #define for Symbian platform. Use uname to determine the OS name and CPU , but limit it to Linux platform only. (I am not able to test other platforms).
Simon Hausmann
Comment 5 2009-07-17 06:55:27 PDT
Comment on attachment 32911 [details] Patch r=me. Seems consistent with firefox and chromium on linux, too.
Holger Freyther
Comment 6 2009-07-17 09:10:14 PDT
From my reading uname is POSIX.1 so we should guard it with something like PLATFORM(UNIX).
Yael
Comment 7 2009-07-17 09:30:10 PDT
(In reply to comment #6) > From my reading uname is POSIX.1 so we should guard it with something like > PLATFORM(UNIX). I wanted to be on the safe side, so I limited this to Linux based platforms. Some types of Unix might not be 100% POSIX.1 compatible, and I did not want to take the risk.
Jan Alonzo
Comment 8 2009-07-17 14:00:46 PDT
Comment on attachment 32911 [details] Patch > String NavigatorBase::platform() const > { > +#if PLATFORM(LINUX) > + struct utsname osname; > + int result = uname(&osname); > + String platformName; > + if (result >= 0) { > + platformName = osname.sysname; > + platformName += " "; > + platformName += osname.machine; > + } > + return platformName; Can we please make use of DEFINE_STATIC_LOCAL here for platformName?
Holger Freyther
Comment 9 2009-07-17 18:00:49 PDT
Comment on attachment 32911 [details] Patch > > String NavigatorBase::platform() const > { > +#if PLATFORM(LINUX) > + struct utsname osname; > + int result = uname(&osname); > + String platformName; > + if (result >= 0) { > + platformName = osname.sysname; > + platformName += " "; > + platformName += osname.machine; > + } > + return platformName; > +#else > return WEBCORE_NAVIGATOR_PLATFORM; > +#endif Jan is right, we should call uname(&osname) exactly once. Yael could you post an updated version of the patch?
Yael
Comment 10 2009-07-17 18:23:32 PDT
Created attachment 32996 [details] Patch Use DEFINE_STATIC_LOCAL as requested in comment #8.
Yael
Comment 11 2009-07-17 19:09:51 PDT
landed in 46078
Note You need to log in before you can comment on or make changes to this bug.