[webkit-reviews] review granted: [Bug 80128] [Qt] Move QStyle theming code out of WebCore into WebKit1 : [Attachment 129846] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Simon Hausmann
<hausmann at webkit.org>'s request for review:
Bug 80128: [Qt] Move QStyle theming code out of WebCore into WebKit1
https://bugs.webkit.org/show_bug.cgi?id=80128

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=129846&action=review


> Source/WebKit/qt/ChangeLog:9
> +	   Install the QStyle factory functions in initWebCoreQt.cpp.

InitWebCoreQt.cpp right

> Source/WebCore/platform/qt/RenderThemeQt.cpp:96
> +void RenderThemeQt::setCustomTheme(QtThemeFactoryFunction factory,
ScrollbarTheme *customScrollbarTheme)

* placement :-), nit though

> Source/WebCore/platform/qt/RenderThemeQt.cpp:142
> +    // If we are not using a custom theme but the "Mobile Qt" default theme,
then we also need
> +    // the extra stylesheet for it.
> +    if (!themeFactory) {

I think it is time we stop calling it Mobile theme :-) but I guess that is for
another time 

"When no theme factory is provided we default to using our platform independent
"Mobile Qt" theme, which required the following stylesheets.

> Source/WebKit/qt/WebCoreSupport/RenderThemeQStyle.cpp:4
> + * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)

time to update? I am sure we made changes after 2008 :-)

> Source/WebKit/qt/WebCoreSupport/RenderThemeQStyle.cpp:83
> +inline static void initStyleOption(QWidget *widget, QStyleOption& option)

* placement. Now you are moving the code it might be good to just fix these
things...

> Source/WebKit/qt/WebCoreSupport/RenderThemeQStyle.cpp:91
> +	   /*
> +	     If a widget is not directly available for rendering, we fallback
to default
> +	     value for an active widget.
> +	    */

like you real C++ comments //

> Source/WebKit/qt/WebCoreSupport/ScrollbarThemeQStyle.h:43
> +    virtual bool paint(ScrollbarThemeClient*, GraphicsContext*, const
IntRect& damageRect);

dirytRect?