Closed Bug 1483887 Opened Last year Closed Last year

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

Categories

(Firefox :: Theme, defect, P3)

61 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox63 --- verified

People

(Reporter: argonvegell, Assigned: dao)

References

(Depends on 1 open bug)

Details

Attachments

(2 files)

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.
Status: UNCONFIRMED → NEW
Component: Untriaged → Theme
Ever confirmed: true
We should replace Minimize.gif and friends with SVGs (possibly from /browser/themes/windows/window-controls) and use context-fill / context-stroke.
Priority: -- → P3
Attached patch patchSplinter Review
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Attachment #9001952 - Flags: review?(ntim.bugs)
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
Attachment #9001952 - Flags: review?(ntim.bugs) → review+
(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.
Keywords: checkin-needed
Blocks: 1484422
Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ea1e0401e03
Copy fullscreen window controls styling from Windows. r=ntim
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3ea1e0401e03
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Depends on: 1485256
Depends on: 1485291
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.