Bug 137803 - [GTK] Several labels are white instead of black
Summary: [GTK] Several labels are white instead of black
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-16 20:38 PDT by Michael Catanzaro
Modified: 2015-11-21 11:45 PST (History)
8 users (show)

See Also:


Attachments
Screenshot (174.79 KB, image/png)
2014-10-16 20:38 PDT, Michael Catanzaro
no flags Details
screenshot #2 (missing text is selected, should instead be white on blue) (93.60 KB, image/png)
2014-10-16 20:39 PDT, Michael Catanzaro
no flags Details
Patch (6.33 KB, patch)
2014-10-20 08:35 PDT, Carlos Garcia Campos
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2014-10-16 20:38:07 PDT
Created attachment 239991 [details]
Screenshot

With GTK+ and WebKit master, all button labels are white instead of black. It's also occurring for selected labels in scrolled boxes like the component and version fields on the new bug report page (what is the name of this type of widget?), and for all text if you select text with your mouse.
Comment 1 Michael Catanzaro 2014-10-16 20:39:12 PDT
Created attachment 239992 [details]
screenshot #2 (missing text is selected, should instead be white on blue)
Comment 2 Carlos Garcia Campos 2014-10-16 23:29:55 PDT
I can't reproduce it, so I bet it's not WebKit master, but GTK+ master and yet another change in the theme.
Comment 3 Carlos Garcia Campos 2014-10-20 08:35:50 PDT
Created attachment 240126 [details]
Patch
Comment 4 Martin Robinson 2014-10-20 08:44:22 PDT
Comment on attachment 240126 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        Recent GTK+ versions require to explicitly set the state before
> +        getting a color.
> +

The changelog does not mention at all the change regarding gtk_style_context_get_background_color vs. gtk_style_context_get_color. It might be best to write a brief note about that here. What necessitated it?

> Source/WebCore/rendering/RenderThemeGtk.cpp:1343
> +    // Recent GTK+ versions require to explicitly set the state before getting the color,
> +    gtk_style_context_set_state(context, state);
> +

Probably best to say what version introduced this change, if possible.
Comment 5 Carlos Garcia Campos 2014-10-20 08:48:26 PDT
(In reply to comment #4)
> Comment on attachment 240126 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240126&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        Recent GTK+ versions require to explicitly set the state before
> > +        getting a color.
> > +
> 
> The changelog does not mention at all the change regarding
> gtk_style_context_get_background_color vs. gtk_style_context_get_color. It
> might be best to write a brief note about that here. What necessitated it?

There's actually no change, I mean, *BakcgroundColor functions called gtk_style_context_get_background color and the *ForegroundColor function called gtk_style_context_get_color. That hasn't changed, I added a enum paramater to call one or the other to avoid adding to functions.

> > Source/WebCore/rendering/RenderThemeGtk.cpp:1343
> > +    // Recent GTK+ versions require to explicitly set the state before getting the color,
> > +    gtk_style_context_set_state(context, state);
> > +
> 
> Probably best to say what version introduced this change, if possible.

It's current git master, I can mention GTK+ 3.15.1 for example.
Comment 6 Martin Robinson 2014-10-20 08:50:13 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 240126 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=240126&action=review
> > 
> > > Source/WebCore/ChangeLog:10
> > > +        Recent GTK+ versions require to explicitly set the state before
> > > +        getting a color.
> > > +
> > 
> > The changelog does not mention at all the change regarding
> > gtk_style_context_get_background_color vs. gtk_style_context_get_color. It
> > might be best to write a brief note about that here. What necessitated it?
> 
> There's actually no change, I mean, *BakcgroundColor functions called
> gtk_style_context_get_background color and the *ForegroundColor function
> called gtk_style_context_get_color. That hasn't changed, I added a enum
> paramater to call one or the other to avoid adding to functions.


Sorry. I misunderstood the change. Might be worth mentioning the refactor though.
Comment 7 Carlos Garcia Campos 2014-10-21 00:46:43 PDT
Committed r174929: <http://trac.webkit.org/changeset/174929>
Comment 8 Michael Catanzaro 2015-11-21 11:45:40 PST
Oops, since this patch we're permanently changing the state of the style contexts that we've cached to get theme colors for GtkTextEntry and GtkTreeView objects, which isn't right. That could cause the colors returned by the style contexts to change unexpectedly. Follow-up in bug #151533.