Closed Bug 240320 Opened 17 years ago Closed 15 years ago

selection highlight color changed in 0.5+ to no longer use system selection color

Categories

(Core Graveyard :: Widget: Mac, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: xolaware.llc, Assigned: brion)

References

Details

(Keywords: fixed1.8.1)

Attachments

(6 files, 3 obsolete files)

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?
Assignee: mscott → webmail
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.
Attachment #148042 - Attachment is obsolete: true
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?
Flags: blocking-aviary1.0mac?
Flags: blocking-aviary1.0mac?
Brion, can you please redo your patch against the trunk? Thanks!
Attached patch Updated patch to current trunk (obsolete) — Splinter Review
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) [edit]
> 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)
Blocks: 329144
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: 15 years ago
Resolution: --- → FIXED
Attached patch Pinstripe fixSplinter Review
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: review?(kevin) → review+
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.
Keywords: fixed1.8.1
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)
Attachment #226072 - Flags: review?(kevin) → review+
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?
Attachment #226072 - Flags: approval1.8.1? → 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.
Whiteboard: [checkin needed] of attachment #226072
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: 1.8.8.4; previous revision: 1.8.8.3
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)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.