Bug 34376 – [Qt] Use Windows style on Maemo 5

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Andreas Kling]

This is a stopgap measure to stop QtWebKit from looking silly on Maemo.

[Andreas Kling]

Created attachment 47769 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 47769 [details]
Patch


>  QStyle* RenderThemeQt::qStyle() const
>  {
> +#ifdef Q_WS_MAEMO_5
> +    return const_cast<RenderThemeQt*>(this)->fallbackStyle();
> +#else

Why not just 'return fallbackStyle();' ?

[Andreas Kling]

Because of:
../../../WebCore/platform/qt/RenderThemeQt.cpp:165: error: passing 'const WebCore::RenderThemeQt' as 'this' argument of 'QStyle* WebCore::RenderThemeQt::fallbackStyle()' discards qualifiers

[Kenneth Rohde Christiansen]

(In reply to comment #3)
> Because of:
> ../../../WebCore/platform/qt/RenderThemeQt.cpp:165: error: passing 'const
> WebCore::RenderThemeQt' as 'this' argument of 'QStyle*
> WebCore::RenderThemeQt::fallbackStyle()' discards qualifiers

Yes, but fallbackStyle and qStyle are things of the Qt port. The thing is that fallbackStyle is not really const. So, with you patch you are "lying" that qStyle is :-)

You could either change qStyle to not be const anymore, or - if the fallback style doesn't change at runtime - move the fallback construction code to the constructor and make both methods const.

I think that I would add
m_fallbackStyle = QStyleFactory::create(QLatin1String("windows"));
to the constructor. We need that anyway for painting menu list buttons

and change fallbackStyle to:

// for some widget painting, we need to fallback to Windows style
QStyle* RenderThemeQt::fallbackStyle() const
{
    return (m_fallbackStyle) ? m_fallbackStyle : QApplication::style();
}

[Ariya Hidayat]

Comment on attachment 47769 [details]
Patch

> +        [Qt] Use Windows style on Maemo 5

Better to refer it as "Use the fallback style...". Note that the fallback style could be the application style if the Windows style can't be created (e.g. the style plugin for that is missing or not available).

>  void RenderThemeQt::setPaletteFromPageClientIfExists(QPalette& palette) const
>  {
> +#ifdef Q_WS_MAEMO_5
> +    static QPalette lightGrayPalette(Qt::lightGray);
> +    palette = lightGrayPalette;
> +#else
>      // If the webview has a custom palette, use it
>      if (!m_page)
>          return;
> @@ -786,6 +795,7 @@ void RenderThemeQt::setPaletteFromPageClientIfExists(QPalette& palette) const
>      if (!pageClient)
>          return;
>      palette = pageClient->palette();
> +#endif

Why don't we have a quick return like in qStyle() patch above? That way, the alternative code path is easier to spot.

r- for these minor issues, otherwise LGTM.

[Andreas Kling]

Created attachment 47831 [details]
New patch, raised issues addressed.

[Kenneth Rohde Christiansen]

Comment on attachment 47831 [details]
New patch, raised issues addressed.

Please remove the #endif and change the #else to become #endif, both places.

[Andreas Kling]

Created attachment 47832 [details]
Same patch, less cruft

[WebKit Commit Bot]

Comment on attachment 47832 [details]
Same patch, less cruft

Clearing flags on attachment: 47832

Committed r54180: <http://trac.webkit.org/changeset/54180>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

(In reply to comment #9)
> (From update of attachment 47832 [details])
> Clearing flags on attachment: 47832
> 
> Committed r54180: <http://trac.webkit.org/changeset/54180>

Manually cherry-picked with commit 69dd29fbeb12d076741dce70ac6bc155101ccd6f into qtwebkit-4.6