429725 - Icons in the Applications Prefpane

Status: NEW

(Firefox :: Theme, defect, P3)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Mockup of what icons should be used in the applications prefpane

Now that we have all of the icons landed we need to update the Applications Prefpane.

content that does not have an icon from the system: http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/Bookmarks-folder-aero.png

webcal: http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/places/calendar.png

mailto: need to land

application: http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/preferences/application.png

Add live bookmark: http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/livemark-folder.png

Save file: http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/preferences/saveFile.png

Comment 1

[Alex Faaborg [:faaborg] (Firefox UX)]

Requesting blocking to pick up wanted, does anyone have cycles to work on this?

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Adding some other potential suspects for helping us find an opportunistic fix, but agree that this is wanted-not-blocking.

Comment 3

[Kai Liu]

I can take this if nobody else will; it's a very simple patch.  There are, however, a few questions I'd like to ask first...

1) Why is the generic file icon named "Bookmarks-folder.png"?  It's not a folder...

2) Are we doing this for Windows only?  If we wish to extend this to Mac/Linux, then the appropriate icons will need to land (e.g., AFAIK, there's no calendar icon for the Mac)

3) What's the ETA for the mail icon?

Comment 4

[Kai Liu]

Attachment: Windows-only patch

This is a Windows-only patch.  All that is needed here is for Alex to provide mail.png and mail-aero.png.

Not requesting review yet because I don't know if this is supposed to be Windows-only or not.  Set the review? flag if you decide that this is Windows-only.  Otherwise, let me know so that I could extend this to Mac or Linux (provided the icons, of course :)).

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

>1) Why is the generic file icon named "Bookmarks-folder.png"?  It's not a
>folder...

We often name icons based on how the icon was used as opposed to what it is a picture of (like "history" instead of "clock").  This means that when the same image is used in a different context we either need to duplicate the icon and change the name, or use an unusually named icon in the new context.  You can duplicate the default favicon in the preferences directory and name it genericContent.png if you think it would be nicer for people to be able to theme this differently.

>2) Are we doing this for Windows only?  If we wish to extend this to Mac/Linux,
>then the appropriate icons will need to land (e.g., AFAIK, there's no calendar
>icon for the Mac)

We should do this on all platforms, although I think linux is pretty much set right now.  I'll review the other platforms right now to see what we need.

>3) What's the ETA for the mail icon?

I'll attach them

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: mail.png

Comment 7

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: mail-aero.png

Comment 8

[Alex Faaborg [:faaborg] (Firefox UX)]

On OS X everything looks good except that a lot of content types are missing a generic icon, and we need to use icons for mailto and webcal.  I'm assuming the application content item is not applicable since apps are distributed through dmg's. 

Comment 9

[Alex Faaborg [:faaborg] (Firefox UX)]

On linux everything looks good except for mailto and webcal needing icons.  cc'ing the linux themers.

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

mail-aero.png would probably work ok on OS X (either as a placeholder or final icon) if Stephen doesn't have an icon ready.  The proto calendar icon is here https://bugzilla.mozilla.org/attachment.cgi?id=317753

Comment 11

[Kai Liu]

Okay.  What would you like for me to use for the pinstripe generic icon?

Comment 12

[Reed Loden [:reed]]

(In reply to comment #9)
> On linux everything looks good except for mailto and webcal needing icons. 

The mail icon is in bug 418868, attachment 304998 [details]. For webcal, the "calendar" icon has already landed. http://mxr.mozilla.org/seamonkey/source/browser/themes/gnomestripe/browser/places/calendar.png

Is that all you need?

Comment 13

[Alex Faaborg [:faaborg] (Firefox UX)]

>What would you like for me to use for the pinstripe generic icon?

where does the the default favicon live in pinstripe?  I couldn't find it, but we should either use or duplicate that for generic content.

Comment 14

[Stephen Horlander [:shorlander]]

(In reply to comment #13)
> >What would you like for me to use for the pinstripe generic icon?
> 
> where does the the default favicon live in pinstripe?  I couldn't find it, but
> we should either use or duplicate that for generic content.
> 

http://mxr.mozilla.org/seamonkey/source/toolkit/themes/pinstripe/mozapps/places/defaultFavicon.png

Comment 15

[Kai Liu]

Attachment: patch v2 for all platforms

I should note that the generic icon will only be used for handlers without an extension or a MIME type (for example, webcal and mailto, if they weren't given special icons of their own, would fall into this category).  In your screenshot, the Windows Media handlers had generic icons, and those won't be replaced by this patch replaced (FWIW, you should not be seeing those generic icons for Windows Media handlers unless your system's file type handlers are misconfigured).  Getting rid of generic icons for handlers that do have extensions of MIME types would be a more invasive process and probably not something that we should try this late.

Comment 16

[Kai Liu]

Attachment: icons to land

Comment 17

[Stephen Horlander [:shorlander]]

Attachment: Proto Mailto Icon

Would it be possible to include this icon for OS X? Or should I do that separately?

Comment 18

[Kai Liu]

> Would it be possible to include this icon for OS X? Or should I do that
> separately?
> 

All that matters to me is that the icons are in place when the new applications.js goes in, and I don't care whether that's by you landing them before this bug lands or by the icons landing as a part of this bug.  So it's up to you how you want to do this.  :)  And if the icons are landed as a part of this bug, I'll change the mail icon to what you provide, since the current icon is just something that I stuck there so that there is an icon.

Comment 19

[Stephen Horlander [:shorlander]]

If you wouldn't mind to land it with this bug that would be most appreciated :) Thank you!

Comment 20

[Kai Liu]

*checks bugmail*  *notices that Alex is landing the Windows mailto icons in bug 430759*

Hey, Alex, since you're landing the Windows mailto icons in the upcoming icon drop, could you also land the Mac icons for this bug too (see zip file I attached and the mail icon that Stephen attached)?  Then I could set 430759 to block this bug and consolidate the icon landings.

Comment 21

[Alex Faaborg [:faaborg] (Firefox UX)]

I don't have cvs access myself, the mail icons are landing over in the windows bug since I use a script to generate all of the files and perform some testing and additional compression.

Comment 22

[Kai Liu]

Attachment: icons to land, updated

Icons to land, either as a part of the 430759 landing or as a part of this bug's landing.  (removed the winstripe icons since that's in 430759 & added Stephen's icon)

Comment 23

[Kai Liu]

Attachment: patch v2.1 (unbitrot)

Minor update to the patch, now that the dust has cleared from bug 430759.

Comment 24

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 318077 [details] [diff] [review]
patch v2.1 (unbitrot)

>Index: browser/components/preferences/applications.js

>+#if defined(XP_WIN) || defined(XP_MACOSX)

Does the XUL preprocessor support this syntax? It doesn't appear to, from a quick skim of preprocessor.py.

Otherwise this looks fine.

Comment 25

[Kai Liu]

(In reply to comment #24)
> Does the XUL preprocessor support this syntax?
> 

From skimming Preprocessor.py and Expression.py, you're right, it doesn't support that (or any sort of binary operators except for equality).  I don't think that I want to use #ifndef XP_UNIX because that does not necessarily mean Windows or Mac, so I guess the only option that I have would be something like:

#ifdef XP_WIN
#define SPECIFY_GENERIC_ICON_FOR_APP_HANDLERS
#elifdef XP_MACOSX
#define SPECIFY_GENERIC_ICON_FOR_APP_HANDLERS
#endif

#ifdef SPECIFY_GENERIC_ICON_FOR_APP_HANDLERS
// code
#endif

It seems mildly hackish; is there a better way?

Comment 26

[Kai Liu]

Attachment: patch v2.2

Comment 28

[Alex Faaborg [:faaborg] (Firefox UX)]

This was a tracking bug for Firefox 3 development, resolving.

Comment 29

[Kai Liu]

(In reply to comment #28)
> This was a tracking bug
>
Huh?

> resolving.
> 
Shouldn't be resolved as FIXED as it's not.  Did you want to WONTFIX?

Comment 30

[Alex Faaborg [:faaborg] (Firefox UX)]

>Shouldn't be resolved as FIXED as it's not.  Did you want to WONTFIX?

While cleaning up about 150 bugs I messed up on this one, I thought this was a bug about the icons at the top of the preferences window.  We still need to get this fixed, hopefully for 3.1

Comment 31

[Alex Faaborg [:faaborg] (Firefox UX)]

this bug is eligible for bug 462081

Comment 32

[Shawn Wilsher :sdwilsh]

Gavin - any chance on getting a review on this any time soon?

Comment 33

[Alfred Kayser]

Why the hardcoded references to images in /skin/ such as:
chrome://browser/skin/preferences/application.png
chrome://browser/skin/places/calendar.png
chrome://browser/skin/preferences/mail.png
chrome://browser/skin/preferences/generic.png

Comment 34

[Robert Kaiser]

(In reply to comment #33)
> Why the hardcoded references to images in /skin/

Probably because it was simpler to implement that way for them. I have reworked this in the SeaMonkey version to not hardcode them, that work could be backported to the Firefox implementation.

Comment 35

[Alex Faaborg [:faaborg] (Firefox UX)]

if you decide to merge the images and use -moz-image-region let me know and I will give you the merged file (need to do it on my side since I'm using scripts to generate all the image files, so it is easier for us to update them later).

Comment 36

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 318808 [details] [diff] [review]
patch v2.2

This patch no longer applies cleanly, unfortunately. The approach from comment 34/comment 35 might be worth investigating if someone wants to bring this patch back up to date.

Comment 37

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P3 - Polish issue that is in a secondary interface, occasionally encountered, or is not easily identifiable.

It would be good to get this right, but it's a secondary interface and people aren't currently noticing that the icons are wrong.

Comment 38

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 39

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.