Closed Bug 67448 Opened 24 years ago Closed 17 years ago

CSS2 system colors in GTK port don't reflect reality

Categories

(Core :: XUL, defect, P3)

Product:
Core
Component:
XUL
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

Attachments

(21 files)

patch
2.34 KB, patch
Details | Diff | Splinter Review
revised patch - also change buttontext
2.57 KB, patch
Details | Diff | Splinter Review
patch to change ThreeDLightShadow to match hewitt's interpretation of CSS2
1.13 KB, patch
Details | Diff | Splinter Review
change to button.css needed to make this look right
1.21 KB, patch
Details | Diff | Splinter Review
patch that should be better across themes
8.94 KB, patch
Details | Diff | Splinter Review
A screenshot of the 'improved' look :)
44.88 KB, image/png
Details
new patch removing blocks-for-documentation
8.73 KB, patch
Details | Diff | Splinter Review
Screenshot of 2001-03-05-05
64.24 KB, image/png
Details
Screenshot (2001030508) showing the BHgtk theme. Now the texts are white again as they should be but all 3D highlights are gone.
18.32 KB, image/png
Details
Screenshot, after the commit went through. 3d highlights are now all black (not a good thing). It looked fine before. *grin*
67.07 KB, image/png
Details
Here's a screenshot showing off both Mozilla and GTK+ -- the theme is Adept.Duo (an plain color RC file theme) with the Raleigh engine selected.
78.93 KB, image/png
Details
workaround
944 bytes, patch
Details | Diff | Splinter Review
patch that seems to fix the 3-D colors problem correctly
944 bytes, patch
Details | Diff | Splinter Review
patch to fix the 3-D colors problem properly (wrong patch last time)
1.52 KB, patch
Details | Diff | Splinter Review
patch to backout
9.15 KB, patch
Details | Diff | Splinter Review
alternate patch that may be the "right thing" but messes up LCARS even more b/c of classic skin's color use
1.22 KB, patch
Details | Diff | Splinter Review
patch to add 3 new system colors (-moz-FieldText, -moz-Dialog, -moz-DialogText)
11.97 KB, patch
Details | Diff | Splinter Review
test page showing correct foreground/background pairs
1.34 KB, text/html
Details
debugging code to debug system color use in skins
2.86 KB, patch
Details | Diff | Splinter Review
patch to fix system color usage in classic skin and HTML form controls and fix CSS errors in skins
32.40 KB, patch
Details | Diff | Splinter Review
Patch to make -moz-FieldText use text instead of fg
1.38 KB, patch
Details | Diff | Splinter Review
The GTK system colors differ from the theme in a number of ways: * the foreground color in the selection and menu highlight is wrong (easily visible with Metal theme, where it's white vs. black) * for themes that have two foreground colors (e.g., Metal), we're using the one that's used for fewer things and probably was never meant to be used as a primary text color as the main text color for the app UI (and even the document if the use system colors is checked in the color prefs). This makes things hard to read. I'm not sure what the distinction is between them, but I think we're using the wrong one. Anyway, I'll attach a patch that seems to help some of these issues (and it seems to work in some other themes too). Any comments on the patch?
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9
I noticed yesterday that the color "ThreeDLightShadow" was equivalent to "ThreeDHighlight" under GTK, but it should really be equivalent to "ThreeDFace". Could you fix that also before you check this in?
I want to investigate whether we can get more accurate system colors in a manner similar to the way we get the tooltip colors now. We get the tooltip colors by actually creating a tooltip widget created with |gtk_tooltips_new| rather than getting the colors off of the style object for a widget created by |gtk_invisible_new|. I should probably look at the GTK source first to see if this will have any real advantages and to figure out exactly how GTK gets the colors it uses for all the different widgets. And, for those readers coming here from a comment in the mozilla 0.8 slashdot thread (at <URI: http://slashdot.org/comments.pl?sid=01/02/16/0254209&cid=61 >), the "they" who "still haven't bothered to check it in" is me, and I haven't bothered to get it reviewed and checked in because: * I have other things to do * I'm not satisfied with the current patch (see above comment for why)
Actually, I changed my mind. Since I probably won't get around to fixing this the right way for a while, I may as well try to get the replacement of one hack with a slightly more accurate one checked in...
r=bryner
r=blizzard
r=pavlov
I checked in the fix 2001-02-17 10:55. Futuring since there's still more work to do if I, or anyone else, wants to do it. (And, FWIW, I disagree with hewitt's comment about the meaning of ThreeDLightShadow, although I admit the spec doesn't make much sense.)
Target Milestone: mozilla0.9 → Future
Ideally, ThreeDLightShadow would be a shade in between ThreeDFace and ThreeDShadow. It certainly should NOT be the same as ThreeDHighlight, as it is now. You'll notice that because of this, classic theme buttons under GTK look a little too bold on the top-left corner. I may just have to switch all references to ThreeDLightShadow back to ThreeDFace in the mean time.
I researched how gtk paints shadows a bit more, and gtk_default_draw_shadow, for GTK_SHADOW_OUT, draws 4 1-pixel lines: * top/left outside: style->light * top/left inside: style->bg * bot/right inside: style->dark * bot/right outside: style->black So, with the way you're interpreting the meaning of the CSS2 system colors (which has no grounding whatsoever in the CSS spec (which is totally unclear), but does to some degree in the Windows documentation on system colors, you're right. I should bring up the issue of what the system colors really mean in the CSS WG. Patch coming up...
Right. I made it a shade between face and highlight when when I did the Mac version of nsLookAndFeel. The spec says: ThreeDDarkShadow Dark shadow for three-dimensional display elements. ThreeDFace Face color for three-dimensional display elements. ThreeDHighlight Highlight color for three-dimensional display elements. ThreeDLightShadow Light color for three-dimensional display elements (for edges facing the light source). ThreeDShadow Dark shadow for three-dimensional display elements. Note there are 5 colours defined, ostensibly ordered like this: DarkShadow, Shadow, LightShadow, Face and Highlight. This makes no sense because its unbalanced. Why 3 shadows and 1 highlight? Also, the descriptions for ThreeDDarkShadow and ThreeDShadow are the same. I think this section of the spec is suffering from bad copying and pasting... I cc:'d Ian Hixie for comments, maybe he can take it back to the CSS working group...? ThreeDLightShadow is desribed as facing the light source, so logically it should fall between highlight and face colours. ("shadow", is a really bad name, it should be something like "ThreeDLighterHighlight and ThreeDDarkerHighlight... that's what the descriptions imply). Ignoring the duplicate description of the DarkShadow and Shadow, reading the * descriptions* rather than the *names* gives DarkShadow, Shadow, Face, LightShadow, Highlight. This is what I implemented in the Mac version, it makes far more sense...
Right. I made it a shade between face and highlight when when I did the Mac version of nsLookAndFeel. The spec says: ThreeDDarkShadow Dark shadow for three-dimensional display elements. ThreeDFace Face color for three-dimensional display elements. ThreeDHighlight Highlight color for three-dimensional display elements. ThreeDLightShadow Light color for three-dimensional display elements (for edges facing the light source). ThreeDShadow Dark shadow for three-dimensional display elements. Note there are 5 colours defined, ostensibly ordered like this: DarkShadow, Shadow, LightShadow, Face and Highlight. This makes no sense because its unbalanced. Why 3 shadows and 1 highlight? Also, the descriptions for ThreeDDarkShadow and ThreeDShadow are the same. I think this section of the spec is suffering from bad copying and pasting... I cc:'d Ian Hixie for comments, maybe he can take it back to the CSS working group...? ThreeDLightShadow is desribed as facing the light source, so logically it should fall between highlight and face colours. ("shadow", is a really bad name, it should be something like "ThreeDLighterHighlight and ThreeDDarkerHighlight... that's what the descriptions imply). Ignoring the duplicate description of the DarkShadow and Shadow, reading the * descriptions* rather than the *names* gives DarkShadow, Shadow, Face, LightShadow, Highlight. This is what I implemented in the Mac version, it makes far more sense...
These colors are basically stolen from the Windows System Color API, where what CSS calls ThreeDLightShadow is called COLOR_3DLIGHT. See [1] http://msdn.microsoft.com/library/psdk/sysmgmt/sysinfo_9b1u.htm [2] http://support.microsoft.com/support/kb/articles/Q139/2/91.asp I just sent a message to the CSS working group ( http://lists.w3.org/Archives/Member/w3c-css-wg/2001JanMar/0265.html ) where I proposed the following new definitions: ButtonFace The face background color for 3-D elements that appear 3-D due to one layer of surrounding border. ButtonHighlight The color of the border facing the light source for 3-D elements that appear 3-D due to one layer of surrounding border. ButtonShadow The color of the border away from the light source for 3-D elements that appear 3-D due to one layer of surrounding border. ThreeDDarkShadow The color of the darker (generally outer) of the two borders away from the light source for 3-D elements that appear 3-D due to two concentric layers of surrounding border. ThreeDFace The face background color for 3-D elements that appear 3-D due to two concentric layers of surrounding border. ThreeDHighlight The color of the lighter (generally outer) of the two borders facing the light source for 3-D elements that appear 3-D due to two concentric layers of surrounding border. ThreeDLightShadow The color of the darker (generally inner) of the two borders facing the light source for 3-D elements that appear 3-D due to two concentric layers of surrounding border. ThreeDShadow The color of the lighter (generally inner) of the two borders away from the light source for 3-D elements that appear 3-D due to two concentric layers of surrounding border. I said "generally inner" and "generally outer" because I'm not sure how these colors are used in depressed buttons. Any thoughts on these proposals (which I believe match the Windows colors)?
Target Milestone: Future → mozilla0.9
Looks good to me. This is compatible with the way MacOS does things. I know Ian is trying to the the working group to drop system colours altogether for CSS-3 and developer a better solution. Both Windows XP and Aqua have features that are going to be impossible to do effectively, no matter how many flat colours are defined. Still, CSS2 errata should be produced for this sort of thing. Good job.
I guess this is the checkin that broke something. I'm using the blueHeart GTK theme in which windows are rendered in rather dark gray and all the text/menus etc. are in white. Mozilla did get these colors fine earlier (including the 0.8 release) but today (Build ID 2001021908) everything's written in black text (menus, buttons, other text) on dark gray background making things hard to read and look bad.
Your patch looks good, David. It fixes a problem with the border that I hadn't really noticed before... but it doesn't really solve the greater problem of ThreeDLightShadow borders being equal to ThreeDHighlight. In order to achieve parity with windows native buttons, we should change ThreeDLightShadow borders to ThreeDFace, which is the way they were previously. I only changed them to ThreeDLightShadow a few weeks ago because I thought it was equal to ThreeDFace (which it is on windows, just not GTK). Also, the wording of your CSS2 errata proposal is on the money, and makes much more sense than the current descriptions in the spec.
One of the patches above (attachment 25578 [details] [diff] [review]) fixes ThreeDLightShadow on GTK. So I guess I'm looking for r= on the current stuff (attachment 25578 [details] [diff] [review] and attachment 25581 [details] [diff] [review]), although I'll try to have something in a few days to fix the problem mentioned 2001-02-20 01:24.
OK, I read the GTK refcounting doc and those commented out unrefs aren't needed, since (essentially) gtk_container_add transfers the acquired refcount of the child to the container...
The reason for the changes in attachment 25771 [details] [diff] [review] is that we need to pick up the style rules in GTK themes that don't apply to all widgets. In particular, many of my changes to |text| were wrong, but just happened to work for all the themes I tested. The real trick in the Metal theme is that the menus and buttons have special rules to give them different colors, so I create menu and button widgets (just like the code for getting tooltip colors) and get the style information from them. This is done only once, and the results are stored in a static member for later use.
r=hewitt on the css patch
I have problem with the LCARS theme for GTK. I think this theme uses black background and black text for buttons. The button text is visible on the background bitmap. Already reported as <A href="http://bugzilla.mozilla.org/show_bug.cgi?id=68938">68938</A>.
*** Bug 68938 has been marked as a duplicate of this bug. ***
Now I downloaded 2001022221. Everything appears in black on black. Nice.. Well I expect that buttons should be in black on black if they miss the bitmap. But where's the bitmap then? I don't know why disabled buttons have yellow text, and button under mouse cursor (intensive?) has blue text. Where these colors came from? What if I had blue/yellow/any other background color set for the buttons? I don't know why menu has black text. As far as I can decipher from the theme, it should be violet. I don't know why all text is black(on black): Page source, location(the text field on the toolbar), bookmarks (both on the sidebar and the toolbar). I can't find any explicit definition in the theme but it is generally violet on black or dark violet in GTK apps. And the text fields ishould have borders.
r=bryner
sr=blizzard
Current patches checked in 2001-03-04 11:55 PST (css) and 12:15 PST (GTK). I still should investigate whether there are any more areas that need improvement.
As you may see on the screenshot, some widgets bacame readable and other are still all black. To be specific: text boxes, lists & trees (these do have a definition in the theme which makes them readable, at least they do not have black text), radio buttons and checkboxes. Disabled buttons are unreadable while disabled checkboxes use gray foreground(seems to be hardcoded, I haven't noticed any gray color in the theme).
I suspect this may have something to do with the -moz-field system color, possibly in combination with hardcoded or inappropriate colors. I'll try to investigate sometime...
And what do the buttons in your GTK theme look like?
Or should we not use buttons for the 3d highlight colors...?
David, if you're dealing with colors in GTK themes it might be a good idea to download some of the most popular GTK themes from themes.org and see for yourself how they behave...
A call to gtk_widget_realize fixes the problem with the messed up 3-D higlights in some themes, but causes a Gtk_CRITICAL assertion since the widget doesn't have a window. Yeah, I should've tested this on more themes than I did. If you think I should back it out, I will.
The window gave a 0-refcount Gtk-CRITICAL message when I tried to gtk_widget_unref it, and the Boehm GC didn't show any leaks with this latest patch, and the GTK refcounting doc also mentions something about using destroy rather than unref for windows (although it's not clear if that only applies to |GdkWindow|s). So I think the refcounting/destruction is right...
r=bryner
Please see bug 70315 - text in menus and boxes unreadable if using dark GTK theme
sr=blizzard
Fix for the 3-D borders problem checked in 2001-03-09 19:17 PST. Some further discussion on the dark-themes problem is on bug 70315.
I don't have time to deal with this issue now, and I fear overall I may have made things worse rather than better (partly because of strange usage of the colors within the classic theme). So I'm proposing that I back out all the changes I've checked in so far and perhaps someone else could check them in later after fixing the remaining problems (adding -moz-field-text, fixing the WindowText to do the right thing (see bug 70315), and fixing the classic skin not to use WindowText on a ThreeDFace background).
What attachment 28095 [details] [diff] [review] does is use the real foreground text color (|fg|) for WindowText, which is what we use for textboxes and for much of the text in dialogs, the personal toolbar, etc. This improves dark themes where the |text| color is black but the |fg| color is the correct light color. However, it hurts LCARS because we (in the Classic skin) use WindowText on a ThreeDFace background extensively (which I think is wrong, but I need to do some reverse engineering of Windows to check), and it hurts Metal (the problem I was originally trying to fix) because we're using WindowText in a way that the base steel-blue color in Metal probably wasn't originally designed to be used... Looking for reviews for this, at least for the trunk... (I'm not sure whether we will want a -moz-field-text color to replace the use of WindowText on a -moz-field background.)
I agree that WindowText on a ThreeDFace is wrong. Should probably use ButtonText instead.
I checked the way things work on Windows, and Windows dialogs actually use WindowText on a ThreeDFace background. So I think we need two new system colors: -moz-DialogFace and -moz-DialogText. On Windows -moz-DialogText would be the same as WindowText and -moz-DialogFace would be the same as ThreeDFace. Windows also always uses WindowText/Window for the text/background color inside textfields. I think we should have -moz-FieldText and -moz-Field (we already have -moz-Field). I'd also like to propose these for CSS3.
sr=blizzard on the backout patch
r=pavlov
a=asa
OK, backed out on branch. If I don't have time to fix it in the next few days I'll probably do the same for the trunk until I have time to fix it correctly...
See bug 53618. They may be using AppWorkSpace for the editor. We should figure out if that's right...
See bug 53723 for the history of -moz-Field.
Well, I'm looking for reviews on attachment 30073 [details] [diff] [review], attachment 30115 [details] [diff] [review], and attachment 30116 [details] [diff] [review]. For attachment 30116 [details] [diff] [review], I didn't yet reach all the obscure corners of the classic skin -- I went through the stuff in global carefully and then hit the important stuff in other places. There's probably still a little, although not that much, more to be done. I also made some decisions on which colors to use that may seem a little strange, but they make sense to me. I'm beginning to wonder if we also want 3-D border colors to correspond to the -moz-Dialog background color -- I repeatedly use -moz-Dialog surrounded by the button borders (which isn't a problem 98% of the time).
r=pierre
... on attachment 30073 [details] [diff] [review] (at least, that's what I asked him to review).
sr=hewitt on the themes part. Might be nice at some point to have highlight/shadow colors for -moz-Dialog, but you're probably right that most of the time, if ThreeDFace and -moz-Dialog differ, the ThreeDFace-based borders will look ok.
sr=blizzard
The bulk of the fix was checked in yesterday. I still need to go through some of the obscure parts of the classic skin more carefully, so moving to 0.9.1 for that.
Target Milestone: mozilla0.9 → mozilla0.9.1
Mozilla 2001-04-24-21 vith LCARS theme: Most vidgets are now usable. Disabled violet buttons have invisible text. Browser scrollbar buttons have invisible foreground. Some sidebar parts have fixed colors (bookmarks, tinderbox, what's related). In page source, part of the text is still invisible. Widgets look ugly and compared to native GTk apps, many of them are inversed. Rollover effects are different from GTK (none in most cases).
i'm not sure if this is related - i think so... anyway, i'm using the BHgtk gtk theme. with 'use system colors' checked, in mail i get white text on white background. unchecking 'use system colors' did the trick until today's builds (20010427, both the 11am and 10pm builds), when the 'use system colors' checkbox now seems to have no effect whatsoever...
Don't delete the cc: list.
sorry about that - didn't realize i had. i avoided playing with the settings up top but i guess i somehow screwed something up - hope it wasn't too much trouble just a followup - the 'use system colors' checkbox seems not to work for all themes as well (i picked 4 at random and tried them) but the white-on-white problem does not exist for all themes. also, i'm again not sure if this is related to the current bug, but mozilla doesn't get its theme updated when the gtk theme is updated. this requires a restart of mozilla - even reapplying the classic theme to mozilla seems not to have any effect.
By BHGtk, do you mean blueheart-GTK? It's a little hard for me to download a theme from http://gtk.themes.org/ if it's not listed there.
Yeah, you do. It calls itself one thing externally and another once you install it. How annoying. The problems in viewsource and in mail reading are pretty clear. I'll try and look into those and/or file separate bugs on them later. (There's also a problem with the caret not showing up well in BHGtk...) I'll try and look into these and other problems some more later, although I'm not going to have time in the next few weeks. (FWIW, I'm not quite as interested in the LCARS theme since the GNOME Panel doesn't even display legibly with it (which makes me think there's a problem with the theme), although I will look into the problems eventually.)
I reopened bug 62678 about the view source problems.
Depends on: 62678
Bug 69436 covers the mailnews issues.
Depends on: 69436
the LCARS theme: My version of gnome-panel is displayed quite well. I admit that apps creating (or modifying) their own widgets(including GNOME) have problems with this theme. It's because their authors didn't expect anything that weird could exist (like the authors of Mozilla Classic skin :-) and didn't include all the configurability of original (standard?) widgets in their modifications.
Adding dependency to bug 77828, which is about color prefs not working. The "Use system colors" being on (because the prefs aren't working) makes many webpages unreadable by default, since they assume dark text on light background as the default.
Depends on: 77828
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Target Milestone: mozilla0.9.5 → mozilla0.9.6
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Target Milestone: mozilla0.9.9 → Future
I downloaded an i686 nightly 20021027. The features as advertised on the Mozilla web site say that GTK widgets are correctly picked up using the Classic theme. The GTK widgets are working properly but the background colors for GTK widgets such as menubars, windows, etc. are not picking up the background colors correctly for GTK widgets I have set in my system which is Mandrake 9.0. I have checked several GTK apps on my system and verified that the background color is inherited correctly. I noticed that the latest Phoenix build does the GTK background colors correctly within the default Phoenix theme. Run Phoenix and Mozilla 1.2b side by side and you will see the widget background color problem.
Unrelated to this bug.
Summary: GTK system colors don't reflect reality → CSS2 system colors in GTK port don't reflect reality
The foreground color of widgets using base as background should be text, not fg.
Did you test with the Metal theme? (I think that's the one that has the textfields with purple-ish text on a white background.) If so, r=dbaron.
The patch makes the text in textfields black instead of purple when using the Metal theme. This is the same way as GtkText widgets look with the theme.
what's the status here? Should we file new bugs for remaining issues?
ok, I'll call this fixed. File new bugs for remaining issues.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Attachment #26851 - Attachment description: Here's a screenshot showing off both Mozilla and GTK+ -- the theme is Adept.Duo (an plain color RC file theme) with the Raleigh engine selected. It's a little more interesting, as buttons are a slightly lighter color (which happens to be reflected in the → Here's a screenshot showing off both Mozilla and GTK+ -- the theme is Adept.Duo (an plain color RC file theme) with the Raleigh engine selected.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: