[10.10] Update toolbar and panel icons for OS X Yosemite

VERIFIED FIXED in Firefox 34

Status

()

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 34
x86
Mac OS X
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Assignee)

Description

4 years ago
Mockup and assets should be in bug 1020551. Please also doublecheck that they've been compressed fully with ImageOptim or similar.

This will probably need to be an OS-version-specific override, because the difference with the Lion-like icons we ship right now is quite stark, so we can't just replace the existing ones with Yosemite ones everywhere.
(Assignee)

Updated

4 years ago
Points: --- → 3
Flags: firefox-backlog+
(Assignee)

Updated

4 years ago
QA Whiteboard: [qa+]
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 34.3
QA Whiteboard: [qa+]
Flags: qe-verify+
Juan, do you want to take Yosemite verifications ? If not let me know.
Flags: needinfo?(jbecerra)
QA Contact: jbecerra
I don't have a working Yosemite installation, but I will have something to test on next week.
Flags: needinfo?(jbecerra)
(Assignee)

Comment 3

4 years ago
Michael, this seems to be missing icons for Loop, as well as the small menuPanel icons (cut/copy/paste, zoom buttons - see menuPanel-small.png) and perhaps the footer icons for the panel? The new tab button, all tabs button (dropdown arrow) and tab navigation arrows are also still the same (possibly also less important).

I think we should at least do the menuPanel-small.png update in this bug, because otherwise the menu looks really wonky. Everything else could be followup if you prefer? Let me know what you think.
Flags: needinfo?(mmaslaney)
(Assignee)

Comment 4

4 years ago
Created attachment 8478629 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

Pretty simple approach, same as for the lion icons - Jared, does this look right to you? I'd want to do the same for menuPanel-small.png and anything else we'd still update this round.
Attachment #8478629 - Flags: feedback?(jaws)
Comment on attachment 8478629 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

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

Unable to test this, but this looks like the right approach.
Attachment #8478629 - Flags: feedback?(jaws) → feedback+
(Assignee)

Comment 6

4 years ago
I believe this also misses out the in-urlbar icons (stop/reload/go).
Created attachment 8480115 [details]
Yosemite-assets.zip

Updated assets.
Flags: needinfo?(mmaslaney)
(Assignee)

Comment 8

4 years ago
Created attachment 8480674 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

Now with a gazillion more icons.
Attachment #8480674 - Flags: review?(jaws)
(Assignee)

Updated

4 years ago
Attachment #8478629 - Attachment is obsolete: true
Comment on attachment 8480674 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

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

We really should get a bug on file to merge more of these files in to a single sprite. It is crazy how many various files there are to find and update (the @2x makes it seem worse).

f+ because the Sync icon in the menu panel is mismatched, http://screencast.com/t/XBuq2LcwpK
Attachment #8480674 - Flags: review?(jaws) → feedback+
(Assignee)

Comment 11

4 years ago
Michael, could you provide the sync icon as well? I sure hope this is the last one... :-(
Flags: needinfo?(mmaslaney)
Comment on attachment 8480674 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

>diff --git a/browser/themes/osx/yosemite/Toolbar.png b/browser/themes/osx/yosemite/Toolbar.png

This is inconsitent with where e.g. lion and aero icons live in the source tree. They use file name suffixes rather than separate folders. I think the former makes more sense, as it keeps the various versions of the same image together. I don't see much value in having all yosemite icons in one folder.
Created attachment 8480785 [details]
sync-yosemite.zip

Sync assets attached
Flags: needinfo?(mmaslaney)
(Assignee)

Comment 14

4 years ago
(In reply to Dão Gottwald [:dao] from comment #12)
> Comment on attachment 8480674 [details] [diff] [review]
> update toolbar/menupanel icons for yosemite,
> 
> >diff --git a/browser/themes/osx/yosemite/Toolbar.png b/browser/themes/osx/yosemite/Toolbar.png
> 
> This is inconsitent with where e.g. lion and aero icons live in the source
> tree. They use file name suffixes rather than separate folders. I think the
> former makes more sense, as it keeps the various versions of the same image
> together. I don't see much value in having all yosemite icons in one folder.

... whereas I thought the root of that folder was already cluttered enough, and duplicating a boatload of images and their @2x equivalents wouldn't make anyone happier.
(Assignee)

Comment 15

4 years ago
Created attachment 8480914 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

Now with sync and with '-yosemite' filenames instead of a directory (still using a directory inside the jar because that's also what lion and aero do).
Attachment #8480914 - Flags: review?(jaws)
(In reply to :Gijs Kruitbosch from comment #14)
> (In reply to Dão Gottwald [:dao] from comment #12)
> > Comment on attachment 8480674 [details] [diff] [review]
> > update toolbar/menupanel icons for yosemite,
> > 
> > >diff --git a/browser/themes/osx/yosemite/Toolbar.png b/browser/themes/osx/yosemite/Toolbar.png
> > 
> > This is inconsitent with where e.g. lion and aero icons live in the source
> > tree. They use file name suffixes rather than separate folders. I think the
> > former makes more sense, as it keeps the various versions of the same image
> > together. I don't see much value in having all yosemite icons in one folder.
> 
> ... whereas I thought the root of that folder was already cluttered enough,

I'm not sure this is a problem in terms of maintainability, but if so we should break up the folder into more sub folders such as "newtab", "tabbrowser" etc.

> and duplicating a boatload of images and their @2x equivalents wouldn't make
> anyone happier.

It makes it more obvious what different versions exist of each image, which might come handy when updating images. The duplication exists either way, hiding it doesn't seem helpful.
(Assignee)

Comment 17

4 years ago
Comment on attachment 8480914 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

Michael, Jared kindly pointed out to me the customize exit button is a close icon instead of the shutdown icon - see

data:text/html,<body style="background-color: blue;"><img src="http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/menuPanel-exit.png?raw=1">

for the current one. Can you provide updated versions of this icon? Thanks!
Attachment #8480914 - Flags: review?(jaws)
Flags: needinfo?(mmaslaney)
Created attachment 8481377 [details]
exit.zip

Exit button attached.
Flags: needinfo?(mmaslaney)
(Assignee)

Comment 19

4 years ago
Created attachment 8481416 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

Now with new shutdown goodness.
Attachment #8481416 - Flags: review?(jaws)
(Assignee)

Updated

4 years ago
Attachment #8480914 - Attachment is obsolete: true
Comment on attachment 8481416 [details] [diff] [review]
update toolbar/menupanel icons for yosemite,

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

Looks good. Thanks for the quick turn-around-time.
Attachment #8481416 - Flags: review?(jaws) → review+
(Assignee)

Comment 21

4 years ago
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #20)
> Comment on attachment 8481416 [details] [diff] [review]
> update toolbar/menupanel icons for yosemite,
> 
> Review of attachment 8481416 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. Thanks for the quick turn-around-time.

Likewise, thanks for the quick review, and sorry it took so many tries to get right. :-\

remote:   https://hg.mozilla.org/integration/fx-team/rev/c98e45bed5e0
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c98e45bed5e0
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 34
In the ZIP file, I also see that the help button misses an inverted icon. It might have been fixed in the patch though.
QA Contact: jbecerra → catalin.varga
Verified as fixed using the following environment:
FF 34
Build id: 20141009004002
OS: Mac Os X 10.10
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.