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)

(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), text/html
Details
(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), image/jpeg
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), image/png
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), 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
Attached patch Header file to go with patch (deleted) — Splinter Review
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
Attached patch New simpler patch (deleted) — Splinter Review
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 (deleted) —
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: