Closed
Bug 983814
Opened 11 years ago
Closed 11 years ago
Australis - Windows 8 shouldn't use menuPanel-aero.png
Categories
(Firefox :: Theme, defect)
Tracking
()
VERIFIED
FIXED
Firefox 32
People
(Reporter: ntim, Assigned: ntim)
References
Details
(Whiteboard: [Australis:P-] p=2 s=33.1 [qa!])
Attachments
(1 file)
(deleted),
patch
|
Gijs
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
Windows 8 currently uses menuPanel-aero.png while it should use menuPanel.png (which is for Windows 8 +)
Assignee | ||
Updated•11 years ago
|
Blocks: theme-win8
Whiteboard: [Australis:P-]
Comment 1•11 years ago
|
||
menuPanel.png is for Windows XP.
No longer blocks: theme-win8
Status: UNCONFIRMED → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
Assignee | ||
Comment 2•11 years ago
|
||
(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 ?
Flags: needinfo?(dao)
Comment 3•11 years ago
|
||
The name is fine as it is. menuPanel.png is used for Windows XP, menuPanel-aero.png is used for Windows Vista and newer.
Flags: needinfo?(dao)
Assignee | ||
Comment 4•11 years ago
|
||
(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•11 years ago
|
||
Stephen, can you confirm that you updated menuPanel.png wrongly?
Flags: needinfo?(shorlander)
Comment 6•11 years ago
|
||
(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.
Flags: needinfo?(shorlander)
Comment 7•11 years ago
|
||
(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•11 years ago
|
||
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•11 years ago
|
||
(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.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INVALID → ---
Updated•11 years ago
|
Assignee: nobody → shorlander
Comment 10•11 years ago
|
||
(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•11 years ago
|
||
(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.
Assignee | ||
Comment 12•11 years ago
|
||
(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
Assignee | ||
Updated•11 years ago
|
Blocks: theme-win8
Comment 13•11 years ago
|
||
(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•11 years ago
|
||
(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.
Updated•11 years ago
|
Flags: firefox-backlog?
Updated•11 years ago
|
Flags: firefox-backlog? → firefox-backlog+
Assignee | ||
Comment 15•11 years ago
|
||
Stealing per IRC :)
Assignee: shorlander → ntim007
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 16•11 years ago
|
||
Attachment #8434920 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 17•11 years ago
|
||
Pushed to try : https://tbpl.mozilla.org/?tree=Try&rev=d43f91be8090
Comment 18•11 years ago
|
||
Comment on attachment 8434920 [details] [diff] [review]
Patch v1
Review of attachment 8434920 [details] [diff] [review]:
-----------------------------------------------------------------
r=me
Attachment #8434920 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 19•11 years ago
|
||
Keywords: checkin-needed
Whiteboard: [Australis:P-] → [Australis:P-][fixed-in-fx-team]
Comment 20•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P-][fixed-in-fx-team] → [Australis:P-]
Target Milestone: --- → Firefox 32
Updated•11 years ago
|
Whiteboard: [Australis:P-] → [Australis:P-] p=0 s=it-32c-31a-30b.3 [qa?]
Updated•11 years ago
|
Whiteboard: [Australis:P-] p=0 s=it-32c-31a-30b.3 [qa?] → [Australis:P-] p=2 s=it-32c-31a-30b.3 [qa+]
Assignee | ||
Comment 21•11 years ago
|
||
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
Attachment #8434920 -
Flags: approval-mozilla-beta?
Attachment #8434920 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Updated•11 years ago
|
Attachment #8434920 -
Flags: approval-mozilla-beta?
Attachment #8434920 -
Flags: approval-mozilla-beta-
Attachment #8434920 -
Flags: approval-mozilla-aurora?
Attachment #8434920 -
Flags: approval-mozilla-aurora+
Comment 22•11 years ago
|
||
It is indeed too late for beta.
Comment 23•11 years ago
|
||
QA Contact: florin.mezei
Updated•11 years ago
|
Whiteboard: [Australis:P-] p=2 s=it-32c-31a-30b.3 [qa+] → [Australis:P-] p=2 s=33.1 [qa+]
Updated•11 years ago
|
QA Contact: florin.mezei → cornel.ionce
Comment 24•11 years ago
|
||
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).
Status: RESOLVED → VERIFIED
Whiteboard: [Australis:P-] p=2 s=33.1 [qa+] → [Australis:P-] p=2 s=33.1 [qa!]
You need to log in
before you can comment on or make changes to this bug.
Description
•