484064 - tango application icons

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Bryan Clark (DevTools PM) [:clarkbw]]

Right now gtk uses a really old set of application icons that I think might even come from the netscape days.  I just figured out what an xpm file was by looking at it through less!

We need to figure out what the requirements are for the app icons and then replace these with new ones that aren't so retro.

http://mxr.mozilla.org/comm-central/source/mail/app/icons/gtk/

Comment 1

[Andreas Nilsson (:andreasn)]

We should be able to use png's for these. 
Related to this, there anything stopping us from installing the app icon into /usr/share/icons/hicolor/$size/apps ?
Installing several sizes would allow for a sharper icon in the smaller sizes.

Comment 2

[Andreas Nilsson (:andreasn)]

The typical behavior for a application on Linux is to have all windows use the application icon (Firefox3 does this for the Library and Downloads windows), so I think we should do this for Thunderbird as well.

Comment 3

[Magnus Melin [:mkmelin]]

(In reply to comment #1)
> Related to this, there anything stopping us from installing the app icon into
> /usr/share/icons/hicolor/$size/apps ?

We don't have an installer on linux... so that part is up to the distros.

Comment 4

[Ludovic Hirlimann [:Usul]]

Adding some interested parties

Comment 5

[Bryan Clark (DevTools PM) [:clarkbw]]

we could provide the right set of icons in this directory for distros to install in the correct $prefix/icons location.

Any particular things (directory/file format, etc) we could do to help the distros?

Comment 6

[Bryan Clark (DevTools PM) [:clarkbw]]

We can probably just drop the different sized icons directly into this directory for now.  In order to be installed in the hicolor/$size/apps directory we'll need some makefile.in magic.  Firefox doesn't install it's icons into the desktop icon spec space either so this work could be valuable to them as well.

adding michael in case he knows anything about this.

thunderbird app mailfile.in
http://mxr.mozilla.org/comm-central/source/mail/app/Makefile.in

firefox app makefile.in
http://mxr.mozilla.org/firefox/source/browser/app/Makefile.in

Comment 7

[Andreas Nilsson (:andreasn)]

It seems like deleting msgcomposeWindow.xpm, abcardWindow.xpm etc. makes the window icon fall back to mozicon50.xpm, so that's all good.
I'll get working on the branded and unbranded variants sizes of the icon.

Comment 8

[Mark Banner (:standard8)]

(In reply to comment #7)
> It seems like deleting msgcomposeWindow.xpm, abcardWindow.xpm etc. makes the
> window icon fall back to mozicon50.xpm, so that's all good.
> I'll get working on the branded and unbranded variants sizes of the icon.

Actually I think it will fall back to default.xpm:

http://mxr.mozilla.org/comm-central/source/mozilla/widget/src/gtk2/nsWindow.cpp#4697

Note that gtk2 supports different sizes and png as well as xpm:

http://mxr.mozilla.org/comm-central/source/mozilla/widget/src/gtk2/nsWindow.cpp#1823

If you remove files, be sure to add them to removed-files.in so that they get removed from builds where people do updates from one to the next.

Comment 9

[Andreas Nilsson (:andreasn)]

Attachment: new icons

Comment 10

[Andreas Nilsson (:andreasn)]

Attachment: comparision between old and new icons

BTW, Also added 256x256 icon as it will work better in GNOME Do and other things using bigger-than-normal icons.

Comment 11

[Bryan Clark (DevTools PM) [:clarkbw]]

caillon: can I assign this to you?  I believe it's just Makefile.in foo work left of which you are a ninja.

Andreas put together the new icons.  We can use a single application icon for every window (addressbook, compose, 3pane) as per the GNOME HIG.  So the other older icons can be removed and these new icons can probably be installed in the xdg icon location.

We can go after the windows icons in a couple weeks when we start working with the Vista theme changes.

Comment 12

[u294409]

Are new icons (attachment 371441 [details]) really tango style, according to bug's title?

Comment 13

[Andreas Nilsson (:andreasn)]

Attachment: screenshot of icon in action

Jakub: I would say it blends in rather well, even though it was a tricky process of getting it to not look alien in the menu and at the same time, not altering the logo beyond what would be reasonable for it to stay the TB logo.
See attached screenshot to judge if it does the job.

Comment 14

[u294409]

I'd say Mozilla apps still look kinda alien but things get better.

IMO colors are still too strong, too dark, compared to others. Elements like globe are not even tangoish/gnomish (compare Firefox' globe and Epiphany's/GNOME's one).

But I understand you can't really replace logos, so I'll stay silent with my own icons ;) .

Even if you didn't really change pallette to Tango and it looks still different, it is better and I'm satisfied with changes you could make and you made.

Comment 15

[Andreas Nilsson (:andreasn)]

Note that I didn't fix up the Firefox icon, I think we need to open a separate bug about that.

Comment 16

[Wolfgang Rosenauer [:wolfiR]]

I'm a bit confused about the status of this issue.
What is the missing bit currently?
About the Makefile foo to install stuff to the "right" places I think that's pretty much useless. Thunderbird on Linux has no installer at all so it makes sense just to ship the icons within dist/bin as it is now (in dist/bin/icons).
Distributors will copy or link to them as they want afterwards (and how it fits their desktop integration).
So I think it's "only" needed to commit the new icons to the right place and install them to dist/bin/icons.
I could help with that if needed.

Comment 17

[Mark Banner (:standard8)]

(In reply to comment #1)
> We should be able to use png's for these. 
> Related to this, there anything stopping us from installing the app icon into
> /usr/share/icons/hicolor/$size/apps ?

Yes, we can only do that if someone actually builds Thunderbird on Linux, via the "install" make file target.

As Wolfgang said in comment 16, we can only ship icons in dist/bin - there is no installer available on Linux so anything outside the package is not possible.

Comment 18

[Ludovic Hirlimann [:Usul]]

FWI : Wolfgang packages Thunderbird on OpenSue.

Comment 19

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #16)
> So I think it's "only" needed to commit the new icons to the right place and
> install them to dist/bin/icons.
> I could help with that if needed.

Awesome!  Wolfgang, I'm going to assign this to you to update the icons.  Can you make a patch for this?  Once the patch is ready you could probably ask Magnus or Mark (both in this bug) for a review.  Thanks!

Comment 20

[Hanno Mueller]

Hi. I'd like to point to Bug 484064 for a different take on this. Feel free to combine these two reports. Thank you.

Comment 21

[Hanno Mueller]

Argh. I posted this comment to the wrong bug report. Sorry.

Comment 22

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch

In that patch I tried to incorporate the different previous comments but it _could_ be that I misunderstood. So what I assumed is:

- XPMs should go away completely for everything
- the TB icon should be used for every window (no different ones for compose 
  windows or alike)
- the patch should work for the current set of assets of branding/nightly (e.g. 
  still XPMs)
- it's enough to have the 16, 32 and 48 pixel icons available for gtk2 (still 
  shipping the additional ones in icons/ to allow distribution integration)
- during cleanup of the Makefiles I completely ignored gtk in favour of gtk2
  (AFAIK gtk1 is not supported anyway anymore, right)

If any assumption is wrong please let me know.
(I've ignored the add and removal of binary (pngs) files in the patch but I've described what I propose) at the top of the patch.

Comment 23

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 379427 [details] [diff] [review]
patch

This patch is missing the file removal itself and therefore also the removed-files.in additions.
I'll create a second patch for that if this one is agreed on.

Comment 24

[Mark Banner (:standard8)]

Comment on attachment 379427 [details] [diff] [review]
patch

>diff --git a/mail/app/Makefile.in b/mail/app/Makefile.in
>--- a/mail/app/Makefile.in

>-ifneq (,$(filter windows os2 gtk gtk2,$(MOZ_WIDGET_TOOLKIT)))
> ifneq (,$(filter windows os2,$(MOZ_WIDGET_TOOLKIT)))
> ICON_SUFFIX=.ico
>-else
>-ICON_SUFFIX=.xpm
>-endif
...
> DESKTOP_ICONS = \
> 	abcardWindow \
> 	addressbookWindow \
> 	msgcomposeWindow \
> 	$(NULL) 
> 
> BRANDED_ICONS = \
> 	messengerWindow \
> 	$(NULL)
...

As you're effectively removing these icons on linux, you'll need to

a) remove the sources from http://mxr.mozilla.org/comm-central/source/mail/app/icons/gtk/
b) Ensure that we have matching (or very similar) changes to the non-official branding files in http://mxr.mozilla.org/comm-central/source/mail/branding/nightly/
c) add the files in a linux specific section of removed-files.in


> ifneq (,$(filter gtk gtk2,$(MOZ_WIDGET_TOOLKIT)))
>-ICON_FILES	= \
>-		$(DIST)/branding/mozicon50.xpm \
>-		$(DIST)/branding/mozicon16.xpm \
>-		$(NULL)
>-
> libs::
>-	$(INSTALL) $(IFLAGS1) $(ICON_FILES) $(DIST)/bin/icons
>-	if test -f $(DIST)/branding/mozicon128.png; then \
>-	  $(INSTALL) $(IFLAGS1) $(DIST)/branding/mozicon128.png $(DIST)/bin/icons; \
>-	fi;
>+	$(INSTALL) $(IFLAGS1) $(DIST)/branding/mozicon* $(DIST)/bin/icons

mozicon128.png is also a removed file - or at least you've not listed it in the new other-licenses makefile.

>diff --git a/other-licenses/branding/thunderbird/Makefile.in b/other-licenses/branding/thunderbird/Makefile.in

> ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
>-	cp $(srcdir)/mozicon16.xpm $(DIST)/branding/mozicon16.xpm
>-	cp $(srcdir)/mozicon50.xpm $(DIST)/branding/mozicon50.xpm
>-	cp $(srcdir)/mozicon128.png $(DIST)/branding/mozicon128.png
>-	cp $(srcdir)/default.xpm   $(DIST)/branding/default.xpm	
>+	cp $(srcdir)/mozicon16.png  $(DIST)/branding/mozicon16.png
>+	cp $(srcdir)/mozicon22.png  $(DIST)/branding/mozicon22.png
>+	cp $(srcdir)/mozicon24.png  $(DIST)/branding/mozicon24.png
>+	cp $(srcdir)/mozicon32.png  $(DIST)/branding/mozicon32.png
>+	cp $(srcdir)/mozicon48.png  $(DIST)/branding/mozicon48.png
>+	cp $(srcdir)/mozicon256.png $(DIST)/branding/mozicon256.png
>+	cp $(srcdir)/mozicon16.png  $(DIST)/branding/default16.png
>+	cp $(srcdir)/mozicon32.png  $(DIST)/branding/default32.png
>+	cp $(srcdir)/mozicon48.png  $(DIST)/branding/default48.png

Presumably these are all the Thunderbird logos? If so, I think maybe its time we came up with a better name for these files.

Also, lets put some comments in here, default*.png is the window icons, whatever*.png are icons shipped with the build for xxx to use. Though I am a bit dubious about having two sets of the same icons shipped.

>-ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
>-	cp $(srcdir)/mozicon50.xpm  $(DIST)/branding/messengerWindow.xpm
>-	cp $(srcdir)/mozicon16.xpm  $(DIST)/branding/messengerWindow16.xpm
>-endif

Like I've said above, we'll need similar change for non-official branding due to: http://mxr.mozilla.org/comm-central/source/mail/branding/nightly/gtk/ (with some form of replacement).

Comment 25

[Wolfgang Rosenauer [:wolfiR]]

(In reply to comment #24)
> 
> As you're effectively removing these icons on linux, you'll need to
> 
> a) remove the sources from
> http://mxr.mozilla.org/comm-central/source/mail/app/icons/gtk/

As noted I haven't created a list of files to be removed yet.

> b) Ensure that we have matching (or very similar) changes to the non-official
> branding files in
> http://mxr.mozilla.org/comm-central/source/mail/branding/nightly/

Why? The patch in the Makefiles make sure that it still works the way it used to work. (Not arguing if that's nice or not but as this bug doesn't contain any new non-branded files it was the way I did it.)

> c) add the files in a linux specific section of removed-files.in

See a)
 
> mozicon128.png is also a removed file - or at least you've not listed it in the
> new other-licenses makefile.

It is indeed. There is no 128 pixel icon in the attachment.
 
> Presumably these are all the Thunderbird logos? If so, I think maybe its time
> we came up with a better name for these files.

They are. And I don't care that much about the name. But making it easier to create integrated Linux apps it makes sense to use an unbranded name so that the names are the same for branding/non-branding builds.
 
> Also, lets put some comments in here, default*.png is the window icons,
> whatever*.png are icons shipped with the build for xxx to use. Though I am a
> bit dubious about having two sets of the same icons shipped.

Actually if we copy all icons (all sizes) to chrome/icons/default we could stop providing them in $(DIST)/icons completely.

> Like I've said above, we'll need similar change for non-official branding due
> to: http://mxr.mozilla.org/comm-central/source/mail/branding/nightly/gtk/ (with
> some form of replacement).

Agreed. Technically it needn't to be part of the patch for the branding case though.

With the first proposed patch the unoffical branding uses default.xpm for every window and gets rid of the window dependent icons to follow the branding case.

What's basically missing in the non-branding case are new (and more) icons (PNG format).
It could be done in one step but it doesn't need to be one step.

Comment 26

[Mark Banner (:standard8)]

(In reply to comment #25)
> > b) Ensure that we have matching (or very similar) changes to the non-official
> > branding files in
> > http://mxr.mozilla.org/comm-central/source/mail/branding/nightly/
> 
> Why? The patch in the Makefiles make sure that it still works the way it used
> to work. (Not arguing if that's nice or not but as this bug doesn't contain any
> new non-branded files it was the way I did it.)

The packaging doesn't work quite the same way. If someone is swapped between an official and non-official build (e.g. alpha -> beta), then they would potentially still have old non-branded icons (which is what removed-files.in prevents). I guess we could ifdef for official branding in removed-files.in.

> > Presumably these are all the Thunderbird logos? If so, I think maybe its time
> > we came up with a better name for these files.
> 
> They are. And I don't care that much about the name. But making it easier to
> create integrated Linux apps it makes sense to use an unbranded name so that
> the names are the same for branding/non-branding builds.

Agreed that we should use the same names. I'm just not convinced mozicon is best any more. How about mailicon?

> > Also, lets put some comments in here, default*.png is the window icons,
> > whatever*.png are icons shipped with the build for xxx to use. Though I am a
> > bit dubious about having two sets of the same icons shipped.
> 
> Actually if we copy all icons (all sizes) to chrome/icons/default we could stop
> providing them in $(DIST)/icons completely.

That would be nice.

> > Like I've said above, we'll need similar change for non-official branding due
> > to: http://mxr.mozilla.org/comm-central/source/mail/branding/nightly/gtk/ (with
> > some form of replacement).
> 
> Agreed. Technically it needn't to be part of the patch for the branding case
> though.

As long as we get removed-files.in right (with ifdefs), then I think that is ok.

Comment 27

[Wolfgang Rosenauer [:wolfiR]]

(In reply to comment #26)
> > Why? The patch in the Makefiles make sure that it still works the way it used
> > to work. (Not arguing if that's nice or not but as this bug doesn't contain any
> > new non-branded files it was the way I did it.)
> 
> The packaging doesn't work quite the same way. If someone is swapped between an
> official and non-official build (e.g. alpha -> beta), then they would
> potentially still have old non-branded icons (which is what removed-files.in
> prevents). I guess we could ifdef for official branding in removed-files.in.

Ah, ok, that makes sense. Didn't know that was supported.
The problem is there is no define for official branding (at least none really usable in that case because I've looked for it already to avoid using asterisk placeholders).
 
> > > Presumably these are all the Thunderbird logos? If so, I think maybe its time
> > > we came up with a better name for these files.
> > 
> > They are. And I don't care that much about the name. But making it easier to
> > create integrated Linux apps it makes sense to use an unbranded name so that
> > the names are the same for branding/non-branding builds.
> 
> Agreed that we should use the same names. I'm just not convinced mozicon is
> best any more. How about mailicon?
>
> > > Also, lets put some comments in here, default*.png is the window icons,
> > > whatever*.png are icons shipped with the build for xxx to use. Though I am a
> > > bit dubious about having two sets of the same icons shipped.
> > 
> > Actually if we copy all icons (all sizes) to chrome/icons/default we could stop
> > providing them in $(DIST)/icons completely.
> 
> That would be nice.

Ok, that also solves the name issue since the icons need to be named default* in that case anyway.
 
> > > Like I've said above, we'll need similar change for non-official branding due
> > > to: http://mxr.mozilla.org/comm-central/source/mail/branding/nightly/gtk/ (with
> > > some form of replacement).
> > 
> > Agreed. Technically it needn't to be part of the patch for the branding case
> > though.
> 
> As long as we get removed-files.in right (with ifdefs), then I think that is
> ok.

Probably not going to work but I'll look at it and complete the patch about the removed-files stuff.

@Andreas, in comment 7 you said you will work on the unbranded icons as well.
Do you have them ready or could finish that work. It appears we need them before we change the branding case.

Comment 28

[Andreas Nilsson (:andreasn)]

Attachment: preview of unbranded icon

Here is the 48x48 for the unbranded icon. Do people feel ok with this approach, not too close to the branded version?
I also recall Raphael had some ideas for doing something cool with the icon for the beta releases, but that's probably outside the scope of this bug.

Comment 29

[Mark Banner (:standard8)]

(In reply to comment #27)
> (In reply to comment #26)
> > > Why? The patch in the Makefiles make sure that it still works the way it used
> > > to work. (Not arguing if that's nice or not but as this bug doesn't contain any
> > > new non-branded files it was the way I did it.)
> > 
> > The packaging doesn't work quite the same way. If someone is swapped between an
> > official and non-official build (e.g. alpha -> beta), then they would
> > potentially still have old non-branded icons (which is what removed-files.in
> > prevents). I guess we could ifdef for official branding in removed-files.in.
> 
> Ah, ok, that makes sense. Didn't know that was supported.
> The problem is there is no define for official branding (at least none really
> usable in that case because I've looked for it already to avoid using asterisk
> placeholders).

True, I'd forgotten I've been trying to simplify that.

(In reply to comment #28)
> Created an attachment (id=379671) [details]
> preview of unbranded icon
> 
> Here is the 48x48 for the unbranded icon. Do people feel ok with this approach,
> not too close to the branded version?
> I also recall Raphael had some ideas for doing something cool with the icon for
> the beta releases, but that's probably outside the scope of this bug.

It was nightly/alpha releases (for Shredder) and covered by bug 433630.

Comment 30

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #28)
> Created an attachment (id=379671) [details]
> preview of unbranded icon
>
> Here is the 48x48 for the unbranded icon. Do people feel ok with this approach,
> not too close to the branded version?

Looks good.

> I also recall Raphael had some ideas for doing something cool with the icon for
> the beta releases, but that's probably outside the scope of this bug.

I think we can open another bug for that, lets get this one together with the icons we have now.

@rebron: do you already have a bug open?

Comment 31

[Andreas Nilsson (:andreasn)]

Attachment: application icons, unbranded

Comment 32

[Wolfgang Rosenauer [:wolfiR]]

Attachment: more complete patch

This is 99% complete now with the new unbranded icons.
The only caveat I see is that unbranded has no 256pixel icon and therefore "updating" from branded to unbranded versions would leave the branded 256 pixel icon around.
My preferred simple fix would be to add the 256 icon to the unbranded set as well.

(This patch contains the file removal and adds as recorded from hg diff so it's quite big)

Comment 33

[Andreas Nilsson (:andreasn)]

Attachment: 256x256 icon

To be put in the folder 256x256

Comment 34

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch #2

This should be finally complete.
- the icons are named mailiconXX.png in the source tree
- we install them only to chrome/icons/default/defaultXX.png to not duplicate the 
  files (integrators still can use them from there)
- the sets for branded and unbranded icons are the same (no problem with cross-
  updates)

Comment 35

[Mark Banner (:standard8)]

Comment on attachment 379956 [details] [diff] [review]
patch #2

>-ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2)
>-ICON_DIR=gtk
>-else
> ICON_DIR=$(MOZ_WIDGET_TOOLKIT)
>-endif

I think we may as well drop ICON_DIR for now as well.

>diff --git a/mail/installer/removed-files.in b/mail/installer/removed-files.in

>+#ifdef MOZ_WIDGET_GTK2
>+chrome/icons/default/abcardWindow.xpm
>+chrome/icons/default/abcardWindow16.xpm
>+chrome/icons/default/addressbookWindow.xpm
>+chrome/icons/default/addressbookWindow16.xpm
>+chrome/icons/default/msgcomposeWindow.xpm
>+chrome/icons/default/msgcomposeWindow16.xpm
>+chrome/icons/default/messengerWindow.xpm
>+chrome/icons/default/messengerWindow16.xpm
>+chrome/icons/default/default.xpm
>+icons/mozicon128.png
>+icons/mozicon16.xpm
>+icons/mozicon50.xpm
>+#endif

Please put these just below the other chrome/ removals.

r=Standard8 with those fixed. Not sure if we need a quick ui-review from Bryan on here.

Comment 36

[Wolfgang Rosenauer [:wolfiR]]

comm-central - changeset - 2738:30a480c4c0de 

Checked in with the above mentioned changes