[Webkit-unassigned] [Bug 126907] [GTK] Hardcoded text color in input fields

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=126907

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #341088|review?, commit-queue?      |review-, commit-queue-
              Flags|                            |

--- Comment #40 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 341088
  --> https://bugs.webkit.org/attachment.cgi?id=341088
Patch

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

I think Carlos Garcia should review the RenderThemeGtk changes, but I can review the RenderThemeGadget bits. You're on the right track, but this isn't quite what I had in mind.

> Source/WebCore/platform/gtk/RenderThemeGadget.cpp:64
> +    // Class to be used by GTK themers if they want custom styling inside webviews.
> +    gtk_style_context_add_class(context.get(), "webview");

Hm, style classes have fallen out of favor, so let's not add a new one now. It would be better to modify the widget path instead.

> Source/WebCore/platform/gtk/RenderThemeGadget.cpp:245
> +    // This workaround is for some themes like HighConstrast lacking "color" gtk-css attribute in entries.
> +    // When the widgets are drawn by GTK they go to this fallback theme. However here they were getting the default color.
> +    // As a result, for example, HighConstrast theme was drawing white text in white background in text inputs.
> +    if (m_context)
> +        gtk_style_context_add_class(m_context.get(), "background");

It's not a workaround at all! It's just the right thing to do. We're trying to put together a widget path that mimics a real GTK+ app.

This isn't the right place, though, because the new widget path should be used for all gadgets, not just text fields. It could fix more than just text color in input fields.

Also, the goal is to adjust the widget path, not add a style class. I'm kinda surprised this worked, actually.

Look at my comment #32. """So the place to do that would be in the RenderThemeGadget constructor. There are two places we currently use gtk_widget_path_new(). Those would need to instead use a default base widget path. "window.background" is what Company thinks would fix the text color. And then, optionally, "webview" to allow theme developers to provide custom styling for WebKit...."""

See here:

    GRefPtr<GtkWidgetPath> path = parent ? adoptGRef(gtk_widget_path_copy(gtk_style_context_get_path(parent->context()))) : adoptGRef(gtk_widget_path_new());
    if (!siblings.isEmpty()) {
        GRefPtr<GtkWidgetPath> siblingsPath = adoptGRef(gtk_widget_path_new());

Instead of passing gtk_widget_path_new(), we want to create a custom widget path to pass there instead. So let's add a createWidgetPath() function to use there instead. Should look something like:

static GRefPtr<GtkWidgetPath> createWidgetPath()
{
    GRefPtr<GtkWidgetPath> path = adoptGRef(gtk_widget_path_new());

    gtk_widget_path_append_type(path, GTK_TYPE_WINDOW);
    gtk_widget_path_iter_add_class (path, -1, GTK_STYLE_CLASS_BACKGROUND);

    gtk_widget_path_append_type(path, WEBKIT_TYPE_WEB_VIEW);
    gtk_widget_path_iter_set_object_name(path, -1, "webview");

    return path;
}

Then we should have a sane widget path for everything that we draw. I copied the first part of that from https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/testsuite/gtk/firefox-stylecontext.c, which seems sufficiently-authoritative to me. Now, will it work? That is the question....

> Source/WebCore/rendering/RenderThemeGtk.cpp:502
> +    switch (part) {
> +    default:
> +        ASSERT_NOT_REACHED();

default at the top is really weird, is that copied from elsewhere in this file? It's idiomatic to put the default case at the bottom of the switch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180523/ba79cf0d/attachment.html>