Bug 27351

Summary:

[Qt] navigator.platform returns an empty string in Symbian and in Linux platforms

Product:

WebKit

Reporter:

Yael <yael>

Component:

WebKit Qt

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

zecke

Priority:

Version:

528+ (Nightly build)

Hardware:

S60 Emulator

OS:

Other

Attachments:

Description	Flags
Patch	staikos: review-
Patch	zecke: review-
Patch	staikos: review+

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-	Details Formatted Diff Diff
Patch (1.72 KB, patch) 2009-07-16 19:43 PDT, Yael	zecke: review-	Details Formatted Diff Diff
Patch (1.79 KB, patch) 2009-07-17 18:23 PDT, Yael	staikos: review+	Details Formatted Diff Diff
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.