Summary: | [GTK] Text not discernable when using a dark Gtk+ style | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alp Toker <alp> | ||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED DUPLICATE | ||||||||||
Severity: | Normal | CC: | christian, Don, gnome, gustavo, jmalonzo, mrobinson, zl29ah | ||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||
Version: | 523.x (Safari 3) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 19486 | ||||||||||
Attachments: |
|
Description
Alp Toker
2007-10-21 12:44:12 PDT
Created attachment 23917 [details]
gtkrc demonstrating the issues
Got this from a Debian bug report. Demonstrates the issue, confirmed present in trunk (and has been this way since at least when we switched to gtkmoz RenderTheme, maybe before).
Put this in ~/.gtkrc-2.0 to see the issues, eg:
Widget (button, entry etc.) backgrounds are dark, yet the foreground (text) color is also dark resulting in illegible text.
Text selection color in entries also has no contrast which is no good.
The fix for this is straightforward, but needs a little testing to make sure it works consistently with dark and light themes and matches GTK+ appearances as closely as possible. What needs to be done to make this work: In RenderThemeGtk.cpp, we need to adjust the CSS style's colors to match what GTK+ would use. void RenderThemeGtk::adjustTextFieldStyle(CSSStyleSelector* selector, RenderStyle* style, Element* e) const { ... + style->setColor(gtkEntry()->style->text[GTK_STATE_NORMAL]); + style->setBackgroundColor(gtkEntry()->style->base[GTK_STATE_NORMAL]); ... } void RenderThemeGtk::adjustButtonStyle(CSSStyleSelector* selector, RenderStyle* style, WebCore::Element* e) const { ... + if (gButtonWidget) { + printf ("gButtonWidget()\n"); + style->setColor(gButtonWidget->style->fg[GTK_STATE_NORMAL]); + style->setBackgroundColor(gButtonWidget->style->bg[GTK_STATE_NORMAL]); + } ... } Don't have time to make a patch right now. If someone can pick this up, it'd be great and I'll be happy to review. *** Bug 19486 has been marked as a duplicate of this bug. *** Created attachment 30177 [details]
A fix for gtkEntry
Hey,
I did as you wrote and it indeed fixed the gtkEntry problem.
I had no luck with the buttons though.
Please include at least this fix, and if you can furthur help me with the buttons I'll be glad to help. I have no knowledge of C/C++, but I can learn and this bug annoys me quite a bit...
Created attachment 30178 [details]
This fixes the whole problem
This fixes the whole problem, but I'm not sure it's the Right Way(tm) to do it. I'm using TreeView to get the colors from, which seems to work great with all the themes I tested (dark and light themes).
Please review...
Comment on attachment 30178 [details]
This fixes the whole problem
Please mark it as '?' if you want it to be reviewed.
(In reply to comment #5) > Please review... > Please see http://webkit.org/coding/contributing.html and provide a changelog entry. (In reply to comment #5) > Created an attachment (id=30178) [review] > This fixes the whole problem > > This fixes the whole problem, but I'm not sure it's the Right Way(tm) to do it. > I'm using TreeView to get the colors from, which seems to work great with all > the themes I tested (dark and light themes). > > Please review... This is not gonna work if the theme suddenly changed from a light to a dark theme or vice-versa. The reason being is that adjust*Styles will not be called when the theme changes hence, the text colors will retain the previous themes color (e.g. changing from Darklooks to Clearlooks will leave the text color using Darklooks's fg color and not Clearlook's). Comment on attachment 30178 [details] This fixes the whole problem This patch requires a ChangeLog. Please see http://webkit.org/coding/contributing.html for more info. > --- RenderThemeGtk.cppp 2009-05-11 12:27:40.523120385 +0300 > +++ RenderThemeGtk.cpp 2009-05-11 12:27:27.969801589 +0300 Please do a diff in Webkit/ instead of WebCore/platform/gtk next time. > @@ -272,6 +272,7 @@ void RenderThemeGtk::adjustButtonStyle(C > style->resetBorder(); > style->setHeight(Length(Auto)); > style->setWhiteSpace(PRE); > + style->setColor(gtkTreeView()->style->text[GTK_STATE_NORMAL]); > setButtonPadding(style); > } else { > // FIXME: This should not be hard-coded. > @@ -306,6 +307,8 @@ void RenderThemeGtk::adjustTextFieldStyl > style->resetPadding(); > style->setHeight(Length(Auto)); > style->setWhiteSpace(PRE); > + style->setColor(gtkEntry()->style->text[GTK_STATE_NORMAL]); > + style->setBackgroundColor(gtkEntry()->style->base[GTK_STATE_NORMAL]); > adjustMozStyle(style, MOZ_GTK_ENTRY); > } As I said in comment #8, we also need to accomodate for on-the-fly desktop theme changes. I'm going to say r- until the issues I raised above gets resolved. Hey, sorry about the incorrect way of posting the patch. I'm having difficulties doing 'svn checkout' for the whole repo, so I can't quite use the tools making the changelog or the diff. If someone could do that I'll be thankful. As for the colors not changing when the user changes theme - I think it's a pretty small problem. With this patch, A might see the text color wrong when switching from Darklooks to Clearlooks while using WebKit/GTK, but he'd see it *all the time* when using Darklooks without this patch. Compare the number of people using a dark theme to the number of people switching from a dark theme to a light one while running WebKit/GTK - surely the latter used a dark theme before switching, but most of users a using dark theme don't change it all the time... I simply have no idea how to check if the user changed the theme, but if this is all we have right now I think it's much better than nothing. Sure it's not complete, but a lot of things are and we should progress one step after another. Also, sorry if my english is incorrect, I'm breaking my teeth here :) Thanks. (In reply to comment #5) > Created an attachment (id=30178) [details] > This fixes the whole problem > > This fixes the whole problem, but I'm not sure it's the Right Way(tm) to do it. I'm using TreeView to get the colors from, which seems to work great with all the themes I tested (dark and light themes). > > Please review... The patch doesn't fix the <textarea> font color: i am ending up with black text on black background. Tested on 1.2.5. There's another bug tracking the same (or very similar) issue. I'm closing this as a duplicate as that bug has a more in-depth patch and the approach in this patch looks incorrect. The color should not unconditionally override the CSS style. *** This bug has been marked as a duplicate of bug 40024 *** |