Bug 34376 - [Qt] Use Windows style on Maemo 5
Summary: [Qt] Use Windows style on Maemo 5
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-30 08:56 PST by Andreas Kling
Modified: 2010-02-02 03:14 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.91 KB, patch)
2010-01-30 08:57 PST, Andreas Kling
ariya.hidayat: review-
Details | Formatted Diff | Diff
New patch, raised issues addressed. (3.43 KB, patch)
2010-02-01 05:41 PST, Andreas Kling
kenneth: review-
Details | Formatted Diff | Diff
Same patch, less cruft (2.87 KB, patch)
2010-02-01 05:55 PST, Andreas Kling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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