Summary: | [Qt] Use Windows style on Maemo 5 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||||
Component: | WebKit Qt | Assignee: | 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
Andreas Kling
2010-01-30 08:56:08 PST
Created attachment 47769 [details]
Patch
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();' ? Because of: ../../../WebCore/platform/qt/RenderThemeQt.cpp:165: error: passing 'const WebCore::RenderThemeQt' as 'this' argument of 'QStyle* WebCore::RenderThemeQt::fallbackStyle()' discards qualifiers (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 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. Created attachment 47831 [details]
New patch, raised issues addressed.
Comment on attachment 47831 [details]
New patch, raised issues addressed.
Please remove the #endif and change the #else to become #endif, both places.
Created attachment 47832 [details]
Same patch, less cruft
Comment on attachment 47832 [details] Same patch, less cruft Clearing flags on attachment: 47832 Committed r54180: <http://trac.webkit.org/changeset/54180> All reviewed patches have been landed. Closing bug. (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 |