Bug 52836 - [GTK] Move scrollbar rendering out of gtk2drawing.c
Summary: [GTK] Move scrollbar rendering out of gtk2drawing.c
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on: 51155
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-20 13:01 PST by Martin Robinson
Modified: 2011-02-08 11:16 PST (History)
5 users (show)

See Also:


Attachments
Patch (20.16 KB, patch)
2011-01-20 13:26 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (20.15 KB, patch)
2011-01-21 14:00 PST, Martin Robinson
no flags Details | Formatted Diff | Diff

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