Closed Bug 240320 Opened 20 years ago Closed 18 years ago
selection highlight color changed in 0
.5+ to no longer use system selection color
676 bytes, text/html
4.52 KB, patch
|Details | Diff | Splinter Review|
2.10 KB, patch
|Details | Diff | Splinter Review|
22.58 KB, image/jpeg
2.47 KB, patch
|Details | Diff | Splinter Review|
1.13 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7b) Gecko/20040411 Firefox/0.8.0+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7b) Gecko/20040411 Firefox/0.8.0+ it would appear that the new appearance of thunderbird 0.5+ for OS X has caused the selection to always be blue as opposed to using the user's preferred selection color as designated in the OS X Appearance System Preferences. this is only a minor annoyance in the folders pane, but in the messages pane, i have long had one of my labels that is a color that is close to the color blue chosen by thunderbird and have become accustomed to what messages of that color mean. now, i can no longer distinguish between a selected message that is an unlabelled message and a selected message that i have labelled with my blue label. Reproducible: Always Steps to Reproduce: 1. label a message with the label that is blue 2. leave a message next to it unlabelled 3. use the up & down arrow keys to go between the two messages Actual Results: both messages appear with the same or almost identical selection highlighting Expected Results: unlabelled messages should appear selected with the user's system preference for selection.
Confirmed using Thunderbird-Mac/2004-04-01. It WFM using Mozilla-Mac/1.7b/classic. Yours, Kevin?
Status: UNCONFIRMED → NEW
Ever confirmed: true
I hardcode the blue highlight color for selected items in menus, listboxes, trees because there is no way for me to get the real highlight color from the system. For example, if you go into the Appearance page of the System Prefs and set your Highlight Color to "Orange", you'll notice that in real Mac apps text selections are highlighted with a light orange (#FFD281), and tree and list items are highlighted with a darker orange (#FF8A22). I can use the lighter orange with a "color: Highlight" CSS rule, but I can't get the darker orange. This is in Firefox and Thunderbird only. It is a bug, but I'm not sure if there's anything I can do about it unless there is a rule to get the correct highlight color that I'm not aware of.
i don't care if it is a lighter orange or darker orange (orange is indeed my selection color of choice) but i definitely prefer it to hard-coded blue. my benchmark for comparing what items in the mail message list pane should be compared to is the finder. in the finder, individually selected file items are highlighted in a "darker" color for selection. perhaps closer would be Mail.app (which i clearly don't use. in that, the items in the mail message list pane, when selected, are also highlighted in the "darker" selection color. what did thunderbird do prior to these changes? the items i am talking about were highlighted correctly at that point. where did the color come from then?
This patch adds -moz-mac-alternateprimaryhighlight (the darker color) and -moz-mac-secondaryhighlight (a light gray for inactive list highlight) as CSS color keywords. Use of these colors in Finder and similar is described here: http://developer.apple.com/documentation/Carbon/Reference/databrow_reference/databrowser_ref/constant_23.html I haven't tested it very thoroughly, and in particular am not sure it does the right thing on earlier than 10.3.
Thanks a lot for coming up with this patch Brion! I've tested on 10.3.3 and it works great but I've had a real hard time packaging Thunderbird and moving it to my iBook running 10.2 to test there. Can anyone else test this patch on a pre-10.3 box?
I haven't managed to make a real enough build that I can copy it to another machine and run it there... However a quick test app that makes the same calls hints that this should return the right colors on 10.2. On 10.1 there's no kAlternatePrimaryHighlightThemeBrush so it'll then fall back to the lighter highlight color. This patch should correct the fallback on 10.1, I did it totally wrong the first time.
This would be a nice bit of polish for Thunderbird, Firefox and Seamonkey. Requesting 1.0mac blockage. Can someone help us test this patch on 10.2 or earlier?
Brion, can you please redo your patch against the trunk? Thanks!
Works for me, built and tested Thunderbird from trunk on 10.3.7.
Attachment #148188 - Attachment is obsolete: true
Brion, did you ever tested this when building against the 10.2.8 sdk?
(In reply to comment #10) > Brion, did you ever tested this when building against the 10.2.8 sdk? Can't say that I have, that I know of. Any special directions for building it this way? I've got a 10.2 partition I can boot to test, but I no longer have access to any 10.1 boxes.
(In reply to comment #11) > (In reply to comment #10) > > Brion, did you ever tested this when building against the 10.2.8 sdk? > Can't say that I have, that I know of. Any special directions for building it > this way? Sure, just add the line "ac_add_options --with-macos-sdk=/Developer/SDKs/MacOSX10.2.8.sdk" to your .mozconfig (assuming you have the sdk's installed in that location). The reason I ask is that the Appearance.h in the 10.2.8 sdk doesn't have any "kThemeBrushAlternatePrimaryHighlightColor", only "kThemeBrushPrimaryHighlightColor". And since official (ppc) builds use the 10.2.8 sdk (building on 10.3.9 with the 10.2.8 sdk for backward compability)... :/ I ran into this bug when trying to fix the selection colour in seamonkey classic theme, See bug 329144, comment #7.
From http://developer.apple.com/releasenotes/Carbon/HIToolboxOlderNotes.html notes for the 10.3.9 SDK: "kThemeBrushAlternatePrimaryHighlightColor is now documented. This brush is actually available on Mac OS X 10.2 and later. It represents the highlight color that is used in some list views." If we need to compile with the older SDK, we can hardcode the value I guess. :)
Josh, Mark - are you guys ok with a new native highlight color? This would make theme authors able to use the "real" color instead of hard-coding a blue color.
Moving to the right component. IMHO this would be nice to have on branch - theme athours could then use the "real" selection colors.
Assignee: kevin → joshmoz
Component: Mail Window Front End → Widget: Mac
Product: Thunderbird → Core
QA Contact: mac
Version: unspecified → Trunk
I don't have a problem with adding these brushes.
Brion, care to make a patch that also takes care of the 10.2.x stuff? Josh thought this was ok too.
You need to do something like this: #if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_3 #define kThemeBrushAlternatePrimaryHighlightColor -5 [...] #endif MAC_OS_X_VERSION_MAX_ALLOWED corresponds to the SDK in use. You don't use #ifdef kThemeBrushWhatever because they might be enums instead of macros.
Will post updated patch once I've tested it on 10.2 & x86 build...
Conditionally defines kThemeBrushAlternatePrimaryHighlightColor on older SDK, as recommended in comment 18. Tested with universal build (10.2 SDK for PPC, 10.4u SDK for Intel); resulting binary appears to run correctly on 10.4/ppc, 10.4/intel, and 10.2/ppc.
Attachment #173592 - Attachment is obsolete: true
(In reply to comment #21) > Created an attachment (id=218397)  > Updated patch to trunk, 10.2 compatible > > Conditionally defines kThemeBrushAlternatePrimaryHighlightColor on older SDK, > as recommended in comment 18. > > Tested with universal build (10.2 SDK for PPC, 10.4u SDK for Intel); resulting > binary appears to run correctly on 10.4/ppc, 10.4/intel, and 10.2/ppc. > Please ask for review (josh or mento) - otherwise it will take another two years ;). That said, I'm not sure you need the fallback color, since we don't support anything below 10.2 (also, difference between new/old highlight is that use of the new highlight color have to be combined with a change of text-color). But I'm absolutely not in the position to comment on this, so you might want to hear the reviewers comments first.
Comment on attachment 218397 [details] [diff] [review] Updated patch to trunk, 10.2 compatible This version does include an extra line for fallback on 10.1, which is no longer necessary since 10.1 support was dropped. I can remove it if desired, but it shouldn't hurt anything to be there, in case someone wants 10.1 support back. ;)
Attachment #218397 - Flags: review?(joshmoz)
Brion - I'm not ignoring this bug, it is just going to take me a little bit to get to it. Thanks for looking into it!
Comment on attachment 218397 [details] [diff] [review] Updated patch to trunk, 10.2 compatible looks fine to me
Attachment #218397 - Flags: review?(joshmoz) → review+
Comment on attachment 218397 [details] [diff] [review] Updated patch to trunk, 10.2 compatible We probably need sr from a layout peer for this. Trying roc.
Attachment #218397 - Flags: superreview?(roc)
Attachment #218397 - Flags: superreview?(roc) → superreview+
Comment on attachment 218397 [details] [diff] [review] Updated patch to trunk, 10.2 compatible Requesting approval-branch-1.8.1. Minor & safe fix that will make it possible to use native styled selection colors in the Mac themes(Pinstripe/Classic).
Attachment #218397 - Flags: approval-branch-1.8.1?(roc)
Comment on attachment 218397 [details] [diff] [review] Updated patch to trunk, 10.2 compatible This changes the nsILookAndFeel interface, but not in any way that could affect anyone, as far as I can tell.
Attachment #218397 - Flags: approval-branch-1.8.1?(roc) → approval-branch-1.8.1+
Josh, Can you check in Brion's patch on trunk/1.8.1 branch, please?
Assignee: joshmoz → brion
landed on branch and trunk, if anyone gets upset about the interface change don't blame the messenger :)
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Thanks for the check-ins, Josh :) I'll attach a patch for Pinstripe here that will make selection colors in listboxes/trees configurable via OS X's system preferences. Thunderbird's folder pane unaffected. Kevin, do you have the time to look at this?
Attachment #225433 - Flags: review?(kevin)
Attachment #225433 - Flags: approval-branch-1.8.1?(bugs.mano)
Comment on attachment 225433 [details] [diff] [review] Pinstripe fix a181=mano.
Attachment #225433 - Flags: approval-branch-1.8.1?(bugs.mano) → approval-branch-1.8.1+
Comment on attachment 225433 [details] [diff] [review] Pinstripe fix Mano, can you check in the patch (trunk/1.8.1), please?
Comment on attachment 225433 [details] [diff] [review] Pinstripe fix Checked in on trunk and MOZILLA_1_8_BRANCH.
For the record, this actually do change the look of selected items in Thunderbirds left folder pane. But I think the behaviour before the patch wasn't correct. Before the patch, upon selection, a darker blue/lighter gray area could be seen around the folder titles. That area is now gone.
Hmm, I forgot some things. The style rules for 'seltype="cell"' etc should also be changed. We don't seem to use it in our tree, but afaik those attributes (cell, text) are implemented so one could create a tree with selectable cells (and navigate between cells in different rows). I'm making them look the same as for the whole row for now. If someone else has any idea of what color to use... I also changed the colors on the "alternatingbackground" stuff at the end. Those are not in use either, but if someone wants to use them...
Attachment #226072 - Flags: review?(kevin)
Comment on attachment 226072 [details] [diff] [review] Additional pinstripe fix This is an additional fix to Pinstripe that should have been in the original patch.
Attachment #226072 - Flags: approval1.8.1?
The tree.css part of attachment # 225433 [details] [diff] [review] might not apply on branch - bug 296040 (cell-based selection in trees) wasn't fixed on the 1.8 branch. So, here's a 1.8 diff of tree.css.
Comment on attachment 226942 [details] [diff] [review] 1.8 branch diff of tree.css Checking in toolkit/themes/pinstripe/global/tree.css; /cvsroot/mozilla/toolkit/themes/pinstripe/global/tree.css,v <-- tree.css new revision: 126.96.36.199; previous revision: 188.8.131.52 done
Whiteboard: [checkin needed] of attachment #226072 → [checkin needed] of attachment #226072 (trunk-only)
Checked in attachment 226072 [details] [diff] [review] on the trunk. mozilla/toolkit/themes/pinstripe/global/tree.css 1.12
Whiteboard: [checkin needed] of attachment #226072 (trunk-only)
You need to log in before you can comment on or make changes to this bug.