Bug 52836 – [GTK] Move scrollbar rendering out of gtk2drawing.c

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

This bug tracks moving scrollbar rendering out of gtk2drawing.c. This is the final piece of the Mozilla theme drawing code that we use, so we can remove these files completely once this bug is resolved.

[Martin Robinson]

Created attachment 79644 [details]
Patch

[Martin Robinson]

Comment on attachment 79644 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79644&action=review

> Source/WebCore/platform/gtk/ScrollbarThemeGtk2.cpp:58
> +                         "slider_width", &m_thumbFatness,
> +                         "trough_border", &m_troughBorderWidth,

I will change the underscores here to dashes before landing.

[Eric Seidel (no email)]

would be nice to see the gtk ews results.

[Martin Robinson]

(In reply to comment #3)
> would be nice to see the gtk ews results.

I can repost it after the blocking bug has landed.

[Martin Robinson]

Created attachment 79787 [details]
Patch

[WebKit Review Bot]

Attachment 79787 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/gtk/ScrollbarThemeGtk2.cpp:62:  Use 0 instead of NULL.  [readability/null] [5]
Source/WebCore/platform/gtk/ScrollbarThemeGtk2.cpp:198:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gustavo Noronha (kov)]

Comment on attachment 79787 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79787&action=review

> Source/WebCore/platform/gtk/ScrollbarThemeGtk2.cpp:112
> +    // properly the theme may draw not draw the thumb borders properly.

s/draw not/not/, right?

> Source/WebCore/platform/gtk/ScrollbarThemeGtk2.cpp:186
> +    gfloat arrowScaling;

Xan does not approve of using g* types when they're just typedefs of the standard C ones.

> Source/WebCore/platform/gtk/WidgetRenderingContext.cpp:155
> +void WidgetRenderingContext::gtkPaintBox(const IntRect& rect, GtkWidget* widget, GtkStateType stateType, GtkShadowType shadowType, const gchar* detail, bool adjustAllocation)

Would it hurt to remove the bool and always perform the allocation? I mean, except for the (negligible?) extra work? It might make this more future-proof, and we can toss this additional argument.

[Martin Robinson]

Committed r77960: <http://trac.webkit.org/changeset/77960>

[Martin Robinson]

Comment on attachment 79787 [details]
Patch

Thanks for the review! Landed with your suggestions.