1389860 - Tab height and text color adjustments

Status: RESOLVED FIXED

(Toolkit :: Themes, enhancement)

Description

[Stefan [:stefanh]]

Attachment: tab-control.png

The size of non-browser tabs (like the ones in the certificate details dialog) needs to be adjusted a bit (see screenshot). Also, nowadays these tabs shouldn't have any text-shadow. Neither should the text color change to white in inactive windows.

Comment 1

[Stefan [:stefanh]]

Attachment: With patch

Comment 2

[Stefan [:stefanh]]

Attachment: Tab text color adjustments for Yosemite and higher. Increase tab height (all OS versions)

I only have 10.9 and 10.12, but I assume that the text color / text-shadow change came with Yosemite. The control height should be adjusted for 10.9+.

Comment 3

[Stefan [:stefanh]]

Attachment: Tab text color adjustments for Yosemite and higher. Increase tab height (all OS versions)

Sorry, wrong commit msg. Se previous comment for details.

Comment 4

[:Gijs (he/him)]

Comment on attachment 8896676 [details] [diff] [review]
Tab text color adjustments for Yosemite and higher. Increase tab height (all OS versions)

Review of attachment 8896676 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, I'm struggling with this review because I don't have 10.9 handy and AFAICT the screenshots are also from more recent versions of OS X. Please see my further comments.

::: toolkit/themes/osx/global/tabbox.css
@@ +34,5 @@
>  }
>  
>  tab {
>    -moz-appearance: tab;
> +  padding-top: 1px;

I wonder if this is enough when the font size is changed?

@@ +60,5 @@
> +    text-shadow: rgba(0, 0, 0, 0.4) 0 1px;
> +  }
> +
> +  tab[visuallyselected="true"]:-moz-window-inactive {
> +    color: #FFF;

Sorry, I don't have 10.9 anymore - can you explain what's going on here? I don't really understand why this needs to be white for selected tabs on *inactive* windows - it feels like those have gone a lighter grey since ~forever, and white on light grey wouldn't work - was it different on 10.9 and have I just forgotten? Or was this supposed to be :not(:-moz-window-inactive)? But between the "all versions of OS X" rule and this one, the color is now always white, so we might as well put it in the generic tab[visuallyselected="true"] rule in the media query? :-)

Comment 5

[Stefan [:stefanh]]

Attachment: 10.9, tabs in active-inactive window

(In reply to :Gijs from comment #4)

> >  tab {
> >    -moz-appearance: tab;
> > +  padding-top: 1px;
> 
> I wonder if this is enough when the font size is changed?

Yeah, changing the font-size will change the height of the tab, but there are only 3 heights to choose from: https://dxr.mozilla.org/mozilla-central/rev/5322c03f4c8587fe526172d3f87160031faa6d75/widget/cocoa/nsNativeThemeCocoa.mm#2011. We're currently using the highest (the extra padding made it snap up one size). So any increase in font-size will not expand the height. Decreasing the font-size will make the height snap down 1 or 2 steps and at some point you'll probably want to adjust the padding, but I'm not sure how to make it look good for all font-sizes. Besides, I don't see any real-world use-cases here, I think we only use one size/type of tabs. 

> 
> @@ +60,5 @@
> > +    text-shadow: rgba(0, 0, 0, 0.4) 0 1px;
> > +  }
> > +
> > +  tab[visuallyselected="true"]:-moz-window-inactive {
> > +    color: #FFF;
> 
> Sorry, I don't have 10.9 anymore - can you explain what's going on here? I
> don't really understand why this needs to be white for selected tabs on
> *inactive* windows - it feels like those have gone a lighter grey since
> ~forever, and white on light grey wouldn't work - was it different on 10.9
> and have I just forgotten? Or was this supposed to be
> :not(:-moz-window-inactive)? But between the "all versions of OS X" rule and
> this one, the color is now always white, so we might as well put it in the
> generic tab[visuallyselected="true"] rule in the media query? :-)

See the screenshot (from 10.9 system preferences). On 10.9, we want to keep it as-is - the text color should be white in selected tabs, even for inactive windows :-)

I decided to do the media query for < 10.10, because I though it would be easier to remove once we drop support for 10.9. I could keep everything as-is and special-case it for Yosemite and higher if that's clearer.

Comment 6

[:Gijs (he/him)]

Comment on attachment 8896676 [details] [diff] [review]
Tab text color adjustments for Yosemite and higher. Increase tab height (all OS versions)

Review of attachment 8896676 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, r=me with the 10.9 thing in a single rule. :-)

::: toolkit/themes/osx/global/tabbox.css
@@ +60,5 @@
> +    text-shadow: rgba(0, 0, 0, 0.4) 0 1px;
> +  }
> +
> +  tab[visuallyselected="true"]:-moz-window-inactive {
> +    color: #FFF;

OK, thanks for the explanation, but then please just put the `color: #FFF;` bit in the `tab[visuallyselected="true"]` rule in the same media query, as on 10.9 we just want it to always be white, and that's a more logical progression from the previous styling (which effectively had exactly that rule, which we're now moving in a 10.9-only block).

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5facecab1233
tab text color adjustments for Yosemite and higher. Increase tab height (all OS versions). r=Gijs.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5facecab1233