15597 2007-10-21 12:44:12 -0700 [GTK] Text not discernable when using a dark Gtk+ style 2010-12-29 16:40:07 -0800 1 1 1 Unclassified WebKit New Bugs 523.x (Safari 3) All All RESOLVED DUPLICATE 40024 Gtk P2 Normal --- 19486 1 alp webkit-unassigned christian Don gnome gustavo jmalonzo mrobinson zl29ah oldest_to_newest 59027 0 alp 2007-10-21 12:44:12 -0700 The text in buttons and other UI elements in the browser remains black, even when the background is also black, causing thr text to be illegible. The fix in RenderThemeGtk will be to extract as much information as possible out of the GTK+ style and modify WebKit's formatting to suit and match what GTK+ would have done. There is a delicate balance, be we can fix the vast majority of issues quite easily and then work on the subtleties. 93267 1 23917 alp 2008-09-29 13:10:35 -0700 Created attachment 23917 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. 93270 2 alp 2008-09-29 13:24:09 -0700 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. 117061 3 christian 2009-04-09 12:10:19 -0700 *** Bug 19486 has been marked as a duplicate of this bug. *** 120611 4 30177 bjesus 2009-05-11 02:22:10 -0700 Created attachment 30177 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... 120612 5 30178 bjesus 2009-05-11 02:48:20 -0700 Created attachment 30178 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... 120625 6 30178 christian 2009-05-11 05:09:52 -0700 Comment on attachment 30178 This fixes the whole problem Please mark it as '?' if you want it to be reviewed. 120629 7 zecke 2009-05-11 05:25:36 -0700 (In reply to comment #5) > Please review... > Please see http://webkit.org/coding/contributing.html and provide a changelog entry. 120842 8 jmalonzo 2009-05-12 07:34:29 -0700 (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). 121614 9 30178 jmalonzo 2009-05-15 17:46:01 -0700 Comment on attachment 30178 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. 121681 10 bjesus 2009-05-17 04:17:01 -0700 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. 298297 11 zl29ah 2010-10-22 18:29:42 -0700 (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. 327744 12 mrobinson 2010-12-29 16:40:07 -0800 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 *** 23917 2008-09-29 13:10:35 -0700 2008-09-29 13:10:35 -0700 gtkrc demonstrating the issues gtkrc-2.0-dark text/plain 2857 alp IyMjIyAtKi0gY29uZiAtKi0KIyMjIyBodHRwOi8vbGlicmFyeS5nbm9tZS5vcmcvZGV2ZWwvZ3Rr L3Vuc3RhYmxlL0d0a1NldHRpbmdzLmh0bWwKIyMjIyBFbmdpbmUgc2V0dGluZ3MgaW4gL3Vzci9z aGFyZS9ndGstZW5naW5lcy8qLnhtbAoKZ3RrLWN1cnNvci10aGVtZS1uYW1lID0gIkRNWi1CbGFj ayIKZ3RrLXRvb2xiYXItc3R5bGUgPSBHVEtfVE9PTEJBUl9CT1RIX0hPUklaCmd0ay1zY3JvbGxl ZC13aW5kb3ctcGxhY2VtZW50ID0gR1RLX0NPUk5FUl9UT1BfUklHSFQKZ3RrLWVuYWJsZS1hbmlt YXRpb25zID0gMAojIyBCcm9rZW46IGNhbid0IGNsb3NlIHdpbmRvd3MgKHN0ZWFscyBDLXcpIGFu ZCBvbmx5IGltcGxlbWVudHMgdGhlCiMjIG1vc3Qgc3VwZXJmaWNpYWwgRW1hY3MgYmVoYXZpb3Vz IGFueXdheSAtLSBub3QgZXZlbiBDLVNQQyEKIyBndGsta2V5LXRoZW1lLW5hbWUgPSAiRW1hY3Mi CiMjIEJyb2tlbjogZm9yY2VzIHRoZSB1c2Ugb2YgSEMgcG5ncyBmb3Igc29tZSBpY29ucy4KIyBn dGstdGhlbWUtbmFtZSA9ICJIaWdoQ29udHJhc3RJbnZlcnNlIgpndGstaWNvbi10aGVtZS1uYW1l ID0gIkhpZ2hDb250cmFzdC1TVkciCgojIyBHdGstY29sb3Itc2NoZW1lIGV4aXN0cyBzbyB0aGF0 IHlvdSBjYW4gY3VzdG9taXplIGEgdGhlbWUncyBjb2xvcgojIyBzY2hlbWUgd2l0aG91dCB1bmRl cnN0YW5kaW5nIHRoZSBpbnRyaWNhY2llcyBvZiB0aGUgdGhlbWUgaXRzZWxmLgojIyBJZiB5b3Ug c2V0IGd0ay1jb2xvci1zY2hlbWUgaW4gZ25vbWUtc2V0dGluZ3MtZGFlbW9uLCB0aGlzIHdvcmtz LgojIyBKdXN0IHNldHRpbmcgaXQgYW5kIGd0ay10aGVtZS1uYW1lIGluIHRoaXMgZmlsZSBET0VT IE5PVCB3b3JrLAojIyBiZWNhdXNlIHN5bWJvbGljIHJlZmVyZW5jZXMgYXJlIHJlc29sdmVkIGF0 IHBhcnNlIHRpbWUgLS0gdGhhdCBpcywKIyMgdGhleSBvbmx5IHdvcmsgd2l0aGluIHRoZSBmaWxl IGluIHdoaWNoIHRoZXkgb2NjdXIsIG5vdCBpbiBpbmNsdWRlZAojIyBmaWxlcyBvciBndGstdGhl bWUtbmFtZSBmaWxlcy4KIyMKIyMgV2hhdCB0byBkbz8gIFlvdSBjYW4gMSkgcnVuIGdub21lLXNl dHRpbmdzLWRhZW1vbjsgMikgY29weSB0aGUKIyMgdGhlbWUgeW91IHdhbnQgaW50byB+Ly50aGVt ZXMvdHdiIGFuZCBjaGFuZ2UgdGhlIE9ORSBMSU5FIHRoYXQgc2V0cwojIyBndGstY29sb3Itc2No ZW1lLCB0aGVuIHNldCBndGstdGhlbWUtbmFtZSB0byAidHdiIi4gIEkgaGF2ZSBkb25lIGEKIyMg dmFyaWFudCBvZiBsYXR0ZXIsIGNvcHktYW5kLXBhc3RpbmcgYSBzdHJpcHBlZC1kb3duIHRoZW1l IGluIGJlbG93LgpndGstY29sb3Itc2NoZW1lID0gImZnX2NvbG9yOiB3aGl0ZQpiZ19jb2xvcjog YmxhY2sKdGV4dF9jb2xvcjogd2hpdGUKYmFzZV9jb2xvcjogYmxhY2sKc2VsZWN0ZWRfZmdfY29s b3I6IHdoaXRlCnNlbGVjdGVkX2JnX2NvbG9yOiBncmV5MjAKdG9vbHRpcF9mZ19jb2xvcjogd2hp dGUKdG9vbHRpcF9iZ19jb2xvcjogYmxhY2siCgojIyBTeW1ib2xpYyBuYW1lcyB3ZXJlIGludHJv ZHVjZWQgaW4gMi4xMCwgc28gb24gb2xkZXIgaG9zdHMgeW91J2xsCiMjIHNlZSBkZWZhdWx0IGNv bG9ycyBhbmQgZ2V0IGEgcGFyc2UgZXJyb3Igd2FybmluZyBvbiBzdGRlcnIuCiMjIFNpbWlsYXJs eSwgaWYgbGliaW5kdXN0cmlhbC5zbyBpc24ndCBhdmFpbGFibGUsIHlvdSdsbCBnZXQgdGhlCiMj IGRlZmF1bHQgZW5naW5lLCBSYWxlaWdoLgojIwojIyBOb3RlIHRoYXQgSSdtIHVzaW5nIHRoZSBv bGQgSW5kdXN0cmlhbCBhbmQgSGlnaCBDb250cmFzdCBlbmdpbmVzLgojIyBUaGF0J3MgYmVjYXVz ZSBub25lIG9mIHRoZSBvdGhlciBlbmdpbmVzIHNlZW0gYWJsZSB0byBjb3BlIHdpdGggYQojIyBj b21wbGV0ZWx5IGJsYWNrIGJhY2tncm91bmQgLS0geW91IGVuZCB1cCB3aXRoIGEgZmFudGFzdGlj YWxseQojIyB1c2VsZXNzIGFtb3VudCBvZiBibGFjay1vbi1ibGFjayBib3JkZXJzIGFuZCBjaGVj a2JveGVzLgpzdHlsZSAidHdiIiB7CiAgZW5naW5lICJoY2VuZ2luZSIgeyBlZGdlX3RoaWNrbmVz cyA9IDEgfQogIGVuZ2luZSAiaW5kdXN0cmlhbCIgeyBjb250cmFzdCA9IDAuNSB9CiAgYmFzZVtJ TlNFTlNJVElWRV0gPSBAYmFzZV9jb2xvcgogIGJnW0lOU0VOU0lUSVZFXSAgID0gQGJnX2NvbG9y CiAgZmdbSU5TRU5TSVRJVkVdICAgPSBAZmdfY29sb3IKICB0ZXh0W0lOU0VOU0lUSVZFXSA9IEB0 ZXh0X2NvbG9yCiAgYmFzZVtOT1JNQUxdICAgICAgPSBAYmFzZV9jb2xvcgogIGJnW05PUk1BTF0g ICAgICAgID0gQGJnX2NvbG9yCiAgZmdbTk9STUFMXSAgICAgICAgPSBAZmdfY29sb3IKICB0ZXh0 W05PUk1BTF0gICAgICA9IEB0ZXh0X2NvbG9yCiAgYmFzZVtBQ1RJVkVdICAgICAgPSBAYmFzZV9j b2xvcgogIGJnW0FDVElWRV0gICAgICAgID0gQGJnX2NvbG9yCiAgZmdbQUNUSVZFXSAgICAgICAg PSBAZmdfY29sb3IKICB0ZXh0W0FDVElWRV0gICAgICA9IEB0ZXh0X2NvbG9yCiAgYmFzZVtTRUxF Q1RFRF0gICAgPSBAc2VsZWN0ZWRfYmdfY29sb3IKICBiZ1tTRUxFQ1RFRF0gICAgICA9IEBzZWxl Y3RlZF9iZ19jb2xvcgogIGZnW1NFTEVDVEVEXSAgICAgID0gQHNlbGVjdGVkX2ZnX2NvbG9yCiAg dGV4dFtTRUxFQ1RFRF0gICAgPSBAc2VsZWN0ZWRfZmdfY29sb3IKICBiYXNlW1BSRUxJR0hUXSAg ICA9IEBzZWxlY3RlZF9iZ19jb2xvcgogIGJnW1BSRUxJR0hUXSAgICAgID0gQHNlbGVjdGVkX2Jn X2NvbG9yCiAgZmdbUFJFTElHSFRdICAgICAgPSBAc2VsZWN0ZWRfZmdfY29sb3IKICB0ZXh0W1BS RUxJR0hUXSAgICA9IEBzZWxlY3RlZF9mZ19jb2xvcgp9CndpZGdldF9jbGFzcyAiKiIgc3R5bGUg InR3YiIKCg== 30177 2009-05-11 02:22:10 -0700 2009-05-11 02:48:20 -0700 A fix for gtkEntry mypatch.patch text/plain 465 bjesus LS0tIFJlbmRlclRoZW1lR3RrLmNwcAkyMDA5LTA1LTExIDExOjU5OjE1LjkyOTk1ODQ5MyArMDMw MAorKysgUmVuZGVyVGhlbWVHdGsuY3BwcAkyMDA5LTA1LTExIDExOjU5OjA3LjgzOTk3OTI3NSAr MDMwMApAQCAtMzA2LDYgKzMwNiw4IEBAIHZvaWQgUmVuZGVyVGhlbWVHdGs6OmFkanVzdFRleHRG aWVsZFN0eWwKICAgICBzdHlsZS0+cmVzZXRQYWRkaW5nKCk7CiAgICAgc3R5bGUtPnNldEhlaWdo dChMZW5ndGgoQXV0bykpOwogICAgIHN0eWxlLT5zZXRXaGl0ZVNwYWNlKFBSRSk7CisgICAgc3R5 bGUtPnNldENvbG9yKGd0a0VudHJ5KCktPnN0eWxlLT50ZXh0W0dUS19TVEFURV9OT1JNQUxdKTsK KyAgICBzdHlsZS0+c2V0QmFja2dyb3VuZENvbG9yKGd0a0VudHJ5KCktPnN0eWxlLT5iYXNlW0dU S19TVEFURV9OT1JNQUxdKTsKICAgICBhZGp1c3RNb3pTdHlsZShzdHlsZSwgTU9aX0dUS19FTlRS WSk7CiB9CiAK 30178 2009-05-11 02:48:20 -0700 2010-06-10 17:40:50 -0700 This fixes the whole problem mypatch.patch text/plain 804 bjesus LS0tIFJlbmRlclRoZW1lR3RrLmNwcHAJMjAwOS0wNS0xMSAxMjoyNzo0MC41MjMxMjAzODUgKzAz MDAKKysrIFJlbmRlclRoZW1lR3RrLmNwcAkyMDA5LTA1LTExIDEyOjI3OjI3Ljk2OTgwMTU4OSAr MDMwMApAQCAtMjcyLDYgKzI3Miw3IEBAIHZvaWQgUmVuZGVyVGhlbWVHdGs6OmFkanVzdEJ1dHRv blN0eWxlKEMKICAgICAgICAgc3R5bGUtPnJlc2V0Qm9yZGVyKCk7CiAgICAgICAgIHN0eWxlLT5z ZXRIZWlnaHQoTGVuZ3RoKEF1dG8pKTsKICAgICAgICAgc3R5bGUtPnNldFdoaXRlU3BhY2UoUFJF KTsKKyAgICAgICAgc3R5bGUtPnNldENvbG9yKGd0a1RyZWVWaWV3KCktPnN0eWxlLT50ZXh0W0dU S19TVEFURV9OT1JNQUxdKTsKICAgICAgICAgc2V0QnV0dG9uUGFkZGluZyhzdHlsZSk7CiAgICAg fSBlbHNlIHsKICAgICAgICAgLy8gRklYTUU6IFRoaXMgc2hvdWxkIG5vdCBiZSBoYXJkLWNvZGVk LgpAQCAtMzA2LDYgKzMwNyw4IEBAIHZvaWQgUmVuZGVyVGhlbWVHdGs6OmFkanVzdFRleHRGaWVs ZFN0eWwKICAgICBzdHlsZS0+cmVzZXRQYWRkaW5nKCk7CiAgICAgc3R5bGUtPnNldEhlaWdodChM ZW5ndGgoQXV0bykpOwogICAgIHN0eWxlLT5zZXRXaGl0ZVNwYWNlKFBSRSk7CisgICAgc3R5bGUt PnNldENvbG9yKGd0a0VudHJ5KCktPnN0eWxlLT50ZXh0W0dUS19TVEFURV9OT1JNQUxdKTsKKyAg ICBzdHlsZS0+c2V0QmFja2dyb3VuZENvbG9yKGd0a0VudHJ5KCktPnN0eWxlLT5iYXNlW0dUS19T VEFURV9OT1JNQUxdKTsKICAgICBhZGp1c3RNb3pTdHlsZShzdHlsZSwgTU9aX0dUS19FTlRSWSk7 CiB9CiAK