1188194 - Top border for lightweight theme in Windows 8 is too dark

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Brian Grinstead [:bgrins]]

Attachment: LWT_top_border.PNG

See the top pixel in the screenshot - the border that connects with the window control buttons looks too dark in Windows 8.

We should either remove the border altogether or update the colors to match Win 8 better.  As I understand it, the border was added to push the LTW background image down 2px in Bug 591930.

Comment 1

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

The border is drawn because otherwise there is no border at all, which looks off with some LWT colors depending on the background against which a normal (restored) window is set (eg a black background with a fully black LWT, or white with a white LWT, ...).

What border color do you propose to "match Win 8"? The border is currently a combination of dark and light so that there is always a visible border. Do you think we should use the Windows 8 theme color? Something else?

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to :Gijs Kruitbosch from comment #1)
> What border color do you propose to "match Win 8"? The border is currently a
> combination of dark and light so that there is always a visible border. Do
> you think we should use the Windows 8 theme color? Something else?

Maybe the same color that is used for the left/right border for the top 1/4 of the window?  I'm not sure what color exactly that is - if that's matching the system colors or the LWT somehow.  Note: I think there may be another bug visible in that screenshot where the left/right border changes to black partway down the window.

Comment 3

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

(In reply to Brian Grinstead [:bgrins] from comment #2)
> (In reply to :Gijs Kruitbosch from comment #1)
> > What border color do you propose to "match Win 8"? The border is currently a
> > combination of dark and light so that there is always a visible border. Do
> > you think we should use the Windows 8 theme color? Something else?
> 
> Maybe the same color that is used for the left/right border for the top 1/4
> of the window?  I'm not sure what color exactly that is - if that's matching
> the system colors or the LWT somehow. 

That's the LWT plus a greyscale border (specifically, hsla(209,67%,12%,0.35) -- see http://mxr.mozilla.org/mozilla-central/search?string=toolbarShadowColor&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central ).

I don't know if using that border would work in terms of providing enough contrast with the surrounding windows, depending on the lwt in use. Philipp?

> Note: I think there may be another
> bug visible in that screenshot where the left/right border changes to black
> partway down the window.

That's the fault of the LWT in question - it provides a background image and a background color - clearly it provides black as the background color, and that then looks... stupid. :-)

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to :Gijs Kruitbosch from comment #3)
> That's the LWT plus a greyscale border (specifically, hsla(209,67%,12%,0.35)
> -- see
> http://mxr.mozilla.org/mozilla-central/
> search?string=toolbarShadowColor&find=&findi=&filter=^[^\0]*%24&hitlimit=&tre
> e=mozilla-central ).
> 
> I don't know if using that border would work in terms of providing enough
> contrast with the surrounding windows, depending on the lwt in use. Philipp?

I should also note that this is an issue with the light Dev Edition theme, which is where I noticed it first.  Don't have a screenshot handy right now, but hopefully any change we put in place would accommodate that UI as well.  (I'm assuming it will, since we are using the lwt system for it).

Comment 5

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

(In reply to :Gijs Kruitbosch from comment #3)
> (In reply to Brian Grinstead [:bgrins] from comment #2)
> > (In reply to :Gijs Kruitbosch from comment #1)
> > > What border color do you propose to "match Win 8"? The border is currently a
> > > combination of dark and light so that there is always a visible border. Do
> > > you think we should use the Windows 8 theme color? Something else?
> > 
> > Maybe the same color that is used for the left/right border for the top 1/4
> > of the window?  I'm not sure what color exactly that is - if that's matching
> > the system colors or the LWT somehow. 
> 
> That's the LWT plus a greyscale border (specifically, hsla(209,67%,12%,0.35)
> -- see
> http://mxr.mozilla.org/mozilla-central/
> search?string=toolbarShadowColor&find=&findi=&filter=^[^\0]*%24&hitlimit=&tre
> e=mozilla-central ).
> 
> I don't know if using that border would work in terms of providing enough
> contrast with the surrounding windows, depending on the lwt in use. Philipp?

I don't think contrast will be a problem here, especially since it only affects one edge of the window. We can use the same color as on the sides.

Comment 6

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

Attachment: MozReview Request: Bug 1188194 - fix top border on win8 for lightweight themes, r?jaws

Bug 1188194 - fix top border on win8, r?bgrins,jaws

Comment 7

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

Comment on attachment 8652554 [details]
MozReview Request: Bug 1188194 - fix top border on win8 for lightweight themes, r?jaws

Bug 1188194 - fix top border on win8, r?bgrins,jaws

Comment 8

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

This also addresses bug 1163184. Tim was right there, and I was wrong, and I feel bad not r+'ing that patch then, but here we are.

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8652554 [details]
MozReview Request: Bug 1188194 - fix top border on win8 for lightweight themes, r?jaws

https://reviewboard.mozilla.org/r/17183/#review15315

Fixes the issue in Dev Edition, thanks!  Would it make sense to split this into two separate commits?  It appears that the browser.css and devedition.css changes are tangential

Comment 10

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

(In reply to Brian Grinstead [:bgrins] from comment #9)
> Comment on attachment 8652554 [details]
> MozReview Request: Bug 1188194 - fix top border on win8, r?bgrins,jaws
> 
> https://reviewboard.mozilla.org/r/17183/#review15315
> 
> Fixes the issue in Dev Edition, thanks!  Would it make sense to split this
> into two separate commits?  It appears that the browser.css and
> devedition.css changes are tangential

Done - landed the devedition bits with bug 1163184's bug number, updating this one to have r?jaws for those bits.

Comment 11

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

Comment on attachment 8652554 [details]
MozReview Request: Bug 1188194 - fix top border on win8 for lightweight themes, r?jaws

Bug 1188194 - fix top border on win8 for lightweight themes, r?jaws

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8652554 [details]
MozReview Request: Bug 1188194 - fix top border on win8 for lightweight themes, r?jaws

https://reviewboard.mozilla.org/r/17183/#review15747

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/1bb5a5e0ff10

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1bb5a5e0ff10

Comment 15

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

Confirming the fix using Firefox 43.0RC build 1 (buildID 20151208100201) on Windows 8 x64.