Closed Bug 1188194 Opened 4 years ago Closed 4 years ago

Top border for lightweight theme in Windows 8 is too dark

Categories

(Firefox :: Theme, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 43
Tracking Status
firefox42 --- affected
firefox43 --- verified

People

(Reporter: bgrins, Assigned: Gijs)

References

Details

Attachments

(2 files)

Attached image 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.
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?
Flags: needinfo?(bgrinstead)
(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.
Flags: needinfo?(bgrinstead)
(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. :-)
Flags: needinfo?(philipp)
(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).
(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.
Flags: needinfo?(philipp)
Bug 1188194 - fix top border on win8, r?bgrins,jaws
Attachment #8652554 - Flags: review?(jaws)
Attachment #8652554 - Flags: review?(bgrinstead)
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
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.
Blocks: 1163184
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
Attachment #8652554 - Flags: review?(bgrinstead) → review+
(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 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
Attachment #8652554 - Attachment description: MozReview Request: Bug 1188194 - fix top border on win8, r?bgrins,jaws → MozReview Request: Bug 1188194 - fix top border on win8 for lightweight themes, r?jaws
No longer blocks: 1163184
Assignee: nobody → gijskruitbosch+bugs
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
Attachment #8652554 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/1bb5a5e0ff10
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Confirming the fix using Firefox 43.0RC build 1 (buildID 20151208100201) on Windows 8 x64.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.