Closed Bug 965322 Opened 6 years ago Closed 4 years ago

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

Categories

(Firefox :: Theme, defect)

All
Windows 8
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 50
Tracking Status
firefox50 --- verified

People

(Reporter: phlsa, Assigned: rakhisharma)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [qx][defect] p=0)

Attachments

(2 files)

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.
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.
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.
Duplicate of this bug: 1009431
According to Bug 1009431, both Windows 7 and Windows 8 themes have a Windows Vista styling (which is similar to the Windows 7 style).
Flags: firefox-backlog?
Hardware: x86 → All
Whiteboard: [defect] p=0 → [qx][defect] p=0
Version: 28 Branch → unspecified
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).
Duplicate of this bug: 1239424
Summary: Full screen windows on Windows 8 have Windows 7 window controls → Full screen windows on Windows 8/8.1/10 have Windows 7/Vista window controls
Shorlander, do we have a Windows 10 asset for this?
Flags: needinfo?(shorlander)
(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 ?
Flags: needinfo?(jaws)
(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.
Flags: needinfo?(shorlander)
Duplicate of this bug: 1243631
Duplicate of this bug: 1206498
Tim, can you work on this bug?
Flags: needinfo?(jaws) → needinfo?(ntim.bugs)
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.
(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.
(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.
Oh, yes, Windows Explorer does only show 1 button.
(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
(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.
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 ?
Flags: needinfo?(ntim.bugs)
I like that idea. Shorlander, you OK with that?
Flags: needinfo?(shorlander)
Tim, it seems there has been ~3 weeks since comment 19. Is there any update on this?
Flags: needinfo?(ntim.bugs)
(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.
Flags: needinfo?(ntim.bugs)
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED
Not going to work on this soon, but if anyone wants to pick this up, the pastebin should be helpful.
Assignee: ntim.bugs → nobody
Status: ASSIGNED → NEW
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 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.
Attachment #8772355 - Flags: review?(gijskruitbosch+bugs) → review+
Assignee: nobody → Rakhish1994
Status: NEW → ASSIGNED
(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.
Pushed by gijskruitbosch@gmail.com:
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.
Blocks: 1287870
(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.
https://hg.mozilla.org/mozilla-central/rev/d1eef136f252
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
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]
Status: RESOLVED → VERIFIED
Rakhi, you rock! Thank you for fixing this, it makes a major improvement for fullscreen on Windows :)
Depends on: 1318903
You need to log in before you can comment on or make changes to this bug.