965322 - Full screen windows on Windows 8/8.1/10 have Windows 7/Vista window controls

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

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

Attachment: Wrong minimize/maximize/close buttons

When putting a window into full screen mode on Windows 8, the window controls (minimize/maximize/close) have a Windows 7 styling.
XP has its own button style and so should Win 8.

Comment 1

[u428464]

I really think the current styling should also be improved on Win 7. I don't see why the controls have to be so small.

Comment 2

[ekerazha]

I can confirm this issue in Firefox 36.

When you're in full screen mode on Windows 8, the window controls use a Windows 7 theme.

Comment 4

[ekerazha]

According to Bug 1009431, both Windows 7 and Windows 8 themes have a Windows Vista styling (which is similar to the Windows 7 style).

Comment 5

[ekerazha]

Firefox 40.0 with the Windows 10 theme also has this issue. In full screen mode, the window controls (minimize/maximize/close) have a Windows Vista styling (on Windows 10 too).

Comment 7

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

Shorlander, do we have a Windows 10 asset for this?

Comment 8

[Tim Nguyen :ntim]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> Shorlander, do we have a Windows 10 asset for this?

Can't we use caption-buttons.svg for this ?

Comment 9

[Stephen Horlander [:shorlander]]

(In reply to Tim Nguyen [:ntim] from comment #8)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> > Shorlander, do we have a Windows 10 asset for this?
> 
> Can't we use caption-buttons.svg for this ?

Yeah, I think we should use those. Doesn't make a lot of sense to replace them.

Comment 12

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

Tim, can you work on this bug?

Comment 13

[Xidorn Quan [:xidorn] UTC+11]

FWIW, it seems the explorer on Windows 10 just has a restore button on the right top corner, not the complete controls. Probably we should just do that as well.

Comment 14

[Nathan V]

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #13)
> FWIW, it seems the explorer on Windows 10 just has a restore button on the
> right top corner, not the complete controls. Probably we should just do that
> as well.

While I'm all for one button instead of 3 where possible, I don't think this fits the case.
I'd much prefer the standard Windows 10 window controls to be there whether I'm in fullscreen or out. Plus they're transparent, which makes themes all the better.

Comment 15

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

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #13)
> FWIW, it seems the explorer on Windows 10 just has a restore button on the
> right top corner, not the complete controls. Probably we should just do that
> as well.

I don't see this on Edge or Internet Explorer 11. I don't see a way to get Edge to go fullscreen, and IE11 shows all three buttons for me.

Comment 16

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

Oh, yes, Windows Explorer does only show 1 button.

Comment 17

[Stephen Horlander [:shorlander]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> Oh, yes, Windows Explorer does only show 1 button.

I see three buttons: http://cl.ly/0P2H0O302743

Comment 18

[Stephen Horlander [:shorlander]]

(In reply to Stephen Horlander [:shorlander] from comment #17)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> > Oh, yes, Windows Explorer does only show 1 button.
> 
> I see three buttons: http://cl.ly/0P2H0O302743

Oh, Windows Explorer, yes. I would still go with the standard controls everywhere.

Comment 19

[Tim Nguyen :ntim]

I can work on this next week or in 2 weeks when my exams are over.
Do we want to replace the fullscreen assets for older platforms as well ? the older assets never felt great for both touch and mouse, since they were tiny. Perhaps we can use the lightweight theme Windows 10 caption buttons for Windows 7 ?

Comment 20

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

I like that idea. Shorlander, you OK with that?

Comment 21

[Stephen Horlander [:shorlander]]

Works for me.

Comment 22

[Xidorn Quan [:xidorn] UTC+11]

Tim, it seems there has been ~3 weeks since comment 19. Is there any update on this?

Comment 23

[Tim Nguyen :ntim]

(In reply to Xidorn Quan [:xidorn] (UTC+8) from comment #22)
> Tim, it seems there has been ~3 weeks since comment 19. Is there any update
> on this?

I've been really busy the last few days. Feel free to take this bug since I probably won't get to it soon.

Comment 24

[Tim Nguyen :ntim]

note to self: https://pastebin.mozilla.org/8861614

Comment 25

[Tim Nguyen :ntim]

Not going to work on this soon, but if anyone wants to pick this up, the pastebin should be helpful.

Comment 26

[Rakhi(:rakhisharma)]

Attachment: Bug 965322 - Full screen windows on Windows 8/8.1/10 have Windows 7/Vista window controls. .

Review commit: https://reviewboard.mozilla.org/r/65186/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65186/

Comment 27

[Rakhi(:rakhisharma)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ddcbba9dbb90

Comment 28

[Tim Nguyen :ntim]

I think we've agreed in comment 19 to use the Windows 10 lightweight theme caption buttons for Windows XP-Vista-7 and remove the old PNGs.

Comment 29

[:Gijs (he/him)]

Comment on attachment 8772355 [details]
Bug 965322 - Full screen windows on Windows 8/8.1/10 have Windows 7/Vista window controls. .

https://reviewboard.mozilla.org/r/65186/#review62208

We should update hover states for the brighttext case, but we should probably do that in bug 1189201. Otherwise, this looks fine to me once you address the nits below - tested on Windows 10 and Windows 8.

::: browser/themes/windows/browser.css:1032
(Diff revision 1)
> + #minimize-button,
> + #restore-button,
> + #close-button {
> +  -moz-appearance: none;
> +  border: none;
> +  margin: 0 !important;
> +  padding: 6px 12px;
> + }
> +
> +#minimize-button {
> +  list-style-image: url(chrome://browser/skin/caption-buttons.svg#minimize);
> + }
> +
> +#restore-button {
> +  list-style-image: url(chrome://browser/skin/caption-buttons.svg#restore);
> + }

Nit: the selectors here should not have spaces before them, nor should the closing braces (`}`)

::: browser/themes/windows/browser.css:1061
(Diff revision 1)
> +  background-color: hsla(0, 0%, 0%, .22);
> +}
> +
> +#close-button {
> +  list-style-image: url(chrome://browser/skin/caption-buttons.svg#close);
> + }

Nit: same.

::: browser/themes/windows/browser.css:1070
(Diff revision 1)
> +  list-style-image: url(chrome://browser/skin/caption-buttons.svg#close-white);
> +}
> +
> +#close-button:hover:active {
> +  background-color: hsl(355, 82%, 69%);
> +  list-style-image: url(chrome://browser/skin/caption-buttons.svg#close-white);

You already set the image for `:hover` so it doesn't need re-setting for `:hover:active`, so please get rid of this line.

Comment 30

[:Gijs (he/him)]

(In reply to Tim Nguyen :ntim (use needinfo?) from comment #28)
> I think we've agreed in comment 19 to use the Windows 10 lightweight theme
> caption buttons for Windows XP-Vista-7 and remove the old PNGs.

But nobody updated the summary? Sigh. At this stage, we should do this in a new bug. The patch attached accomplishes exactly what the summary says and the original bug got filed for, and we shouldn't scope-creep it.

Updating things for xp/vista/7 will in and of itself require a bunch more testing that isn't easy to do for Rakhi because there's no access to those platforms, and there's equally no plans for the highlight styles (as the icons don't have any so we need CSS background highlighting for hover/active) of the buttons that would work on XP/vista/win7 on all the different OS themes, which will compound the problems in bug 1184934 to also start happening for fullscreen on xp/vista/win7 at all times (even when not using a lwtheme). Blocking this bug on figuring all that out at this stage doesn't seem like a good idea.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/d1eef136f252
Full screen windows on Windows 8/8.1/10 should not have Windows 7/Vista window controls. r=Gijs.

Comment 32

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #30)
> and there's equally no plans for the highlight styles (as the
> icons don't have any so we need CSS background highlighting for
> hover/active) of the buttons that would work on XP/vista/win7 on all the
> different OS themes, which will compound the problems in bug 1184934 to also
> start happening for fullscreen on xp/vista/win7 at all times (even when not
> using a lwtheme).

Copy-paste fail, I meant bug 1189201.

Comment 33

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d1eef136f252

Comment 34

[Azmina]

I have reproduced this bug with Nightly 29.0a1 (2014-01-29) on Windows 8.1, 64 bit!

The bug's fix is verified on Latest Nightly 50.0a1 .

Build ID : 20160722030235
User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0

[bugday-20160720]

Comment 35

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

Rakhi, you rock! Thank you for fixing this, it makes a major improvement for fullscreen on Windows :)