[webkit-reviews] review granted: [Bug 73917] [Qt][WK2] fast/forms/access-key-for-all-elements.html fails : [Attachment 163865] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Lauro Moura Maranhao Neto
<lauro.neto at openbossa.org>'s request for review:
Bug 73917: [Qt][WK2] fast/forms/access-key-for-all-elements.html fails
https://bugs.webkit.org/show_bug.cgi?id=73917

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

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=163865&action=review


> Source/WebCore/platform/qt/UserAgentQt.cpp:74
> +#ifdef Q_WS_MAC
> +	       "Macintosh; "
> +#elif defined Q_WS_QWS
> +	       "QtEmbedded; "
> +#elif defined Q_WS_WIN
> +	       // Nothing.
> +#elif defined Q_WS_X11
> +	       "X11; "
> +#else
> +	       "Unknown; "
> +#endif

As a side-note, those Q_WS_* defines don't work anymore :)

> Source/WebCore/platform/qt/UserAgentQt.cpp:155
> +#ifdef Q_OS_AIX
> +	       "AIX"
> +#elif defined Q_OS_WIN32
> +	       windowsVersionForUAString()
> +#elif defined Q_OS_DARWIN
> +#ifdef __i386__ || __x86_64__
> +	       "Intel Mac OS X"
> +#else
> +	       "PPC Mac OS X"
> +#endif
> +
> +#elif defined Q_OS_BSDI
> +	       "BSD"
> +#elif defined Q_OS_BSD4
> +	       "BSD Four"
> +#elif defined Q_OS_CYGWIN
> +	       "Cygwin"
> +#elif defined Q_OS_DGUX
> +	       "DG/UX"
> +#elif defined Q_OS_DYNIX
> +	       "DYNIX/ptx"
> +#elif defined Q_OS_FREEBSD
> +	       "FreeBSD"
> +#elif defined Q_OS_HPUX
> +	       "HP-UX"
> +#elif defined Q_OS_HURD
> +	       "GNU Hurd"
> +#elif defined Q_OS_IRIX
> +	       "SGI Irix"
> +#elif defined Q_OS_LINUX
> +
> +#if defined(__x86_64__)
> +	       "Linux x86_64"
> +#elif defined(__i386__)
> +	       "Linux i686"
> +#else
> +	       "Linux"
> +#endif
> +
> +#elif defined Q_OS_LYNX
> +	       "LynxOS"
> +#elif defined Q_OS_NETBSD
> +	       "NetBSD"
> +#elif defined Q_OS_OS2
> +	       "OS/2"
> +#elif defined Q_OS_OPENBSD
> +	       "OpenBSD"
> +#elif defined Q_OS_OS2EMX
> +	       "OS/2"
> +#elif defined Q_OS_OSF
> +	       "HP Tru64 UNIX"
> +#elif defined Q_OS_QNX6
> +	       "QNX RTP Six"
> +#elif defined Q_OS_QNX
> +	       "QNX"
> +#elif defined Q_OS_RELIANT
> +	       "Reliant UNIX"
> +#elif defined Q_OS_SCO
> +	       "SCO OpenServer"
> +#elif defined Q_OS_SOLARIS
> +	       "Sun Solaris"
> +#elif defined Q_OS_ULTRIX
> +	       "DEC Ultrix"
> +#elif defined Q_OS_UNIX
> +	       "UNIX BSD/SYSV system"
> +#elif defined Q_OS_UNIXWARE
> +	       "UnixWare Seven, Open UNIX Eight"
> +#else
> +	       "Unknown"
> +#endif

We can also probably clean up this list, some of those OSes are definitely not
supported anymore by Qt.

> Source/WebCore/platform/qt/UserAgentQt.cpp:161
> +	   QString version =
QString(QLatin1String("%1.%2")).arg(QString::number(webkitMajorVersion),
> +								 
QString::number(webkitMinorVersion));

This is also worth cleaing up in a separate patch, maybe using StringBuilder
instead of QString(QLatin1String()).arg() just to insert a dot between two
QStrings :)