Last Comment Bug 67448 - CSS2 system colors in GTK port don't reflect reality
: CSS2 system colors in GTK port don't reflect reality
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: x86 Linux
: P3 normal with 6 votes (vote)
: Future
Assigned To: David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
: John Morrison
Mentors:
: 68938 (view as bug list)
Depends on: 62678 69436 77828
Blocks:
  Show dependency treegraph
 
Reported: 2001-02-02 09:21 PST by David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
Modified: 2014-04-26 03:07 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.34 KB, patch)
2001-02-02 09:23 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
revised patch - also change buttontext (2.57 KB, patch)
2001-02-02 21:31 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch to change ThreeDLightShadow to match hewitt's interpretation of CSS2 (1.13 KB, patch)
2001-02-18 18:43 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
change to button.css needed to make this look right (1.21 KB, patch)
2001-02-18 19:33 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch that should be better across themes (8.94 KB, patch)
2001-02-20 20:18 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
A screenshot of the 'improved' look :) (44.88 KB, image/png)
2001-02-23 07:16 PST, Michal 'hramrach' Suchanek
no flags Details
new patch removing blocks-for-documentation (8.73 KB, patch)
2001-03-04 09:17 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
Screenshot of 2001-03-05-05 (64.24 KB, image/png)
2001-03-05 08:12 PST, Michal 'hramrach' Suchanek
no flags 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)
2001-03-05 14:02 PST, Vesa Halttunen
no flags 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)
2001-03-05 19:10 PST, Garrett LeSage
no flags 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)
2001-03-05 19:57 PST, Garrett LeSage
no flags Details
workaround (944 bytes, patch)
2001-03-06 08:21 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch that seems to fix the 3-D colors problem correctly (944 bytes, patch)
2001-03-06 18:42 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch to fix the 3-D colors problem properly (wrong patch last time) (1.52 KB, patch)
2001-03-06 19:42 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch to backout (9.15 KB, patch)
2001-03-19 05:47 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags 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)
2001-03-19 05:55 PST, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch to add 3 new system colors (-moz-FieldText, -moz-Dialog, -moz-DialogText) (11.97 KB, patch)
2001-04-08 07:21 PDT, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
test page showing correct foreground/background pairs (1.34 KB, text/html)
2001-04-08 07:33 PDT, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details
debugging code to debug system color use in skins (2.86 KB, patch)
2001-04-08 21:40 PDT, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags 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)
2001-04-08 21:44 PDT, David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
no flags Details | Diff | Splinter Review
Patch to make -moz-FieldText use text instead of fg (1.38 KB, patch)
2002-11-01 17:51 PST, Johan Tavelin
no flags Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-02 09:21:36 PST
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?
Comment 1 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-02 09:23:16 PST
Created attachment 24215 [details] [diff] [review]
patch
Comment 2 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-02 21:31:37 PST
Created attachment 24296 [details] [diff] [review]
revised patch - also change buttontext
Comment 3 Joe Hewitt (gone) 2001-02-02 22:52:10 PST
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?
Comment 4 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-15 21:42:56 PST
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)
Comment 5 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-16 07:02:55 PST
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...
Comment 6 Brian Ryner (not reading) 2001-02-16 07:05:53 PST
r=bryner
Comment 7 Christopher Blizzard (:blizzard) 2001-02-16 08:54:15 PST
r=blizzard
Comment 8 Stuart Parmenter 2001-02-16 14:00:06 PST
r=pavlov
Comment 9 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-18 12:45:47 PST
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.)
Comment 10 Joe Hewitt (gone) 2001-02-18 17:24:38 PST
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.
Comment 11 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-18 18:38:56 PST
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...
Comment 12 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-18 18:43:24 PST
Created attachment 25578 [details] [diff] [review]
patch to change ThreeDLightShadow to match hewitt's interpretation of CSS2
Comment 13 Andrew Thompson 2001-02-18 18:47:39 PST
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...
Comment 14 Andrew Thompson 2001-02-18 18:49:31 PST
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...
Comment 15 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-18 19:16:10 PST
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)?
Comment 16 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-18 19:33:14 PST
Created attachment 25581 [details] [diff] [review]
change to button.css needed to make this look right
Comment 17 Andrew Thompson 2001-02-18 20:09:26 PST
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.
Comment 18 Vesa Halttunen 2001-02-20 01:24:48 PST
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.
Comment 19 Joe Hewitt (gone) 2001-02-20 14:40:32 PST
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.
Comment 20 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-20 18:36:50 PST
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.
Comment 21 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-20 20:18:22 PST
Created attachment 25771 [details] [diff] [review]
patch that should be better across themes
Comment 22 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-20 21:01:55 PST
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...
Comment 23 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-20 21:09:27 PST
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.
Comment 24 Joe Hewitt (gone) 2001-02-20 23:35:30 PST
r=hewitt on the css patch
Comment 25 Michal 'hramrach' Suchanek 2001-02-21 03:06:16 PST
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>.
Comment 26 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-02-22 11:49:12 PST
*** Bug 68938 has been marked as a duplicate of this bug. ***
Comment 27 Michal 'hramrach' Suchanek 2001-02-23 07:09:20 PST
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.
Comment 28 Michal 'hramrach' Suchanek 2001-02-23 07:16:54 PST
Created attachment 26017 [details]
A screenshot of the 'improved' look :)
Comment 29 Brian Ryner (not reading) 2001-02-25 13:22:50 PST
r=bryner
Comment 30 Ben Goodger (use ben at mozilla dot org for email) 2001-02-27 21:35:48 PST
a=ben@netscape.com for the css patch
Comment 31 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-04 09:17:25 PST
Created attachment 26730 [details] [diff] [review]
new patch removing blocks-for-documentation
Comment 32 Christopher Blizzard (:blizzard) 2001-03-04 09:26:34 PST
sr=blizzard
Comment 33 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-04 12:17:43 PST
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.
Comment 34 Michal 'hramrach' Suchanek 2001-03-05 08:12:28 PST
Created attachment 26785 [details]
Screenshot of 2001-03-05-05
Comment 35 Michal 'hramrach' Suchanek 2001-03-05 09:22:02 PST
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).
Comment 36 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-05 11:48:31 PST
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...
Comment 37 Vesa Halttunen 2001-03-05 14:02:30 PST
Created attachment 26806 [details]
Screenshot (2001030508) showing the BHgtk theme. Now the texts are white again as they should be but all 3D highlights are gone.
Comment 38 Garrett LeSage 2001-03-05 19:10:58 PST
Created attachment 26843 [details]
Screenshot, after the commit went through. 3d highlights are now all black (not a good thing). It looked fine before. *grin*
Comment 39 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-05 19:17:02 PST
And what do the buttons in your GTK theme look like?
Comment 40 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-05 19:17:48 PST
Or should we not use buttons for the 3d highlight colors...?
Comment 41 Garrett LeSage 2001-03-05 19:57:42 PST
Created attachment 26851 [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.
Comment 42 Vesa Halttunen 2001-03-06 01:23:03 PST
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...
Comment 43 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-06 07:28:36 PST
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.
Comment 44 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-06 08:21:36 PST
Created attachment 26889 [details] [diff] [review]
workaround
Comment 45 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-06 18:42:11 PST
Created attachment 26972 [details] [diff] [review]
patch that seems to fix the 3-D colors problem correctly
Comment 46 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-06 19:42:39 PST
Created attachment 26977 [details] [diff] [review]
patch to fix the 3-D colors problem properly (wrong patch last time)
Comment 47 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-06 22:21:40 PST
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...
Comment 48 Brian Ryner (not reading) 2001-03-08 19:15:27 PST
r=bryner
Comment 49 Damian Christey 2001-03-08 22:08:26 PST
Please see bug 70315 - text in menus and boxes unreadable if using dark GTK theme
Comment 50 Christopher Blizzard (:blizzard) 2001-03-09 10:49:21 PST
sr=blizzard
Comment 51 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-10 12:52:25 PST
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.
Comment 52 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-19 05:43:12 PST
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).
Comment 53 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-19 05:47:32 PST
Created attachment 28094 [details] [diff] [review]
patch to backout
Comment 54 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-19 05:55:01 PST
Created attachment 28095 [details] [diff] [review]
alternate patch that may be the "right thing" but messes up LCARS even more b/c of classic skin's color use
Comment 55 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-19 12:24:13 PST
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.)
Comment 56 Joe Hewitt (gone) 2001-03-19 13:18:46 PST
I agree that WindowText on a ThreeDFace is wrong.  Should probably use
ButtonText instead.
Comment 57 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-19 17:33:45 PST
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.
Comment 58 Christopher Blizzard (:blizzard) 2001-03-19 18:36:11 PST
sr=blizzard on the backout patch
Comment 59 Stuart Parmenter 2001-03-19 19:15:04 PST
r=pavlov
Comment 60 Asa Dotzler [:asa] 2001-03-19 19:28:32 PST
a=asa 
Comment 61 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-19 19:52:59 PST
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...
Comment 62 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-20 08:08:35 PST
See bug 53618.  They may be using AppWorkSpace for the editor.  We should figure
out if that's right...
Comment 63 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-03-20 11:33:30 PST
See bug 53723 for the history of -moz-Field.
Comment 64 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-08 07:21:56 PDT
Created attachment 30073 [details] [diff] [review]
patch to add 3 new system colors (-moz-FieldText, -moz-Dialog, -moz-DialogText)
Comment 65 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-08 07:33:11 PDT
Created attachment 30074 [details]
test page showing correct foreground/background pairs
Comment 66 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-08 21:40:39 PDT
Created attachment 30115 [details] [diff] [review]
debugging code to debug system color use in skins
Comment 67 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-08 21:44:43 PDT
Created attachment 30116 [details] [diff] [review]
patch to fix system color usage in classic skin and HTML form controls and fix CSS errors in skins
Comment 68 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-08 21:50:34 PDT
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).
Comment 69 Pierre Saslawsky 2001-04-11 23:18:28 PDT
r=pierre
Comment 70 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-12 20:02:42 PDT
... on attachment 30073 [details] [diff] [review] (at least, that's what I asked him to review).
Comment 71 Joe Hewitt (gone) 2001-04-15 13:32:51 PDT
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.
Comment 72 Christopher Blizzard (:blizzard) 2001-04-16 14:16:38 PDT
sr=blizzard
Comment 73 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-17 18:35:37 PDT
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.
Comment 74 Michal 'hramrach' Suchanek 2001-04-25 06:32:24 PDT
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).
Comment 75 Brian Tarricone 2001-04-28 01:18:38 PDT
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...
Comment 76 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-28 06:47:33 PDT
Don't delete the cc: list.
Comment 77 Brian Tarricone 2001-04-28 10:19:25 PDT
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.
Comment 78 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-28 10:47:51 PDT
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.
Comment 79 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-28 10:58:59 PDT
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.)
Comment 80 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-28 11:13:29 PDT
I reopened bug 62678 about the view source problems.
Comment 81 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-04-28 11:22:14 PDT
Bug 69436 covers the mailnews issues.
Comment 82 Michal 'hramrach' Suchanek 2001-05-02 06:21:33 PDT
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.
Comment 83 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2001-05-02 13:33:11 PDT
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.
Comment 84 mark bokil 2002-10-29 06:39:31 PST
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.
Comment 85 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2002-10-29 07:17:31 PST
Unrelated to this bug.
Comment 86 Johan Tavelin 2002-11-01 17:51:14 PST
Created attachment 104917 [details] [diff] [review]
Patch to make -moz-FieldText use text instead of fg

The foreground color of widgets using base as background should be text, not
fg.
Comment 87 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2002-11-01 18:05:01 PST
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.
Comment 88 Johan Tavelin 2002-11-01 23:23:45 PST
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.
Comment 89 basic 2004-03-04 06:57:36 PST
what's the status here? Should we file new bugs for remaining issues?
Comment 90 Julien "_FrnchFrgg_" RIVAUD 2008-01-13 17:35:10 PST
ping ?
Comment 91 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2008-01-13 17:55:27 PST
ok, I'll call this fixed.  File new bugs for remaining issues.

Note You need to log in before you can comment on or make changes to this bug.