Bug 137803

Summary: [GTK] Several labels are white instead of black
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, esprehn+autocc, glenn, kondapallykalyan, mcatanzaro, mrobinson, otte
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Screenshot
none
screenshot #2 (missing text is selected, should instead be white on blue)
none
Patch mrobinson: review+

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.