Hi Markus, Is there a way to get a consistent style regardless of whether -moz-mac-source-list is set on the parent ? That will be especially useful on Mojave where we'll want to match the native accent color.

At the moment? I'm not sure. But we can make it work any way you want. Do I understand correctly that you want to stop using -moz-mac-source-list? If so, in what cases and why? What styling does Mojave's dark mode apply to native source lists, do they get dark vibrancy? Should we add -moz-mac-source-list-dark?

Or do you just want to use the active selection color? Basically, you don't want to use any "native" vibrancy at all, just the blue color (iirc on 10.9 the -moz-mac-active-source-list-selection color is a gradient)?

(In that case I'd suggest using pure css instead)

(In reply to Markus Stange [:mstange] from comment #3) > At the moment? I'm not sure. But we can make it work any way you want. > > Do I understand correctly that you want to stop using -moz-mac-source-list? > If so, in what cases and why? What styling does Mojave's dark mode apply to > native source lists, do they get dark vibrancy? Should we add > -moz-mac-source-list-dark? In the built-in dark theme, we don’t use dark vibrancy because the colors usually end up too light. So instead, we prefer using a custom sidebar background color, but with still the same native highlight. (In reply to Stefan [:stefanh] from comment #5) > (In that case I'd suggest using pure css instead) That could work, but it would be nic e if it worked without.

Argh... looks like the summary didn't say the right thing.

(In reply to Markus Stange [:mstange] from comment #3) > At the moment? I'm not sure. But we can make it work any way you want. > > Do I understand correctly that you want to stop using -moz-mac-source-list? > If so, in what cases and why? What styling does Mojave's dark mode apply to > native source lists, do they get dark vibrancy? Should we add > -moz-mac-source-list-dark? Basically, I want to use the appearance from attachment 9011579 [details] with a dark (solid, not vibrant) background behind it. but instead I get attachment 9011578 [details]. Can we do this ?

Note that attachment 9011579 [details] doesn't change according to the Mojave accent color, but attachment 9011578 [details] actually does...

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #9) > Note that attachment 9011579 [details] doesn't change according to the > Mojave accent color, but attachment 9011578 [details] actually does... Urgh, meant the opposite, I'm really tired today :'(

Just some background since I implemented this for a couple of years ago: The reason you get the gradient is that it's the fallback style - you get the 10.9 "active" source list selection style. The reason I didn't made it possible to use *source-list-selection without a parent styled with -moz-mac-source-list was mainly this: https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/widget/cocoa/nsNativeThemeCocoa.mm#3491. Then, of course, I didn't thought it made sense since the native source list selection colors are very much tied to the background of the source list. How should the selection and the background of the source list look when the window is in the background (non-active)?

(In reply to Stefan [:stefanh] from comment #11) > How should the selection and the background of the source list look when the > window is in the background (non-active)? The theme API applies a custom translucent background in this case. (In reply to Stefan [:stefanh] from comment #11) > Just some background since I implemented this for a couple of years ago: > > The reason you get the gradient is that it's the fallback style - you get > the 10.9 "active" source list selection style. The reason I didn't made it > possible to use *source-list-selection without a parent styled with > -moz-mac-source-list was mainly this: > https://searchfox.org/mozilla-central/rev/ > 881a3c5664ede5e08ee986d76433bc5c4b5680e6/widget/cocoa/nsNativeThemeCocoa. > mm#3491. Then, of course, I didn't thought it made sense since the native > source list selection colors are very much tied to the background of the > source list. Makes sense for the non focused style, but the focused style is pretty much a solid color.

What do you think Stefan ?

I'll take a look. The idea here is to make any selection work without the parent styled with -moz-mac-source-list. I think the font smoothing problem is gone since the color is now set in the css. But I fear this might be non-trivial for XUL trees. Markus, don't we need a nsDisplayClearBackground item somewhere in nsTreeBodyFrame for this to work in a XUL tree?

Yes, you'd need that. Which is why I think this is maybe not the right way to go. How do you feel about hardcoding a solid fill color, and falling back to that on 10.10+ instead of falling back to the gradient? (Maybe hardcode multiple colors based on window activeness state and blue/graphite.)

(In reply to Markus Stange [:mstange] from comment #15) > How do you feel about hardcoding a solid fill color, and falling back to > that on 10.10+ instead of falling back to the gradient? (Maybe hardcode > multiple colors based on window activeness state and blue/graphite.) Yes, that would work for me. On Mojave, it would have to take in account of the accent color too.

Yeah, after trying to add a nsDisplayClearBackground item I agree it's not the right way to go. (In reply to Tim Nguyen :ntim (please use needinfo?) from comment #16) > On Mojave, it would have to take in account of > the accent color too. Looks like there are many accent colors. So, the bad thing here is that I don't have access to Mojave (my mbp is too old) which makes me wonder if I'm the right person to do this.

(In reply to Stefan [:stefanh] from comment #17) > Yeah, after trying to add a nsDisplayClearBackground item I agree it's not > the right way to go. > > (In reply to Tim Nguyen :ntim (please use needinfo?) from comment #16) > > > On Mojave, it would have to take in account of > > the accent color too. > > Looks like there are many accent colors. So, the bad thing here is that I > don't have access to Mojave (my mbp is too old) which makes me wonder if I'm > the right person to do this. I think if it matches the Blue/Graphite color, it should be fine on Mojave too.

OK, but there are 6 more accent colors on Mojave.

Attaching a WIP. Note that we don't use any vibrancy here, so we just have to pick one reasonable blue/graphite color (the vibrant colors change a bit depending on the colors behind the window). I had the window in front of a white backgrounf´d when picking the colors. Tim, I used rgb(63, 149, 246) / rgb(162, 162, 167) for Aqua/Graphite. What do you think?

Hi Stefan, Thanks for sharing your WIP. The solid fill approach sounds good to me, but is it possible to use a system color like "Highlight" or a variant as fill color ? "Highlight" automatically matches the accent color on Mojave, so using it should give accent color support for free.

(I'm referring to the CSS color "Highlight" in case it wasn't clear)

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #21) > Hi Stefan, > > Thanks for sharing your WIP. The solid fill approach sounds good to me, but > is it possible to use a system color like "Highlight" or a variant as fill > color ? > > "Highlight" automatically matches the accent color on Mojave, so using it > should give accent color support for free. Hi Tim, "HighLight" doesn't follow the accent color (control tint) on pre-Mojave. For example, on 10.12 "HighLight" follows the selection color you choose in system prefs instead of the control tint. Right now (I'm on 10.12) I have "Graphite" as the control tint and light yellow as selection color and "HighLight" is then a slightly darker variant of the yellow color I picked for selection color. Or have I misunderstood what you mean by accent color?

(In reply to Stefan [:stefanh] from comment #23) > (In reply to Tim Nguyen :ntim (please use needinfo?) from comment #21) > > Hi Stefan, > > > > Thanks for sharing your WIP. The solid fill approach sounds good to me, but > > is it possible to use a system color like "Highlight" or a variant as fill > > color ? > > > > "Highlight" automatically matches the accent color on Mojave, so using it > > should give accent color support for free. > > Hi Tim, > > "HighLight" doesn't follow the accent color (control tint) on pre-Mojave. > For example, on 10.12 "HighLight" follows the selection color you choose in > system prefs instead of the control tint. Right now (I'm on 10.12) I have > "Graphite" as the control tint and light yellow as selection color and > "HighLight" is then a slightly darker variant of the yellow color I picked > for selection color. Hmm right, the selection color and the accent color can be set separately. I mentioned Highlight, but it could be any of the cocoa system colors: https://developer.apple.com/documentation/appkit/nscolor/ui_element_colors?changes=latest_minor&language=objc Also, following the selection color wouldn't be too bad of a choice, as this is the color that is shown when selecting an item in the Finder content area.

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #24) > I mentioned Highlight, but it could be any of the cocoa system colors: > https://developer.apple.com/documentation/appkit/nscolor/ > ui_element_colors?changes=latest_minor&language=objc System colors should only be used for what they're designed for - especially the dynamic ones. That said, "HighLight" would probably be fine. But why not then just use "background-color: HighLight;"? > Also, following the selection color wouldn't be too bad of a choice, as this > is the color that is shown when selecting an item in the Finder content area. Right, we actually mimic this is in Firefox when, for example, selecting a bookmark in the Bookmarks Manager content area. The "HighLight" color is generally used in lists and the source list selections are used in the sidebars (together with "-moz-mac-source-list") in order to mimic a source list (https://developer.apple.com/design/human-interface-guidelines/macos/windows-and-views/sidebars/).

Stefan, I took your patch and added Mojave accent color support using the controlAccentColor system color. Works properly when I test on Mojave. What do you think ?

Nice. Does my colors for unfocused / inactive match Mojave?

(In reply to Stefan [:stefanh] from comment #28) > Nice. Does my colors for unfocused / inactive match Mojave? Yeah, there isn't much difference from the previous versions for unfocused/inactive. Do you want to submit it for review ?

Oh, one question since I don't have Mojave: + if (aIsActiveSelection) { + if (nsCocoaFeatures::OnMojaveOrLater()) { + SetCGContextFillColor(cgContext, NSColorToColor([NSColor controlAccentColor])); + } else if ([NSColor currentControlTint] == NSGraphiteControlTint) { On Mojave, source list selection colors always follow the chosen controlAccentColor? (In reply to Tim Nguyen :ntim from comment #29) > Do you want to submit it for review ? Sure, I'll split the patch in 2 so you also get credit. I'm slightly afk atm so it might take a few days, though.

(In reply to Stefan [:stefanh] from comment #30) > Oh, one question since I don't have Mojave: > > + if (aIsActiveSelection) { > + if (nsCocoaFeatures::OnMojaveOrLater()) { > + SetCGContextFillColor(cgContext, NSColorToColor([NSColor > controlAccentColor])); > + } else if ([NSColor currentControlTint] == NSGraphiteControlTint) { > > On Mojave, source list selection colors always follow the chosen > controlAccentColor? Yes, I've tested with a accent color different from the highlight color. > (In reply to Tim Nguyen :ntim from comment #29) > > Do you want to submit it for review ? > > Sure, I'll split the patch in 2 so you also get credit. I'm slightly afk atm > so it might take a few days, though. No worries, and no need to split the patch :)

I tested my WIP and found out that the background doesn't change when I switch from an active window with a non-focused selection to an inactive window. This might be a general problem with FrameIsInActiveWindow(nsIFrame* aFrame), though. It's easy to reproduce: 1) Open the bookmarks window and select a list item. The row is now blue (active selection). 2) Put focus in the search field. The row background changes to the right color (inactive selection in a active window) 3) Click outside the window so the window becomes inactive: Expected result: row background changes to inactive selection in an inactive window Actual result: row background doesn't change Not sure what's happening here... It looks like isInActiveWindow is true in 3). It works fine if you switch from active window and focused selection to an inactive window - then we get the right background color. Markus, any idea (maybe I missed something obvious here)?

Does Gecko invalidate the tree when the window focus change happens? You may need to adjust nsNativeThemeCocoa::WidgetAppearanceDependsOnWindowFocus so that it does.

nsNativeThemeCocoa::WidgetAppearanceDependsOnWindowFocus returns true by default.

Oh, right. And it's only called by nsDisplayThemeGeometry, which is not used for XUL trees. I think nsDisplayTreeBody::ComputeInvalidationRegion needs a similar "has window focus changed" check.

Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/c30f17a79566 Render solid colors for the source-list-selection -moz-appearance values on 10.10+ if they're not used in a vibrant context. r=spohl