Bug 118234

Created attachment 205793 [details] Patch

Comment on attachment 205793 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=205793&action=review > Source/WebCore/platform/gtk/RenderThemeGtk.cpp:220 > + style->setColor(menuListButtonTextColor(element)); Doesn't this prevent the CSS theme from adjusting the text color?

(In reply to comment #2) > (From update of attachment 205793 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=205793&action=review > > > Source/WebCore/platform/gtk/RenderThemeGtk.cpp:220 > > + style->setColor(menuListButtonTextColor(element)); > > Doesn't this prevent the CSS theme from adjusting the text color? I'm not sure, maybe the css color is overriden after this. Check RenderThemeMac::adjustMenuListStyle, mac also sets the color there.

(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 205793 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=205793&action=review > > > > > Source/WebCore/platform/gtk/RenderThemeGtk.cpp:220 > > > + style->setColor(menuListButtonTextColor(element)); > > > > Doesn't this prevent the CSS theme from adjusting the text color? > > I'm not sure, maybe the css color is overriden after this. Check RenderThemeMac::adjustMenuListStyle, mac also sets the color there. It might be worth overriding it always, but it also should be possible to avoid it, I think, by only overriding it if it is set to the default color.

I confirm it fixes the bug in my case. Thanks!

I think the same bug is happening with input buttons.

(In reply to comment #6) > I think the same bug is happening with input buttons. Actually no; this seems to be a different bug.

Martin, do you object to this patch? It would be good to get it in.

(In reply to comment #8) > Martin, do you object to this patch? It would be good to get it in. This patch overrides the color even if the CSS changes it. I think it should either have RenderThemeGtk::systemColor the proper color or if that is impossible to check if the style specifies the default color and in that case to override it conditionally.

Comment on attachment 205793 [details] Patch "This patch overrides the color even if the CSS changes it." <-- This is a problem. I guess it's probably a problem elsewhere in this file, though. Also, this code won't work now that the button style class was removed from GTK+. I think the cell style class might be due for removal as well....

Created attachment 276473 [details] Patch People using dark themes have suffered this for too long already.

(In reply to comment #10) > Comment on attachment 205793 [details] > Patch > > "This patch overrides the color even if the CSS changes it." <-- This is a > problem. I guess it's probably a problem elsewhere in this file, though. > > Also, this code won't work now that the button style class was removed from > GTK+. I think the cell style class might be due for removal as well.... I think we need to implement RenderTheme::isControlStyled() properly to decide whether to apply native style or not.

Comment on attachment 276473 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=276473&action=review > Source/WebCore/rendering/RenderThemeGtk.cpp:776 > +#if GTK_CHECK_VERSION(3, 20, 0) Seems slightly nicer to put the #if inside the function since there is no difference in the arguments type or usage. > Source/WebCore/rendering/RenderThemeGtk.cpp:779 > + RenderThemeGadget::Info info = { RenderThemeGadget::Type::Generic, "combobox", element->isDisabledFormControl() ? GTK_STATE_FLAG_INSENSITIVE : GTK_STATE_FLAG_NORMAL, { } }; I believe there is no need for the "=" here. > Source/WebCore/rendering/RenderThemeGtk.cpp:781 > + Vector<RenderThemeGadget::Info> children = { I believe there is no need for the "=" here. > Source/WebCore/rendering/RenderThemeGtk.cpp:786 > + return std::make_unique<RenderThemeBoxGadget>(info, children, comboGadget.get())->child(0)->color(); Why does this need to be constructed on the heap with make_unique instead of just constructing it on the stack? return RenderThemeBoxGadget(info, children, comboGadget.get()).child(0)->color();

Committed r199659: <http://trac.webkit.org/changeset/199659>

*** Bug 165072 has been marked as a duplicate of this bug. ***