Bug 152888 – [GTK] Cleanup RenderThemeGtk

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

[Carlos Garcia Campos]

Use a common path for GTK+ 3.19 and previous versions, simplifying the code and removing a lot of ifdefs.

[Carlos Garcia Campos]

Created attachment 268539 [details]
Patch

[Carlos Garcia Campos]

This has been tested with GTK+ from current master and the internal jhbuild.

[Michael Catanzaro]

Comment on attachment 268539 [details]
Patch

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

Excellent patch; this worked quite well. Thanks for taking the time to work on this. Please watch the bots when this lands.

Looks like this also fixes bug #151502. Please close that when this lands.

Somewhat related: look at bug #152273.

> Source/WebCore/rendering/RenderThemeGtk.cpp:140
> +enum RenderThemePart {

I prefer to almost always use enum class in new code, both for safety, and also because I think it's more readable to use the RenderThemePart:: scope everywhere. But if you prefer to avoid the scoping, this is fine.

> Source/WebCore/rendering/RenderThemeGtk.cpp:379
> +    gtk_style_context_set_state(context.get(), gtk_widget_path_iter_get_state(path.get(), -1));

What's this for? Looks like a mistake. You have not set any state on the GtkWidgetPath. If this code actually does anything, then I would suspect that context might be inheriting its state from its parent... in which case it would be clearer to pass GTK_STATE_FLAGS_NONE.

[Carlos Garcia Campos]

Committed r194847: <http://trac.webkit.org/changeset/194847>

[Mario Sanchez Prada]

This patch broke accessibility/gtk/entry-and-password.html. See bug 153062.

[Michael Catanzaro]

This patch broke ~85 layout tests. Carlos fixed ~70 already so there are ~15 left to discover, including accessibility/gtk/entry-and-password.html. Thanks for helping Mario. :)