1184656 - Use lighter separator between content and toolbars on Windows 10 and 8

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P4)

Description

[Tim Nguyen :ntim]

See https://people.mozilla.org/~shorlander/mockups/Windows-10/Windows-10.html .
If we want to care about High Contrast, let's use ThreeDLightShadow.

Comment 1

[Justin Dolske [:Dolske]]

I'm not seeing any significant color difference here either?

Comment 2

[Tim Nguyen :ntim]

(In reply to Justin Dolske [:Dolske] from comment #1)
> I'm not seeing any significant color difference here either?

Mockup : #A8A8A8
Currently : #A0A0A0

Comment 3

[Dão Gottwald [:dao]]

(In reply to Tim Nguyen [:ntim] from comment #0)
> See
> https://people.mozilla.org/~shorlander/mockups/Windows-10/Windows-10.html .
> If we want to care about High Contrast, let's use ThreeDLightShadow.

ThreeDLightShadow is lighter than what's used in the mockup. Stephen, are you ok with this?

Comment 4

[Dão Gottwald [:dao]]

ThreeDLightShadow is rgb(227,227,227) ... Dunno how to take a screenshot with my fancy laptop that doesn't have a Print key.

Comment 5

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [:dao] from comment #4)
> ThreeDLightShadow is rgb(227,227,227) ... Dunno how to take a screenshot
> with my fancy laptop that doesn't have a Print key.

Thanks I will see what that looks like. The Snippet tool can take screenshots, but not as convenient as a Print key :-\

Comment 6

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [:dao] from comment #4)
> ThreeDLightShadow is rgb(227,227,227) ... Dunno how to take a screenshot
> with my fancy laptop that doesn't have a Print key.

I recommend Shotty : http://shotty.devs-on.net/en/Overview.aspx
It adds a small taskbar tray item with convenient screenshot options.

Comment 7

[Stephen Horlander [:shorlander]]

So ThreeDLightShadow definitely won't work because it's too light: http://cl.ly/image/3S2H291k2C17

ThreeDShadow is #a0a0a0 / rgb(160,160,160) which is almost what we have now. So we could use that but it might not work if we decide to change the contrast later.

What we have now seems to work in three of the four High Contrast mode presets: http://cl.ly/image/0R0A221H302k

I would prefer not to rely on the color keywords in the default case, but would be ok using them for just High Contrast themes.

Comment 8

[Dão Gottwald [:dao]]

Your screenshots seem to be cut off in the middle of the navigation toolbar. Did you misunderstand what this bug is about? It's about the border /below/ toolbars, above the content area.

(In reply to Stephen Horlander [:shorlander] from comment #7)
> ThreeDShadow is #a0a0a0 / rgb(160,160,160) which is almost what we have now.
> So we could use that but it might not work if we decide to change the
> contrast later.

It's precisely what we have now, is it not? http://hg.mozilla.org/mozilla-central/annotate/24f4d8e5e24b/browser/themes/windows/browser.css#l99
If this color is fine, can we close this as INVALID / WONTFIX?

Comment 9

[Dão Gottwald [:dao]]

Note that hardcoding the color for the default theme would be easy, but we should only do it if that yields benefits.

Comment 10

[Tim Nguyen :ntim]

(In reply to Stephen Horlander [:shorlander] from comment #7)
> So ThreeDLightShadow definitely won't work because it's too light:
> http://cl.ly/image/3S2H291k2C17
> 
> ThreeDShadow is #a0a0a0 / rgb(160,160,160) which is almost what we have now.
> So we could use that but it might not work if we decide to change the
> contrast later.
> 
> What we have now seems to work in three of the four High Contrast mode
> presets: http://cl.ly/image/0R0A221H302k
> 
> I would prefer not to rely on the color keywords in the default case, but
> would be ok using them for just High Contrast themes.

I'm not seeing the separator between the content and the toolbars in each of these mockup. The separator I'm talking about is the bottom border separating the navbar from the content.

Comment 11

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment #10)
> separating the navbar from the content.
meant web content if that isn't obvious.

Comment 12

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [:dao] from comment #8)
> Your screenshots seem to be cut off in the middle of the navigation toolbar.
> Did you misunderstand what this bug is about? It's about the border /below/
> toolbars, above the content area.
> 
> (In reply to Stephen Horlander [:shorlander] from comment #7)
> > ThreeDShadow is #a0a0a0 / rgb(160,160,160) which is almost what we have now.
> > So we could use that but it might not work if we decide to change the
> > contrast later.
> 
> It's precisely what we have now, is it not?
> http://hg.mozilla.org/mozilla-central/annotate/24f4d8e5e24b/browser/themes/
> windows/browser.css#l99
> If this color is fine, can we close this as INVALID / WONTFIX?

Well, I totally misread this bug. I thought it was talking about tab separators for some reason.

Comment 13

[Tim Nguyen :ntim]

What's your take on comment 8 ?

Comment 14

[Stephen Horlander [:shorlander]]

(In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment #13)
> What's your take on comment 8 ?

Tried using ThreeDLightShadow for a while. The problem is that ThreeDShadow is too dark and ThreeDLightShadow is too light. Need a ThreeDMediumShadow…

Comment 15

[Dão Gottwald [:dao]]

(In reply to Stephen Horlander [:shorlander] from comment #14)
> (In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment
> #13)
> > What's your take on comment 8 ?
> 
> Tried using ThreeDLightShadow for a while. The problem is that ThreeDShadow
> is too dark and ThreeDLightShadow is too light. Need a ThreeDMediumShadow…

Ok, let's hardcode it then. Should I use #A8A8A8 (as used in the mockup according to comment 2)?

Comment 16

[Dão Gottwald [:dao]]

And only for Windows 10 or older versions as well?

Comment 17

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [:dao] from comment #15)
> (In reply to Stephen Horlander [:shorlander] from comment #14)
> > (In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment
> > #13)
> > > What's your take on comment 8 ?
> > 
> > Tried using ThreeDLightShadow for a while. The problem is that ThreeDShadow
> > is too dark and ThreeDLightShadow is too light. Need a ThreeDMediumShadow…
> 
> Ok, let's hardcode it then. Should I use #A8A8A8 (as used in the mockup
> according to comment 2)?

Can you please use #c2c2c2 / hsl(0,0%,76%) It sits nicely in the middle.

(In reply to Dão Gottwald [:dao] from comment #16)
> And only for Windows 10 or older versions as well?

XP looks fine with ThreeDShadow and Windows 7 is already hardcoded to something else. This change would work in Windows 8 though.

Comment 18

[Dão Gottwald [:dao]]

Attachment: patch

Comment 19

[:Gijs (out for now; he/him)]

Comment on attachment 8647899 [details] [diff] [review]
patch

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

Nice!

FWIW, this means we continue to use an opaque (ThreeDShadow) color for the lwt case. I wonder if we should update that in a separate bug to be partially transparent so it blends in with the lwtheme more?

Comment 20

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #19)
> FWIW, this means we continue to use an opaque (ThreeDShadow) color for the
> lwt case. I wonder if we should update that in a separate bug to be
> partially transparent so it blends in with the lwtheme more?

Sounds like a good idea.

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/208e060b123a

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/208e060b123a

Comment 23

[Dão Gottwald [:dao]]

Comment on attachment 8647899 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: Windows 10
[User impact if declined]: aesthetics
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: low (trivial CSS change)
[String/UUID change made/needed]:

Comment 24

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8647899 [details] [diff] [review]
patch

Polish for Windows 10, taking it.

Comment 25

[Cornel Ionce [:noni] [Hubs QA]]

Confirming this fix for latest NIghtly 43.0a1, build ID: 20150816091433.
The font used now is #C2C2C2.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/66976c6ac88a

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0364790f94b1

Comment 28

[Cornel Ionce [:noni] [Hubs QA]]

I've also verified this using latest 42.0a2 Aurora (build ID: 20150818004007) and Firefox 41.0b2 (build ID: 20150817163452).