Closed Bug 1050809 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: Theme, defect)

x86
macOS
defect
Not set
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 34
Iteration:
34.3

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 3 obsolete files)

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.
Points: --- → 3
Flags: firefox-backlog+
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)
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)
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+
I believe this also misses out the in-urlbar icons (stop/reload/go).
Attached file Yosemite-assets.zip
Updated assets.
Flags: needinfo?(mmaslaney)
Now with a gazillion more icons.
Attachment #8480674 - Flags: review?(jaws)
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+
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.
Attached file sync-yosemite.zip
Sync assets attached
Flags: needinfo?(mmaslaney)
(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.
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.
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)
Attached file exit.zip
Exit button attached.
Flags: needinfo?(mmaslaney)
Now with new shutdown goodness.
Attachment #8481416 - Flags: review?(jaws)
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+
(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
Closed: 5 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.
Depends on: 1060944
Filed bug 1060944 for the issue.
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.