1483887 - Using a dark theme in Lubuntu, the Minimize, Maximize and Close buttons in Full Screen are hard to see

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P3)

Description

[RagglenLove]

Attachment: 2018-08-16-191711_1600x900_scrot.jpg

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180704194950

Steps to reproduce:

OS: Lubuntu 16.04.5 LTS
Install this dark theme: https://github.com/B00merang-Project/Windows-10-Dark/ in /home/user/.themes
Open Firefox and press F11 for Full Screen.


Actual results:

The color of the Minimize, Maximize and Close buttons in Full Screen are dark and hard see it on Full Screen.


Expected results:

Since I have no access to Windows, I don't know what the intended behavior should be, but if I were to guess, the color of the Minimize, Maximize and Close buttons in Full Screen should be white.

Comment 1

[Dão Gottwald [:dao]]

We should replace Minimize.gif and friends with SVGs (possibly from /browser/themes/windows/window-controls) and use context-fill / context-stroke.

Comment 2

[Dão Gottwald [:dao]]

Attachment: patch

Comment 3

[Tim Nguyen :ntim]

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

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

Looks fine to me. Seems to look OK on Linux, but I'm not sure the Windows controls are what we want here. Maybe some Photon ones could be better.

::: browser/themes/linux/browser.css
@@ +153,5 @@
> +#minimize-button,
> +#restore-button,
> +#close-button {
> +  -moz-appearance: none;
> +  border: none;

I'm not seeing any default border in toolbarbutton.css, is this needed ?

@@ +154,5 @@
> +#restore-button,
> +#close-button {
> +  -moz-appearance: none;
> +  border: none;
> +  margin: 0 !important;

Is there a reason to use !important here ?

@@ +186,4 @@
>  }
> +
> +#TabsToolbar[brighttext] > #window-controls > #minimize-button:hover:active,
> +#TabsToolbar[brighttext] > #window-controls > #restore-button:hover:active {

The selectors here and above could probably just be

#TabsToolbar[brighttext] #*-button

Comment 4

[Dão Gottwald [:dao]]

(In reply to Tim Nguyen :ntim from comment #3)
> Comment on attachment 9001952 [details] [diff] [review]
> patch
> 
> Review of attachment 9001952 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks fine to me. Seems to look OK on Linux, but I'm not sure the Windows
> controls are what we want here. Maybe some Photon ones could be better.

It doesn't look like https://design.firefox.com/icons/viewer/ has all the necessary icons, plus the minimize icon doesn't seem to fit here.

> ::: browser/themes/linux/browser.css
> @@ +153,5 @@
> > +#minimize-button,
> > +#restore-button,
> > +#close-button {
> > +  -moz-appearance: none;
> > +  border: none;
> 
> I'm not seeing any default border in toolbarbutton.css, is this needed ?
> 
> @@ +154,5 @@
> > +#restore-button,
> > +#close-button {
> > +  -moz-appearance: none;
> > +  border: none;
> > +  margin: 0 !important;
> 
> Is there a reason to use !important here ?

I just copied this from the Windows stylesheet. Chances are we can simplify this in both places, so I'll file a followup on that.

> @@ +186,4 @@
> >  }
> > +
> > +#TabsToolbar[brighttext] > #window-controls > #minimize-button:hover:active,
> > +#TabsToolbar[brighttext] > #window-controls > #restore-button:hover:active {
> 
> The selectors here and above could probably just be
> 
> #TabsToolbar[brighttext] #*-button

The child selector is preferable as it's faster.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ea1e0401e03
Copy fullscreen window controls styling from Windows. r=ntim

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/3ea1e0401e03

Comment 7

[Anca Soncutean, Desktop QA]

I could see this issue was also reproducible on Ubuntu 16.04 x64, on Fx 62.0b17.
On the latest builds, Nightly 64.0a1 (2018-09-17) and Beta 63.0b7, under the same platform, the window controls are now visible in fullscreen mode.