983814 - Australis - Windows 8 shouldn't use menuPanel-aero.png

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Tim Nguyen :ntim]

Windows 8 currently uses menuPanel-aero.png while it should use menuPanel.png (which is for Windows 8 +)

Comment 1

[Dão Gottwald [:dao]]

menuPanel.png is for Windows XP.

Comment 2

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [:dao] from comment #1)
> menuPanel.png is for Windows XP.

Yeah but stephen updated the menuPanel.png icons to have the Windows 8 style. Just compare the bug 969904 menuPanel.png and the Windows 8 live mockup.

Should this bug be about renaming the graphics ?

Comment 3

[Dão Gottwald [:dao]]

The name is fine as it is. menuPanel.png is used for Windows XP, menuPanel-aero.png is used for Windows Vista and newer.

Comment 4

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [:dao] from comment #3)
> The name is fine as it is. menuPanel.png is used for Windows XP,
> menuPanel-aero.png is used for Windows Vista and newer.

Sure but the actual problem here is that the menuPanel.png glyph has the Windows 8 style. While Windows 8 actually uses menuPanel-aero.png.

Maybe Stephen Horlander was confused in bug 969904 when changing the icon style since we now do the same thing with the Toolbar.png glyphs (Toolbar.png for Win 8, Toolbar-aero for Win aero, Toolbar-luna*.png for XP). 

Should I file a new bug about this, or should this bug be changed ?

Comment 5

[Dão Gottwald [:dao]]

Stephen, can you confirm that you updated menuPanel.png wrongly?

Comment 6

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [:dao] from comment #5)
> Stephen, can you confirm that you updated menuPanel.png wrongly?

Well, I wouldn't say wrongly ;)

I made menuPanel.png more neutral/generic so that it would work on XP and Windows 8. I perhaps did get confused about which image Windows 8 was actually using. For example we use Toolbar.png on Windows 8 not Toolbar-aero.png.

Should do the same for menuPanel.png.

Comment 7

[Dão Gottwald [:dao]]

(In reply to Stephen Horlander [:shorlander] from comment #6)
> (In reply to Dão Gottwald [:dao] from comment #5)
> > Stephen, can you confirm that you updated menuPanel.png wrongly?
> 
> Well, I wouldn't say wrongly ;)
> 
> I made menuPanel.png more neutral/generic so that it would work on XP and
> Windows 8. I perhaps did get confused about which image Windows 8 was
> actually using. For example we use Toolbar.png on Windows 8 not
> Toolbar-aero.png.
> 
> Should do the same for menuPanel.png.

Metro_Glyph-menuPanel.png seems to match menuPanel-aero.png, not menuPanel.png.

Toolbar.png is used *only* on Windows 8. We don't share it between XP and 8.

Comment 8

[Dão Gottwald [:dao]]

Generally, the way our Windows theme is set up, the aero folder and file suffix just excludes XP / includes Windows Vista and later.

Comment 9

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [:dao] from comment #8)
> Generally, the way our Windows theme is set up, the aero folder and file
> suffix just excludes XP / includes Windows Vista and later.

Makes sense, thanks! I will figure out a different way to do it.

Comment 10

[Marco Bonardo [:mak]]

(In reply to Dão Gottwald [:dao] from comment #8)
> Generally, the way our Windows theme is set up, the aero folder and file
> suffix just excludes XP / includes Windows Vista and later.

This should be clarified once and for all, cause just a couple weeks ago I was said the opposite was decided for Australis (aero is just vista/7, base theme is XP/8). Since you are the de-facto theme module owner, I'd like to understand how to code and review things from now on...

Comment 11

[Stephen Horlander [:shorlander]]

(In reply to Marco Bonardo [:mak] from comment #10)
> (In reply to Dão Gottwald [:dao] from comment #8)
> > Generally, the way our Windows theme is set up, the aero folder and file
> > suffix just excludes XP / includes Windows Vista and later.
> 
> This should be clarified once and for all, cause just a couple weeks ago I
> was said the opposite was decided for Australis (aero is just vista/7, base
> theme is XP/8). Since you are the de-facto theme module owner, I'd like to
> understand how to code and review things from now on...

On a conceptual level -aero made sense when we were talking about just XP and Vista/7. Windows 8 messes this up because while it is still technically using aero it is aesthetically very different.

While we are trying to unify graphics where it makes sense, there are still instances where we will want a graphic for XP, Vista/7 and 8. Maybe more than one depending on system theme variants, but generally those are the buckets.

It would be nice to have a more sensible system for this. Also something that is a little future proof in case Microsoft changes the system style again.

Comment 12

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [:dao] from comment #7)
> Metro_Glyph-menuPanel.png seems to match menuPanel-aero.png, not
> menuPanel.png.

That's bug 983254

Comment 13

[Marco Bonardo [:mak]]

(In reply to Stephen Horlander [:shorlander] from comment #11)
> On a conceptual level -aero made sense when we were talking about just XP
> and Vista/7. Windows 8 messes this up because while it is still technically
> using aero it is aesthetically very different.

I'm not arguing about either choices, they are well understandable. My problem is that someone tells me I should use non-aero assets on Win8, others tell me I should use aero assets on Win8.  Le me clarify I'm not blaming anyone, I'm just trying to figure what's the rule of thumb.

Comment 14

[Stephen Horlander [:shorlander]]

(In reply to Marco Bonardo [:mak] from comment #13)
> (In reply to Stephen Horlander [:shorlander] from comment #11)
> > On a conceptual level -aero made sense when we were talking about just XP
> > and Vista/7. Windows 8 messes this up because while it is still technically
> > using aero it is aesthetically very different.
> 
> I'm not arguing about either choices, they are well understandable. My
> problem is that someone tells me I should use non-aero assets on Win8,
> others tell me I should use aero assets on Win8.  Le me clarify I'm not
> blaming anyone, I'm just trying to figure what's the rule of thumb.

I would say the general rule of thumb should be to use aero assets on Win8 if there are no Win8 specific versions.

Comment 15

[Tim Nguyen :ntim]

Stealing per IRC :)

Comment 16

[Tim Nguyen :ntim]

Attachment: Patch v1

Comment 17

[Tim Nguyen :ntim]

Pushed to try : https://tbpl.mozilla.org/?tree=Try&rev=d43f91be8090

Comment 18

[:Gijs (he/him)]

Comment on attachment 8434920 [details] [diff] [review]
Patch v1

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

r=me

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/24f0094b0c29

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/24f0094b0c29

Comment 21

[Tim Nguyen :ntim]

Comment on attachment 8434920 [details] [diff] [review]
Patch v1

Note that this might be too late for beta, but still asking in case it isn't.
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 969904
User impact if declined: Wrong icons used for Windows 8
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): Very low
String or IDL/UUID changes made by this patch: None

Comment 22

[Sylvestre Ledru [:Sylvestre]]

It is indeed too late for beta.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1e772fb4b313

Comment 24

[Cornel Ionce [:noni] [Hubs QA]]

Confirming that Windows 8 & Windows 8.1 now use menuPanel.png items on latest Nightly (build ID: 20140609030202) and on latest Aurora (build ID: 20140609004001).