Bug 52836

Summary: [GTK] Move scrollbar rendering out of gtk2drawing.c
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, eric, gns, webkit.review.bot, xan.lopez
Priority: P3 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 51155    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Description Martin Robinson 2011-01-20 13:01:24 PST
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.
Comment 1 Martin Robinson 2011-01-20 13:26:13 PST
Created attachment 79644 [details]
Patch
Comment 2 Martin Robinson 2011-01-20 15:28:37 PST
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.
Comment 3 Eric Seidel (no email) 2011-01-21 02:57:49 PST
would be nice to see the gtk ews results.
Comment 4 Martin Robinson 2011-01-21 09:06:48 PST
(In reply to comment #3)
> would be nice to see the gtk ews results.

I can repost it after the blocking bug has landed.
Comment 5 Martin Robinson 2011-01-21 14:00:19 PST
Created attachment 79787 [details]
Patch
Comment 6 WebKit Review Bot 2011-01-21 14:02:07 PST
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.
Comment 7 Gustavo Noronha (kov) 2011-02-07 10:46:15 PST
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.
Comment 8 Martin Robinson 2011-02-08 11:15:47 PST
Committed r77960: <http://trac.webkit.org/changeset/77960>
Comment 9 Martin Robinson 2011-02-08 11:16:45 PST
Comment on attachment 79787 [details]
Patch

Thanks for the review! Landed with your suggestions.