[webkit-reviews] review denied: [Bug 48636] [Qt] Mobile Devices should include Model and Firmware Version in Webkit Generated User Agent String : [Attachment 72824] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 16 15:30:13 PST 2010


Laszlo Gombos <laszlo.1.gombos at nokia.com> has denied Stanislav Paltis
<Stanislav.Paltis at nokia.com>'s request for review:
Bug 48636: [Qt] Mobile Devices should include Model and Firmware Version in
Webkit Generated User Agent String
https://bugs.webkit.org/show_bug.cgi?id=48636

Attachment 72824: Patch
https://bugs.webkit.org/attachment.cgi?id=72824&action=review

------- Additional Comments from Laszlo Gombos <laszlo.1.gombos at nokia.com>
This patch has been only tested on Symbian and it seems that this really only
works well on Symbian. We should either guard the code with SYMBIAN (which I do
not recommend) or consider other platforms as well. 

On Linux desktop (QtMobility installed) the patch results in the following
User-Agent string: "Mozilla/5.0 (X11; N; Linux i686i686
Other/2.6.32-24-generic; en-US) ...". 
Notice that the CPU architecture (i686) is reported twice and a space seems to
be missing.

In general for most OSs I would suggest to test if QT_SYSTEMINFO is available
and if it is, than we should report the values returned by QtMobility
(runtime). As a fall-back we can have compile time flags in QtWebKit (as we
have now) when Qt_MOBILITY is not available. 

In general we should keep the User-Agent small and simple and not report
information just because it is available. 

I'm not sure if we really want to report QtMobility::QSystemInfo::Firmware,
however I think reporting the model or parts of the model make sense.

r- as non-Symbian platforms needs to be addressed as well.


More information about the webkit-reviews mailing list