874459 - Set a valid window icon for desktop on Mac

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Alexandre Poirot [:ochameau]]

Bug 865210 didn't fixed the icon on Mac.
We should also have a valid icon on b2g desktop app on this OS.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Set a valid window icon for desktop on Mac

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 752200 [details] [diff] [review]
Set a valid window icon for desktop on Mac

Myk, I tried to mimic what is being done for Firefox but haven't tested this patch. Could you give it a try?

Comment 3

[Myk Melez [:myk] [@mykmelez]]

The build succeeds after I correct the path to which the image file is copied:

    cp -RL $(srcdir)/b2g.icns $(DIST)/$(APP_NAME).app/Contents/Resources/$(MOZ_APP_NAME).icns

But Mac OS still doesn't display the icon in the dock, the Alt-Tab list, and the Finder.

Perhaps the only icon in the file is too small?  The Firefox ICNS files contain multiple sizes, including some large ones; and if I copy one of those to b2g.icns, then the icon does appear.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

I did some research, and I can't find good info on icon requirements in Apple's docs <http://developer.apple.com/library/mac/navigation/>, but Wikipedia has some useful info <http://en.wikipedia.org/wiki/Apple_Icon_Image_format>, as does this Stack Overflow question <http://stackoverflow.com/questions/6337787/how-can-i-set-the-icon-for-a-mac-application-in-xcode>.

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: b2g.icns

I used imagemagick to create this icns file, but it looks like it can't work:
  http://leonid.shevtsov.me/en/how-to-create-icns-icons-for-os-x
I attached a new version converted via this online service that is using libicns:
  http://iconverticons.com/online/

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 752200 [details] [diff] [review]
Set a valid window icon for desktop on Mac

The new image works!  So this seems great with the new image and the corrected path.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Set a valid window icon for desktop on Mac

Comment 8

[Alexandre Poirot [:ochameau]]

Comment on attachment 754424 [details] [diff] [review]
Set a valid window icon for desktop on Mac

New patch with MacOS-compatible icns file and corrected icon path.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 754424 [details] [diff] [review]
Set a valid window icon for desktop on Mac

I'm confident in Myk here.

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 754424 [details] [diff] [review]
Set a valid window icon for desktop on Mac

(Aside: for some reason this and the earlier patches fail to apply to my Git clone of the Git mirror of mozilla-central.  But they do apply to my Hg clone of inbound, and a diff generated from that clone then applies to the Git clone.)

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 754424 [details] [diff] [review]
Set a valid window icon for desktop on Mac

https://hg.mozilla.org/projects/birch/rev/ccff1558f51b

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ccff1558f51b

Comment 13

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 754424 [details] [diff] [review]
Set a valid window icon for desktop on Mac

[Approval Request Comment]
Bug caused by (feature/regressing bug #): No regression; this never worked.
User impact if declined: The impact is minor, but both B2G Desktop and the Firefox OS Simulator would continue to lack customized dock and app icons on Mac if this request were to be denied, which is unexpected and seems off to users.
Testing completed: In addition to local machine testing, this has been baking on mozilla-central for a week and a half.
Risk to taking this patch (and alternatives if risky): The risk is low (no changes on Gonk), and there are no alternatives.
String or UUID changes made by this patch: None.

Comment 14

[Alex Keybl [:akeybl]]

leo+, but only because this is NPOTB. We're past the point of approvals.

Comment 15

[Myk Melez [:myk] [@mykmelez]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/bcf12de92882


(In reply to Alex Keybl [:akeybl] from comment #14)
> leo+, but only because this is NPOTB. We're past the point of approvals.

Thanks, Alex, I'll treat desktop-specific changes as NPOTB from now on and request leo+ approval on that basis.

But note that <https://wiki.mozilla.org/Release_Management/B2G_Landing> still directs me to "nominate for… approval-mozilla-b2g18" for "all other landings for v1.x".

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/bcf12de92882