Bug 34376

Summary: [Qt] Use Windows style on Maemo 5
Product: WebKit Reporter: Andreas Kling <kling>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hausmann, kenneth
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
ariya.hidayat: review-
New patch, raised issues addressed.
kenneth: review-
Same patch, less cruft none

Description Andreas Kling 2010-01-30 08:56:08 PST
This is a stopgap measure to stop QtWebKit from looking silly on Maemo.
Comment 1 Andreas Kling 2010-01-30 08:57:26 PST
Created attachment 47769 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2010-01-30 15:22:59 PST
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();' ?
Comment 3 Andreas Kling 2010-01-30 17:13:01 PST
Because of:
../../../WebCore/platform/qt/RenderThemeQt.cpp:165: error: passing 'const WebCore::RenderThemeQt' as 'this' argument of 'QStyle* WebCore::RenderThemeQt::fallbackStyle()' discards qualifiers
Comment 4 Kenneth Rohde Christiansen 2010-01-31 07:09:21 PST
(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();
}
Comment 5 Ariya Hidayat 2010-01-31 15:45:12 PST
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.
Comment 6 Andreas Kling 2010-02-01 05:41:12 PST
Created attachment 47831 [details]
New patch, raised issues addressed.
Comment 7 Kenneth Rohde Christiansen 2010-02-01 05:46:49 PST
Comment on attachment 47831 [details]
New patch, raised issues addressed.

Please remove the #endif and change the #else to become #endif, both places.
Comment 8 Andreas Kling 2010-02-01 05:55:01 PST
Created attachment 47832 [details]
Same patch, less cruft
Comment 9 WebKit Commit Bot 2010-02-01 17:16:56 PST
Comment on attachment 47832 [details]
Same patch, less cruft

Clearing flags on attachment: 47832

Committed r54180: <http://trac.webkit.org/changeset/54180>
Comment 10 WebKit Commit Bot 2010-02-01 17:17:03 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Simon Hausmann 2010-02-02 03:14:16 PST
(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