Closed Bug 1004 Opened 26 years ago Closed 24 years ago

should support CSS2 system colors

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

PowerPC
All
enhancement

Tracking

()

VERIFIED FIXED
mozilla0.9

People

(Reporter: angus, Assigned: pierre)

References

()

Details

(Keywords: css2, Whiteboard: Verify Me)

Attachments

(16 files)

1.49 KB, text/plain
Details
17.93 KB, text/plain
Details
2.02 KB, text/plain
Details
11.51 KB, patch
Details | Diff | Splinter Review
533 bytes, patch
Details | Diff | Splinter Review
8.02 KB, patch
Details | Diff | Splinter Review
593 bytes, patch
Details | Diff | Splinter Review
1.07 KB, text/html
Details
1.10 KB, text/plain
Details
5.88 KB, text/plain
Details
1.75 KB, text/plain
Details
70.32 KB, image/jpeg
Details
7.87 KB, patch
Details | Diff | Splinter Review
23.67 KB, image/png
Details
7.87 KB, patch
Details | Diff | Splinter Review
7.72 KB, patch
Details | Diff | Splinter Review
CSS2 provides a mechanism for the author to designate a color based on
aliases to system color defaults, rather than an explicit color name or hex
value. Specified values are outlined at the above URL.

Support for this would be particularly helpful to those trying to build an
appliction interface with the native look and feel using only HTML/CSS.
Status: NEW → ASSIGNED
See also bug 987.  (Should they be used as default colors for forms? - I think
so.)
Setting all current Open/Normal to M4.
Target Milestone: M4 → M7
Hardware: PC → All
Summary: CSS2 System Color values not implemented → CSS2 System Color and font values not implemented
Amended subject to add fonts also (as listed in CSS2 spec, sec 15.2.5). The
list of font values in the spec is rather restrictive; controlling fonts in
various places in the UI will probably require a few more values.

All the dialog work is dependent on this, in the sense that dialog layout cannot
be finalized until we can display the correct fonts. I would thus hope that
this could be moved to M5 or M6 if possible.
Bug 4951 has been closed on the basis of this bug.
Severity: normal → enhancement
Priority: P2 → P1
Summary: CSS2 System Color and font values not implemented → should support CSS2 system colors
See also bug 3371, relating to system fonts.

Setting severity/priority to enhancement/P1. This is an RFE but is required
for the UI stuff.
Blocks: 1763
Target Milestone: M9 → M10
I would hope this doesn't get put off too much longer, especially since the
Prefs UI is coming soon.
Summary: should support CSS2 system colors → {css2} should support CSS2 system colors
Blocks: 1021
Assignee: peterl → rods
Status: ASSIGNED → NEW
Style code now supports system colors. Transferring to rods to add CSS colors to
nsILookAndFeel
Target Milestone: M10 → M12
Changing to M12
*** Bug 4605 has been marked as a duplicate of this bug. ***
No longer blocks: 1763
No longer blocks: 1021
Depends on: 1021
No longer depends on: 1021
Blocks: 1021
Can this be marked as fixed now?
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Using 10/20 app with 7/27 test case, verified fixed. Email sent to rods, py8ieh,
and dbaron to clarify questions regarding CSS spec definition of
ButtonHighlight and Scrollbar implementation (not included in the test case).
Status: VERIFIED → REOPENED
Summary: {css2} should support CSS2 system colors → [PP] {css2} should support CSS2 system colors
Resolution: FIXED → ---
Previous verification of fix was based on Win95 testing. Realized this is cross
platform. Reopening bug and now labeling it a parity bug as values are not
working correctly on Linux or Mac.
OS: Windows 95 → Linux
see bug #16245 for Mac.
Assignee: rods → pavlov
Status: REOPENED → NEW
Thanks Michael, reassigning this bug to pavlov for Linux.
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
i did this long ago.
Status: RESOLVED → REOPENED
I'm assuming "long ago" means before 8AM PDT yesterday, and reopening.

http://www.fas.harvard.edu/~dbaron/css/test/sec1802 doesn't look too good.  If
you need a screenshot, I can attach one.
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Target Milestone: M12 → M11
mass-moving most m11 bugs to m12
Target Milestone: M12 → M11
M11
Blocks: 17907
Whiteboard: will check in fix for unix as soon as tree opens for M12
Target Milestone: M11 → M12
Assignee: pavlov → pinkerton
Status: ASSIGNED → NEW
fixed on unix.  pink, your turn.
ok, so what do i have to do here? what values are wrong? what should they be?
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → DUPLICATE
*** This bug has been marked as a duplicate of 16245 ***
This bug still needs to be verified on Win and Linux.  Also, the mileston for
bug 16245 should be changed and the deps should be updated, unless you want to
resolve as duplicate the other way round, which might be better...
I'd vote for keeping stuff in this bug, too, since the other platforms aren't
verified. (And besides, bug 16245 was marked M20, and I can't wait that long for
native colors on Mac.:-)

For the Mac, you need to hook up CSS's default colors, fonts, and mouse pointers
(and sounds too, if CSS supports sounds ... which it probably doesn't) from the
Appearance Manager.
http://developer.apple.com/techpubs/macos8/HumanInterfaceToolbox/AppManager/
ProgWithAppearanceMgr/

And more specifically,
http://developer.apple.com/techpubs/macos8/HumanInterfaceToolbox/AppManager/
ProgWithAppearanceMgr/Appearance.4.html
ummm, ok. someone need to tell me how to map the appearance colors to the win32
specific CSS2 colors. There is an impl on mac, but i don't know if it is correct
at all. Right now the colors are just guesses and hard-coded. If someone tells me
the right colors to hard-code, I'll use those instead. Hooking it up to the AM
won't come before m20, unless someone else does the work.....
Status: RESOLVED → REOPENED
For hard-coding, assuming the Apple Platinum theme (default with MacOS 8), this
is what my screen grabber and Photoshop tell me.

ActiveBorder: #000000
ActiveCaption: #000000
AppWorkspace: N/A
Background: highly variable (just use a darkish grey)
ButtonFace: #dedede
ButtonHighlight: #ffffff
ButtonShadow: #737373
ButtonText: #000000
CaptionText: #000000
GrayText: #767676
Highlight: #ccccff
HighlightText: #000000
InactiveBorder: #525252
InactiveCaption: #dedede
InactiveCaptionText: #767676
InfoBackground: #ffffff for Help Balloons (but you probably want to use #ffffaa
  or something, I suppose)
InfoText: #000000
Menu: #dedede
MenuText: #000000
Scrollbar: #adadad
ThreeDDarkShadow: #9c9c9c
ThreeDFace: #dedede
ThreeDHighlight: N/A
ThreeDLightShadow: #ffffff
ThreeDShadow: #9c9c9c
Window: #dedede
WindowFrame: #cecece
WindowText: #000000

Urgh. Where's the ScrollbarThumb property? :-)

And I don't know how you're mapping CSS's font families to font families suitable
for UI display ... but default fonts for MacOS 8 are Charcoal for headings,
buttons, and menus (including combo-box menus); Geneva for general UI text; and
Geneva also for list views (such as the tree widget).

-- mpt (who's only doing this because he knows it's going to be temporary)
*** Bug 16245 has been marked as a duplicate of this bug. ***
Resolution: DUPLICATE → ---
Status: REOPENED → ASSIGNED
Whiteboard: will check in fix for unix as soon as tree opens for M12
clearing status whiteboard. accepting bug.

thanks a lot for the colors! I'll get them in shortly!!! I love free source ;)
Priority: P1 → P3
marking P3 and M20 like bug #16245 was (which was closed for some unknown
reason). I'll get to pulling the colors out of the appearance manager at some
point, but certainly post beta.
ok, colors from mpt26@student.canterbury.ac.nz checked in, and they look ok i
guess. minor tweaking required. see nsLookAndFeel.cpp.

keeping open as remined to do AM colors. if you want to close out this bug for
win32 and linux so you can verify it, maybe you shouldn't have closed out my
personal placeholder bug ;)
Target Milestone: M12 → M20
moving to M20.
Keywords: css2
Migrating from {css2} to css2 keyword. The {css1}, {css2}, {css3} and {css-moz}
radars should now be considered deprecated in favour of keywords.
I am *really* sorry about the spam...
Keywords: pp
Summary: [PP] {css2} should support CSS2 system colors → should support CSS2 system colors
Some Mac programmer is going to see beta 1 and want to fix this. (I hope.) 
Marking helpwanted.
Keywords: helpwanted
OS: Linux → All
Mass move of all M20 bugs to M30.
Mass moving M20 bugs to M30
Target Milestone: M20 → M30
If Mozilla is going to be taken seriously by the average Mac user, this has to be 
fixed by M20. (Just my $0.02 ...)
CSS2 is missing a color that would correspont to the UI accent color defined in 
Appearance control panel. Until W3C fixes the spec, that color should be accessible via a 
Mozilla-only CSS extension for use in chrome.
Just to expand on Henri's comment, the Appearance Manager highlight color is 
needed for system-savvy:
* progress indicators
* scrollbars
* sliders
* disclosure triangles (aka `twisties')
* focus rings (borders around the currently active widget, such as have started
  appearing in Mozilla on all platforms)
* menus and popup menus.

The current CSS3 draft for UI colors doesn't look promising in this regard
<http://www.w3.org/TR/2000/WD-css3-userint-20000216.html#color>, so if you can 
pull strings at the W3C now is the time to do it.
Blocks: 4252
The CSS color referring to the Apple Platinum accent color should refer to some 
meaningful color on other platforms, too.

Windows: the color used in solid-color window titlebars and in progress meters
BeOS: the color used in window title tabs
GTK: ??? Can apps get the main color of the active theme form GTK?
Mass-moving all M20-M30 XPToolkit bugs to Future
Target Milestone: M30 → Future
Henri, on Windows the highlight color (as used in progress meters and menus) 
happens to be exactly the same as the selection color. On Unix, the highlight 
color is not used for anything much, except (sometimes) to fill in checked 
checkboxes and radio buttons.
Blocks: 39375
The basics (at least) of system color support appears to be there on Linux and
Windows. Changing platform, OS to Mac.
OS: All → Mac System 9.0
Hardware: All → Macintosh
No longer blocks: 17907
I can't build on Mac or I'd be fixing this right now!
Two things need to happen:
We need to agree an extra constants for the missing concept of Highlight/
Variation colour and put it in:
mozilla/source/widget/public/nsILookAndFeel.h

Then someone needs to edit:
http://lxr.mozilla.org/mozilla/source/widget/src/mac/nsLookAndFeel.cpp

and retreive Mac specific colours from the AppearnceManager by doing

GetThemeBrushAsColor(kThemeBrushUtilityWindowBackgroundActive, 32, true, &macRGB)

The mapping is the hard part - the fix itself is trivial. Hell, I'll even do the 
typing if someone else can compile the file for me, though that's verging on the 
ridiculous!

What I will try to find time to do is write a program that dumps all of the 
ThemeBrush colours as hex. Then we should have an easier time determining the 
mapping. 
Blocks: 46174
OK, I attached the program so others can try it out with different Appearance 
themes installed, if they have a burning desire to do so. (www.macthemes.org)

This page lists all the theme brush constant names:
http://developer.apple.com/techpubs/macos8/HumanInterfaceToolbox/AppManager/﷒0﷓

This page lists all the theme text names:
http://developer.apple.com/techpubs/macos8/HumanInterfaceToolbox/AppManager/﷒1﷓

You can cross reference them with the output of the Java program I attached, when 
run on my Mac (which has the standard Platinum theme active)

Theme brushes
1 R dd G dd B dd
2 R dd G dd B dd
3 R dd G dd B dd
4 R dd G dd B dd
5 R dd G dd B dd
6 R dd G dd B dd
7 R ff G ff B ff
8 R ff G ff B ff
9 R dd G dd B dd
10 R ee G ee B ee
11 R ff G ff B ff
12 R ff G ff B ff
13 R 0 G 0 B 0
14 R 99 G 99 B 0
15 R ff G ff B ff
16 R ff G ff B ff
17 R 0 G 0 B 0
18 R 55 G 55 B 55
19 R 99 G 99 B 0
20 R 0 G 0 B 0
21 R ff G ff B ff
22 R 88 G 88 B 88
23 R ff G 0 B 0
24 R 0 G 0 B 0
25 R ee G ee B ee
26 R ff G ff B ff
27 R 0 G 0 B 0
28 R 88 G 88 B 88
29 R dd G dd B dd
30 R dd G dd B dd
31 R 66 G 66 B 66
32 R 77 G 77 B 77
33 R dd G dd B dd
34 R 99 G 99 B 99
35 R ff G ff B ff
36 R dd G dd B dd
37 R dd G dd B dd
38 R dd G dd B dd
39 R dd G dd B dd
40 R 44 G 44 B 44
41 R 88 G 88 B 88
42 R 55 G 55 B 55
43 R 77 G 77 B 77
44 R ff G ff B ff
45 R 88 G 88 B 88
46 R dd G dd B dd
47 R bb G bb B bb
48 R ff G ff B c6
Text Theme colours
1 R 0 G 0 B 0
2 R 77 G 77 B 77
3 R 0 G 0 B 0
4 R 77 G 77 B 77
5 R 0 G 0 B 0
6 R 77 G 77 B 77
7 R 0 G 0 B 0
8 R 77 G 77 B 77
9 R 0 G 0 B 0
10 R 77 G 77 B 77
11 R ff G ff B ff
12 R 0 G 0 B 0
13 R 77 G 77 B 77
14 R ff G ff B ff
15 R 0 G 0 B 0
16 R 77 G 77 B 77
17 R ff G ff B ff
18 R 0 G 0 B 0
19 R 77 G 77 B 77
20 R ff G ff B ff
21 R 0 G 0 B 0
22 R 0 G 0 B 0
23 R 0 G 0 B 0
24 R 77 G 77 B 77
25 R 0 G 0 B 0
26 R 77 G 77 B 77
27 R 0 G 0 B 0
28 R 77 G 77 B 77
29 R 0 G 0 B 0
30 R 77 G 77 B 77
31 R 0 G 0 B 0
32 R ff G ff B ff
33 R 77 G 77 B 77
34 R 0 G 0 B 0
35 R ff G ff B ff
36 R 77 G 77 B 77
37 R 0 G 0 B 0
38 R 77 G 77 B 77
39 R 0 G 0 B 0
40 R 0 G 0 B 0
41 R ff G ff B ff
42 R 77 G 77 B 77
43 R 77 G 77 B 77
44 R ff G ff B ff
45 R 0 G 0 B 0
46 R 77 G 77 B 77
47 R 0 G 0 B 0
I had a go at matching up the colours. Its not easy! There's a serious disconnect 
between Mozilla/web paradigm and the Mac/Appearence manager, which is severly 
agravated in that the CSS2 system colours are just straight word for word rip 
offs of the Windows system colours. (How on earth did they ratify something so 
sloppy?)




The thin border around a window.  Mostly meaningless on Mac OS
ActiveBorder	= kThemeBrushBlack

Colour on window titlebar

This is a tough one. The Appearence manager is set up s.t. you draw windows by 
making function
calls, not asking it what colour it would choose to draw a window in. In the 
themes I looked
at, it seems the closest match for this is the colour of an unselected Tab, but 
you can't 
get that colour either :-( Any ideas?

ActiveCaption   = ?? (cccccc in platinum)

MDI window background. Utterly Meaningless on Mac OS.
AppWorkspace:   = kThemeBrushDocumentWindowBackground  (white)

The desktop background. Since when was this *ever* a solid colour on any 
platform.
(I know it can be on Windows, Unix an Mac OS but almost everyone has a pattern or 
picture)
Background: 	= light blue?  (completely and utterly meaningless on Mac OS)

Push button
ButtonFace: kThemeBrushButtonFaceActive 

Push button highlight (lighter of 2 tone highlight on Mac)
ButtonHighlight: kThemeBrushButtonActiveLightHighlight (white)

Push button shadow (darker of two tone highlight on the mac)
ButtonShadow: kThemeBrushButtonActiveDarkShadow (dddddd)

Text on buttons
ButtonText: kThemeTextColorPushButtonActive (black)

Text in Window titlebars, etc
CaptionText: kThemeTextColorWindowHeaderActive (black)

Disabled text
GrayText: kThemeTextColorDialogInactive (777777)

"Selected items in a control" (who wrote this drivel into the CSS2 spec? Just 
what is that
supposed to mean? One assumes its the highlight colour for selected text)
Highlight: Moz alreay has a GetPortHiliteColor function, that Pinkerton wrote.
			This CSS2 color is already wired up and working.

Text in selected items in a control
HighlightText: kThemeTextColorBlack  (appears to always be black. 
									Tried an alternate Appearence Theme to make 
sure)

Color on inactive border around window.

InactiveBorder: kThemeBrushScrollBarDelimiterInactive (555555 in platinum) 

Colour of an inactive window title bar. Same problem as above
InactiveCaption: ????     in platinum dddddd or maybe 
kThemeBrushDialogBackgroundInactive

Text on titlebar of a window in the background
InactiveCaptionText:     kThemeTextColorWindowHeaderInactive  (777777)

Tooltips!
InfoBackground: kThemeBrushNotificationWindowBackground (MAC OS 9 ONLY!)
You probably can't use this constant as Nav6 will support Mac OS < 9, so
instead use its value under Platinum which is ffffc6

InfoText: kThemeTextColorNotification (MAC OS 9 ONLY!)
You probably can't use this constant as Nav6 will support Mac OS < 9, so
instead use its value under Platinum which is black

Same problem as for window titlebars
Menu: ?????  use dddddd or kThemeBrushDialogBackgroundActive

Text in menus
MenuText: kThemeTextColorMenuItemActive

Scrollbar trough - same problem as for window titlebars. 
Scrollbar: ??  As Mark Thomas said, use adadad

The dark shadow on a 3D object
ThreeDDarkShadow: kThemeBrushButtonActiveDarkShadow  (777777)

ThreeDFace: kThemeBrushButtonFaceActive  (dddddd)

ThreeDHighlight: kThemeBrushButtonActiveLightHighlight  (ffffff)

This is described as being on the sie *facing* the light, so its not a shadow, 
its a highlight!
ThreeDLightShadow: kThemeBrushButtonActiveDarkHighlight  (dddddd)

Desciption of this one in CSS2 has to be a typo - its identical to 
ThreeDDarkShadow
ThreeDShadow: kThemeBrushButtonActiveLightShadow (999999)

Window background
Window: kThemeBrushDocumentWindowBackground

WindowFrame: cccccc  (same problem as ever)

WindowText: kThemeTextColorDialogActive (black) would seem to be safest
The real problems here for proper Mac OS colour support and Appearence compliance 
for themes in general are way outside the scope of this bug, or any such simple 
fix.

As Matthew would say: I refer the interested reader to bug 39375 (platform 
consistent look and feel for widgets) and bug 31726 (tinting of graphics)

The idea of adding one colour to Mozilla to handle Mac scrollbar thumb's isn't 
going to work. Mac scrollbar thumbs use 3 colours in Platinum and an arbitrary 
number in other themes. A true fix is stage 3 of the above mentioned bug 39375

That said - 99.9% of Mac users probably use Platinum (but of course Aqua is 
iminent!) so if we can capture the

ScrollbarThumbFace
ScrollbarThumbHighlight and
ScrollbarThumbShadow				(names I just made up on the spot)

that would be good enough for Nav6.0. Actually, the only one you can get from 
appearence is the ScrollbarThumbShadow (its the focus ring colour, among other 
things) so you'd either have to calculate the other two colours from this one 
(!!!) or hack up a way to get all three.

I just asked about this on the HIT developers list at Apple so hopefully GuyF and 
PeterG are feeling helpful...
Further work on Mac scrollbar colours now has its own bug 46174

Still need CSS2 System Colour implementation for standard CSS2 colours, however.
I just threw up some code to implement CSS2 system colours on the Mac.
This code will not work.

i) I'm not a C++ developer. Syntax errors will exist
ii) I can't build on Mac OS. That's right, I've never even tried to compile this!

However the bulk of the leg work doing the typing has been done, so someone 
should be able to complete from here.

Notes:

1) I used the mapping I proposed above. No one seems to have disagreed yet.

2) Pay careful attention to any Netscape specific coding conventions I may have 
broken in my ignorance.

3) Specifically, look at how I #include <Appearance.h> in the .h file. I have no 
idea whether its OK to include platform headers in this file.

4) Its never been compiled. There will be embarassing problems.

Blocks: 49144
A way to access the focus outline color is needed for bug 50804.
Blocks: 50804
Hixie thinks you need to see this Pierre
Hixie's changes to html.css last night have suddenly made this bug go from latent 
to extremely obvious. So its getting nsbeta3 nominated in spite of the deadline.

Probably should be nsmac1 or nsmac2 also.
Keywords: nsbeta3
cc jrgm
What exactly are you seeing?  What are the steps to repro?  Why should this be
nsmac1?
Whiteboard: [need info]
Conferred with Hixie and he says its not as serious as he feared.
Clearing nsbeta3 nomination.

I intend to fix this if I ever get a working build with CW6
Keywords: nsbeta3
Assignee: pinkerton → pierre
Status: ASSIGNED → NEW
wow, i didn't know i owned this bug. pierre is much more passionate about it,
perhaps he should own it.
NB: we should also get the page background and link color defaults from Internet 
Config on the Mac. See comments in bug 19329.
Nominating for Mozilla 1.0.
Keywords: mozilla1.0
Much improved patch which should fix this bug - though I'm sure we can argue a 
bit more about which colours to use once we've seen this run.

This patch compiles, and should work, but its untested as I am still unable to 
build Mozilla completely on Mac OS (soon, I hope).

There may, of course, also be bugs in the CSS where the wrong CSS system colour 
is used in the Mac Classic skin?

Need someone who can build to apply this and try running Mozilla under some 
themes (www.macthemes.org - look in the new themes and theme archive sections) 
and see how it fairs...
Changing from helpwanted to review
Keywords: helpwantedreview
In response to lordpixel [2000-10-08 13:20]: yes, the Mac Classic skin uses 
images or hard-coded colors for the scrollbars and other UI elements.

Thanks a lot for the patch. I'll take a look and check it in after we ship.
Status: NEW → ASSIGNED
Attached file CSS file for testcase
OS: All
I can finally build so I tested my patch and it works pretty nicely. Try out the
test case and change the system theme then reload to see.

Most of the classic skin on Mac OS still uses hardcoded colours, so I fixed up
menu.css to use the system colours. Try this version in an alternative theme.

Some further comments:

1/ We need to listen for theme change events from the appearence manager and
force a refresh. I'll eventually open a seperate bug on this

2/ The CSS2 system colours are absolutely inadequate for the needs of the
classic skin

3/ The colours that the Mac apperence manager will give us are incomplete. Even
if we define lots of -moz- colours to fix 2, ee may have to make the decision
between leaving certain things hardcoded or mapping them to colours that "just
happen to work" in Platinum.

That said, I favour the "works in Platinum" approach because if we hardcode
we're 100% likely to be wrong in every other theme, whereas if we simply try to
use an appropriate colour that happens to map OK in Platinum we may be right or
close to right in other themes.
Apologies for the spam, but it starts to look so good if you use this that I
just had to post it. Try this out and see Mozilla's toolbar in other themes :-)

This is the last css fix I'll post on this bug. I'll open a proper bug and make
patches as and when I fix the rest of the classic css files.

BTW - oddly enough the form controls are already using the system colours, so
all webforms are appearence compliant (well they use vaguely the right colours
anyway). Natually this doesn't use the right textures :-|
Blocks: 57490
Whiteboard: [need info] → [review patch]
Target Milestone: Future → mozilla0.6
Pierre... you do know we can check this in on the trunk right now if you review 
and I russle up a super review?

I'm not trying to be funny - I've encountered a couple of Netscape employees in 
the last month who didn't know that they're allowed to checking reviewed and 
approved *external contributions* onto the trunk. The rules are actually looser 
for non-netscape people, since we can determine what we work on for ourselves.

I'd just hate to wait until after RTM for this for the wrong reasons. I have a 
bunch of other stuff in the works that depends on getting this checked in.

If you knew this already, apologies for wasting your time. And if [review patch] 
in the status whiteboard means you're planning to do it soon, then double 
apologies. I haven't seen that status before.
No longer blocks: 53927
Keywords: patch
erm. Pierre, you meant mozilla0.9, not mozilla0.6 right?

new feature checkin time is long since past :-( ...
I don't understand anything to all these milestones that come and go, and the doc 
is by far outdated (http://www.mozilla.org/projects/seamonkey/milestones/). 
Anyhow the way I'm using this popup, I'd be just as glad with color tags or even 
scent codes as in "uh oh, I'm sorry I let some external contributor sit here for 
so long; it smells like I should take care of it ASAP now". It's a little bit 
what I meant by mozilla0.6 but I may be wrong. Pushing to 0.9 then.
Target Milestone: mozilla0.6 → mozilla0.9
Ooops, please read "external contributor _patch_", of course...
I reviewed a copy of nsLookAndFeel.cpp and nsLookAndFeel.h that <lordpixel> sent 
me by email.  The code looks good except that in many instances, we have "res = 
NS_OK;" immediately followed by "res = this->GetMacTextColor()".  The first line 
should be removed everywhere, of course.

Then I installed the menu.css and toolbar.css attached to this bug report, 
rebuilt the jar archives and tried the Mac Classic skin.  I did not notice much 
difference, which is a good sign because I only have Platinum.  Two problems 
though:
- In menu.css line 84, we have "background-color: orange; #CCCCCC;" which looks 
like some debug code.
- The tooltips have a gray background instead of yellow. It could come from 
menu.css line 213 which reads "background-color: Info" instead of "background-
color: InfoBackground".

Conclusion:
- I can mark nsLookAndFeel.cpp/.h reviewed after the changes above. I recommend 
Simon Fraser <sfraser@netscape.com> for the super-review.
- Ben should be reviewer/super-reviewer for the css files.  They must be tested 
under Aqua or other themes, which I didn't do.

I'm going to attach an image that shows the colors before and after <lordpixel>'s 
changes using a Platinum theme.  (before = background window on the left, after = 
front window on the right).
Please read "I can mark nsLookAndFeel.cpp/.h reviewed after the changes above 
_are_made_". And of course many thanks to <lordpixel> for making the Mac better.
re: Pierre's other comments about the .css files. Yeah, those are pretty rough. 
There are some better examples hanging off of bug 46174. However even those 
changes are not complete. When I am happy with the .css revisions I'll attach 
them to bug 57490 which I opened to track the issue of fixing the CSS.

I'll go bug smfr about a super review.

(BTW www.macthemes.org if you want to test with other themes)
Hmmm, been looking at the themes on MacThemes.org and it seems they all have a 
bug where they return the colours of the wrong theme through GetThemeBrushAsColor 
etc. 

This is likely a bug in the Theme Machine tool that's under development at that 
site. Its based on reverse engineering existing themes. Platinum is aparently to 
highly customised, so they reverse engineered some other themes and it seems all 
their self created themes are returning the colours for the orginal theme that 
they originally reverse engineered. (this is partly guesswork)

I'll see if I can get them to fix this...
FYI: The primary problem is that the clr# resource in most of the themes from
macthemes.org is wrong. Its giving the colours for the wrong theme. I'm going to
raise it on their discussion list and stop spamming this bug about it, since its
nothing to do with Mozilla.
Comments on the patch in http://bugzilla.mozilla.org/
showattachment.cgi?attach_id=18444

1. Nit: using this->GetMac.... is redundant. Drop the this->
2. Are you sure that GetThemeBrushAsColor() and GetThemeTextColor() are
   available with all versions of the Appearance Manager that we run with?
3. Can we really not get the scrollbar trough color, or something that equates to 
   it?

Rest of the patch looks good. I'll r= when those comments have been addressed.
To answer on of Simon's points, these functions are Appearance 1.1 functions, 
which was introduced with Mac OS 8.5 which is the lowest Mac OS that Mozilla 
supports.

That said, these functions are aparently in Carbon 1.02 or better, which is 
supported back on 8.1, so when Mozilla moves to Carbon, the support (for these 
functions at least) actually extends further back (to 8.1) as well as forward (to 
Mac OS X)
As for the scrollbar trough colour - its #AAAAAA which is simply not on the list 
(see the comment from July 24th).

I mean, there is a general solution, which is to render a scrollbar into an 
offscreen buffer and sample pixels off of it, but that's a whole other kettle of 
fish.

Its a shame but I can't see a workaround at this point.
Simon can you super review or do I need Blizzard to look at this (he seems to be 
super reviewer for "widget").

Oh also, the "Module Owner" is still supposed to review right? Would that be 
Blizzard also?
Adding Blizzard to cc:

Blizzard can you do super review please, for Widget.

Assuming Pierre and Simon and happy with the final patch naturally.
Thanks for you comments guys.
How often is this code called and how expensive is it?  It is worth it to try
and cache it?  Please pardon my X-conservative self. :)
No, that's a very good question and one I looked at while testing my patch to bug 
46174 (which is essenitally the same thing but with extensions into other parts 
of the chrome).

Most of this seems to be called on a "once per window" basis... i.e. when opening 
a new browser window.

When testing bug 46174 I found one of the most intensive places where its called 
(bear in mind I haven't yet tried converting the *entire* classic skin) is in 
popup (contextual) menus (obv. on Mac OS menus on the main menubar are native) 
where its called at least twice each time you mouse over a menu item.

So I did a little test where I hardcoded a colour into skin then switched to 
this. Well, XUL menus are considerably slower than native ones, but from my 
informal test I failed to notice any significant difference between having this 
code (well this code's twin brother on 46174) enabled and just hardcoding the 
colour. Further, the popup menus feel just as slow on Modern2.

So I think its OK. In the event you wanted to cache this stuff, then it could go 
into an nsHashTable I guess. If one did cache this stuff you'd have to flush this 
cache as a part of the fix to bug 57799 which deals with the Mac OS system 
appearance theme changing.

My informal testing tells me that I think its OK without a cache. 
These calls are intended for drawing custom user controls, so I'd imagine Apple 
had efficiency in mind when they designed them.
sr=blizzard
Thanks Mr. B. Is this good to go on the trunk now, i.e. is blizzard also the 
module owner for this or do I have to track that person down. 

(is there some way of mapping files -> Module Owners - its by far the most opaque 
part of the checkin process right now - it seems like usually the super reviewer 
is almost always the module owner also, but the guidelines imply I need to check 
this)
pav is the module owner for the gtk code.
I asked Pavlov to look at this last night on IRC and he says he's happy with it.

Who wants to check it in to the trunk for me?
I have a fix in nsLookAndFeel.{cpp, h} that I'll checkin tomorrow once it gets 
superreviewed.  I'll check this patch in with it, so please don't check this in 
tonight.  Thanks!
Assuming lordpixel's 11/2 patch was the only thing that had to be checked in, 
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago24 years ago
Resolution: --- → FIXED
To verify, get 10/15 22:55 and 22:56 Testcase  files and open in Mozilla under
two different Mac OS appearance themes (www.macthemes.org).

Load the test case, observer the colours, switch themes, reload the test case -
observe (some) of the colours change to match the theme.
Keywords: patch, pp, reviewverifyme
Whiteboard: [review patch] → Verify Me
Netscape's standard compliance QA team reorganised itself once again, so taking 
remaining non-tables style bugs. Sorry about the spam. I tried to get this done 
directly at the database level, but apparently that is "not easy because of the 
shadow db", "plus it screws up the audit trail", so no can do...
QA Contact: chrisd → ian
mpt, could you do the verification honours here? Pleeeease...
QA Contact: ian → mpt
David Baron: In your opinion, are WindowText and Window intended to be the 
foreground and background colors for (a) text fields, or (b) labels? At the 
moment WindowText is (b)-ish, but Window is (a)-ish, which seems to be what's 
causing bug 66829.

Anyway. Full results will take a few more hours to collect, and I have to go to 
bed. But here's the preliminary results:

1.  Generally, we seem to do considerably better than Internet Explorer 5.0.
2.  `ThreeDLightShadow' and `ThreeDDarkShadow' seem to be wrong, but I haven't
    tested them thoroughly yet.
3.  `Menu' is slightly wrong -- sometimes it's a little too dark, sometimes
    it's completely the wrong color. Internet Explorer gets it right every
    time.
4.  There is a bug in the Sherbet Kaleidoscope scheme which makes HTML form
    controls appear in Mozilla as almost invisible pale pink text on a pale
    orange background. Great fun.
5.  We need to reconsider our use of Highlight and HighlightText for text
    selection colors, when most Web authors will be wanting to use it for menu
    selection colors instead.
6.  The origins of bug 66829 are a little clearer now.
7.  GrayText is thoroughly broken. It looks right in Platinum, but that's about
    it.
8.  The people who came up with this set of colors for the CSS specification
    should be thoroughly ashamed of themselves.
Ugh!  I have been toying around with some css trying to fix the "forms should
look better/look platform native" types of bugs and getting the CSS system
colors to work sensibly in all themes is horrid (there's a gkt bug w/ similar
problems).Some of is the theme authors and much of it is the choice of the CSS
system colors themselves, but I think there are a few things that mozilla can fix.

Anyway, bug 66829 is because -moz-Field is being set to white but -moz-fieldText
is set to the dialog box text color.  In dark themes, the dialog boxes often
have a dark background and light text.  However, in every theme I've tried, the
text entry fields are white with black text.  Maybe they should be hardcoded as
such.  Otherwise, I think it would be safe to set -moz-fieldText to black and
-moz-Field to kThemeBrushDocumentWindowBackground, since 1) document window
backgrounds seems to be quite consistently white, and 2) I don't think the
appearance manager provides a way to even get something as simple as the plain
old document text color, so it almost must be assumed to always be black.  (same
for window/windowtext if they're to mean document windows).

There are other places where mozilla is vulnerable to this.  For Menu it uses
the dialog box background color but for MenuText it uses the real color of menu
text.  It would be safer to use the dialog text color here (button colors might
be better though, since a popup menu rather looks like a button).
-moz-dialogtext is the dialog text color, but -moz-dialog is the button
background color (should use kThemeBrushDialogActiveBackground).  In cases where
either the desired color is not available from the appearance manager or its not
clear what the desired color even is, I think it would be best to be
conservative.  Better to get not quite perfect colors but at least ensure that
the text is readable when placed against an appropriate background.

Btw Matthew, are you using Kaleidoscope for much of your testing?  The bug isn't
just in the Sherbet scheme.  Any scheme in which the button color is
substantially different from the dialog background is vulnerable.  It appears
that Kaleidoscope sends the real button text color, but when asked about the
button background/highlights/shadows it seems to give colors based on the dialog
background.  Works most of the time, since the colors are ususually compatible,
but does weird things every so often.  Kaleidoscope also seems to mess up some
of the button highlights and shadows.  I should test this some more to be sure
it's Kaleidoscope and not Mozilla, though viewing the html page and css attaced
on 10/15/00 in both the genuine Apple Platinum appearance and the Kaleidoscope
platinum scheme suggests that something's up.
Bug 67448 may answer some of your questions about the colors.  If not, I may be 
able to when I have a little more time.
VERIFIED due to age and I've seen pages that appear to use this and it seems to
render fine.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: