Bug 51830 – [GTK] Use GtkStyleContext to get platform colors

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

[Carlos Garcia Campos]

This depends on bug #51764 since the patch for stock icons contains the initial GtkStyleContext port

[Carlos Garcia Campos]

Created attachment 77821 [details]
Use GtkStyleContext API to get platform colors

[Martin Robinson]

Comment on attachment 77821 [details]
Use GtkStyleContext API to get platform colors

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

Looks great. I have one suggested improvement below.

> WebCore/platform/gtk/RenderThemeGtk3.cpp:716
> +static Color getStyleColor(GType widgetType, bool background, GtkStateFlags flags)

It think it's okay to use a bool here since this helper is so localized, but using an enum as an argument type is generally preferred.

> WebCore/platform/gtk/RenderThemeGtk3.cpp:731
> +    gdkColor.pixel = 0;
> +    gdkColor.red = (guint16) (gdkRGBAColor.red * 65535);
> +    gdkColor.green = (guint16) (gdkRGBAColor.green * 65535);
> +    gdkColor.blue = (guint16) (gdkRGBAColor.blue * 65535);
> +

I think it makes sense to create a WebCore color here directly instead of relying on a GdkColor intermediary. Perhaps this could just be:

return Color(gdkRGBAColor.red, gdkRGBAColor.green, gdkRGBAColor.blue, gdkRGBAColor.alpha);

I could even see adding a specialized constructor in Color.h and Color.cpp for GDK 3.x's GdkRGBA.

[Carlos Garcia Campos]

Created attachment 77870 [details]
Updated patch

I've added Color constructor for GdkRGBA which simplifies everything, thanks Martin :-)

[Martin Robinson]

Comment on attachment 77870 [details]
Updated patch

Very nice!

[Carlos Garcia Campos]

Committed r75056: <http://trac.webkit.org/changeset/75056>