Bug 31961

Summary: [Qt][Symbian] Report SymbianOS in user agent string for Symbian
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, hausmann
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: PC   
OS: S60 3rd edition   
Bug Depends on:    
Bug Blocks: 27065    
Attachments:
Description Flags
1st try
hausmann: review-
2nd try none

Laszlo Gombos
Reported 2009-11-29 12:46:34 PST
In general the guideline is that we want a user agent string that is consistent with (but distinguishable from) previous S60 WebKit ports. In addition we want a user agent string that allows client side user agent detection (http://www.quirksmode.org/js/detect.html is probably one good test). I believe Abhinav Mithal has a patch for this - I contacted him to see if he can upload his proposed changes.
Attachments
1st try (3.08 KB, patch)
2009-11-29 13:15 PST, Laszlo Gombos 		hausmann: review-
DetailsFormatted DiffDiff
2nd try (3.08 KB, patch)
2009-11-29 19:27 PST, Laszlo Gombos 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Laszlo Gombos
Comment 1 2009-11-29 13:15:29 PST
Created attachment 43991 [details] 1st try Abhinav is out for the rest of the year, I uploading his patch on his behalf.
Simon Hausmann
Comment 2 2009-11-29 15:53:41 PST
Comment on attachment 43991 [details] 1st try In principle I think this patch looks good, but there are a few style issues: > + // Placeholder for SubPlatfrom Version from -> form > + "%4; "); > + > +// Platform Version This should be indented 4 spaces > + QString osVer; > +#ifdef Q_OS_SYMBIAN > + QSysInfo::SymbianVersion symbianVersion = QSysInfo::symbianVersion(); > +switch (symbianVersion) { This line is also missing indentation. > + case QSysInfo::SV_9_2: > + osVer = "/9.2"; > + break; > + case QSysInfo::SV_9_3: > + osVer = "/9.3"; > + break; > + case QSysInfo::SV_9_4: > + osVer = "/9.4"; > + break; > + default: > + osVer = "Unknown"; > + } > +#else > + osVer = ""; > +#endif I suggest to omit the #else block that assigns an empty string to the string that is already a null string. > +// SubPlatform Version Indentation > + QString subPlatformVer; > +#ifdef Q_OS_SYMBIAN > + QSysInfo::S60Version s60Version = QSysInfo::s60Version(); > +switch (s60Version) { Indentation > + case QSysInfo::SV_S60_3_1: > + subPlatformVer = "/3.1"; > + break; > + case QSysInfo::SV_S60_3_2: > + subPlatformVer = "/3.2"; > + break; > + case QSysInfo::SV_S60_5_0: > + subPlatformVer = "/5.0"; > + break; > + default: > + subPlatformVer = " Unknown"; > + } > +#else > + subPlatformVer = ""; > +#endif Same as above, the assignment is unnecessary.
Laszlo Gombos
Comment 3 2009-11-29 19:27:33 PST
Created attachment 44005 [details] 2nd try Incorporate Simon's review comments.
Adam Barth
Comment 4 2009-11-30 12:48:55 PST
style-queue ran check-webkit-style on attachment 44005 [details] without any errors.
WebKit Commit Bot
Comment 5 2009-11-30 15:20:11 PST
Comment on attachment 44005 [details] 2nd try Clearing flags on attachment: 44005 Committed r51515: <http://trac.webkit.org/changeset/51515>
WebKit Commit Bot
Comment 6 2009-11-30 15:20:16 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.