New icons for address book

As abcard is now a dude rather than a address card, because of the changes in Bug 484409 , this will also affect the icon shown in the message compose title.

wow, big patch. Here's the hg version of all the work andreas did plus some small additions. I created an icons directory in the addrbook directory because it was getting annoying mixed in with all the CSS files. I took the new compose icons and moved them into the generic icons directory since they're being used by a number of other places and not just the addressbook. Changes to the addressbook.xul so I could get the menu item icons to show up properly. I tried to cleanup the areas I touched into the right alignment. I rearranged the File menu so the print items could use the print icons and I think they make more sense separated by type instead of action. So now it looks like this: Print Preview Contact... Print Contact... -------------------- Page Setup... -------------------- Print Preview Address Book... Print Address Book... changed primaryToolbar.css so that we'd use the new Write button icon. added an id to messagengercompose.xul so I could select it in the css file. changed messaengercompose.css so we'd use the new contact and addressbook icons in the menus. the jar.nm file is wrapping a little long right now. Phil, can you take a look at this patch?

phil: do you want me to move this over to magnus? I know you're working on the autoconfig bug which is a blocker and a bit bigger magnus: do you want to take this?

Yeah, sorry, I didn't realize autoconfig was going to eat a whole weekend before I even got to the code. The one thing that worries me here (and elsewhere) is switching from -moz-image-region to a lot of separate images. I can't find the bug right now, but I remember fairly recently someone running a patch through Firefox's perf tests, and finding that even combining just a few images into one was a huge difference. Having done it, I know assembling a single combined image is an annoying fiddly task, but it's worth doing.

ugh, I was afraid this was going to come up. The image region code is a real pain, but I might have recently noticed some effects of the individual images with the folder pane patch. Images in collapsed children only appeared to load after they were expanded, not sure if that is the same issue. I'll see if I can find that bug.

the whole image splicing process feels to me like something that really ought to be part of the build process somehow. It's fiddly, as you say, and yet should be automateable once-and-for-all. Not sure how to make that happen though...

Yeah, not having icons for hidden things preloaded is another aspect of it: with combined images you pay that cost up front, but the marginal cost of another 16x16 or 64x64 pixels is absolutely nothing compared to the cost of dragging something out of a jar on disk.

Oh, I had no idea of this issue, but if it impacts a lot on performance I'll rework it. Here are the toolbar icons combined into one. Only active and disabled states, since the icons don't change on hover (Firefox only does two states as well). No delete icon, since we'll use a GTK+ stock icon for that. Will rework the css and attach that as well.

here's a patch with all the same updates but using moz-image-region instead of the individual files.

Comment on attachment 373213 [details] [diff] [review] updated patch using moz-image-region Looks really nice! +#menu_properties { + list-style-image: url("chrome://messenger/skin/addressbook/icons/addressbook-toolbar-small.png"); + -moz-image-region: rect(96px 16px 112px 0px); /* contact-properties-small.png */ +} There's a gtk stock icon for properties. (And we use it for right-click contact | properties). Maybe it would be best to use that? Or at least make it consistent. http://library.gnome.org/devel/gtk/2.16/gtk-Stock-Items.html#GTK-STOCK-PROPERTIES--CAPS + list-style-image: url("chrome://messenger/skin/addressbook/icons/addressbook-toolbar.png"); trailing tab, x2

(In reply to comment #13) > (From update of attachment 373213 [details] [diff] [review]) > Looks really nice! > > +#menu_properties { > + list-style-image: > url("chrome://messenger/skin/addressbook/icons/addressbook-toolbar-small.png"); > + -moz-image-region: rect(96px 16px 112px 0px); /* > contact-properties-small.png */ > +} > > There's a gtk stock icon for properties. (And we use it for right-click contact > | properties). Maybe it would be best to use that? Or at least make it > consistent. > http://library.gnome.org/devel/gtk/2.16/gtk-Stock-Items.html#GTK-STOCK-PROPERTIES--CAPS Ah right, I'm going to go with the properties/info icon we have instead of the gtk-stock icon because it feels more like a contact info icon than the stock one does. > + list-style-image: > url("chrome://messenger/skin/addressbook/icons/addressbook-toolbar.png"); > > trailing tab, x2 wtf! Where did I get tabs from? :) I also added in some popup menu properties that I missed on the first pass. Now the right click menus use the same icons as the file/view/edit menus. Thanks!

Comment on attachment 374307 [details] [diff] [review] updated patch w/o tabs r=mkmelin

changeset: 2466:3fe3e732c210 http://hg.mozilla.org/comm-central/rev/3fe3e732c210 ->FIXED

I think the older patch got in, I just noticed some of the changes to the popup menu didn't get through. http://hg.mozilla.org/comm-central/rev/3fe3e732c210 looks like attachment 373213 [details] [diff] [review] instead of attachment 374307 [details] [diff] [review]

hg export 2466 | patch -R -p1 then applied the other patch again with patch changeset: 2488:4395ed099cde http://hg.mozilla.org/comm-central/rev/4395ed099cde ->FIXED again. Sorry for the mess :(