Closed Bug 484064 Opened 16 years ago Closed 16 years ago

tango application icons

Categories

(Thunderbird :: General, defect)

All
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: clarkbw, Assigned: wolfiR)

References

Details

Attachments

(7 files, 2 obsolete files)

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/
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.
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.
(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.
Adding some interested parties
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?
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
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.
Assignee: nobody → nisses.mail
(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.
Attached file new icons (deleted) —
Attached image comparision between old and new icons (deleted) —
BTW, Also added 256x256 icon as it will work better in GNOME Do and other things using bigger-than-normal icons.
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.
Assignee: nisses.mail → caillon
Are new icons (attachment 371441 [details]) really tango style, according to bug's title?
Status: NEW → ASSIGNED
Attached image screenshot of icon in action (deleted) —
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.
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.
Note that I didn't fix up the Firefox icon, I think we need to open a separate bug about that.
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.
(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.
FWI : Wolfgang packages Thunderbird on OpenSue.
(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!
Assignee: caillon → mozilla
Hi. I'd like to point to Bug 484064 for a different take on this. Feel free to combine these two reports. Thank you.
Argh. I posted this comment to the wrong bug report. Sorry.
Attached patch patch (obsolete) (deleted) — Splinter Review
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 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.
Attachment #379427 - Flags: review?(bugzilla)
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).
Attachment #379427 - Flags: review?(bugzilla) → review-
(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.
(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.
(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.
Attached image preview of unbranded icon (deleted) —
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.
(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.
(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?
Attached patch more complete patch (obsolete) (deleted) — Splinter Review
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)
Attachment #379427 - Attachment is obsolete: true
Attached image 256x256 icon (deleted) —
To be put in the folder 256x256
Attached patch patch #2 (deleted) — Splinter Review
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)
Attachment #379846 - Attachment is obsolete: true
Attachment #379956 - Flags: review?(bugzilla)
Attachment #379956 - Flags: ui-review?(clarkbw)
Attachment #379956 - Flags: review?(bugzilla)
Attachment #379956 - Flags: review+
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.
Attachment #379956 - Flags: ui-review?(clarkbw) → ui-review+
comm-central - changeset - 2738:30a480c4c0de Checked in with the above mentioned changes
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: