348720 - New icon set for "SeaMonkey Default Theme"

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Manuel Reimer]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.8.0.5) Gecko/20060720 SeaMonkey/1.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.8.0.5) Gecko/20060720 SeaMonkey/1.0.3

A new SeaMonkey theme is in work. This bug is meant as "tracking bug". Please use the newsgroup mozilla.dev.apps.seamonkey for discussion.

First patches follow soon...

Reproducible: Always

Comment 1

[Manuel Reimer]

Attachment: Patch to add the new icons

This is the patch to edit all the css files

Comment 2

[Manuel Reimer]

And here:

http://prefbar.mozdev.org/icons.tar.gz

are the icons. I'm unable to attach them here. The file is too large.

Comment 3

[Robert Kaiser]

Comment on attachment 233785 [details] [diff] [review]
Patch to add the new icons

>Index: themes/classic/jar.mn
>===================================================================
>-  skin/classic/messenger/smime/icons/smbtn1.gif                         (messenger/smime/icons/smbtn1.gif)
>+  skin/classic/messenger/smime/icons/smimeicons.png                        (messenger/smime/icons/smimeicons.png)
>   skin/classic/messenger/smime/icons/sbSignOk.gif                       (messenger/smime/icons/sbSignOk.gif)

Keep the right side of this line alinged, plase :)

Requesting reviews, as Manuel seems to have missed that...

Comment 4

[Stefan [:stefanh]]

+/*
 #search-button:hover {
   list-style-image: url("chrome://communicator/skin/icons/search-hov.gif");
 }
 
 #search-button:hover:active {
   list-style-image: url("chrome://communicator/skin/icons/search-act.gif");
 }
+*/

Should be removed, right?

Comment 5

[Manuel Reimer]

(In reply to comment #4)
> +/*
>  #search-button:hover {
>    list-style-image: url("chrome://communicator/skin/icons/search-hov.gif");
>  }
> 
>  #search-button:hover:active {
>    list-style-image: url("chrome://communicator/skin/icons/search-act.gif");
>  }
> +*/
> 
> Should be removed, right?

No, I just don't have the new icons for the two, currently. The patch is just for my current icon set. There may (will) be changes in future.

Comment 6

[[e-mail]]

Comment on attachment 233785 [details] [diff] [review]
Patch to add the new icons

>+        skin/classic/communicator/home.png              (skin/classic/home.png)
I think this file should be merged into navigatoricons.png (and the stop button removed, as it's available in printicons.png).

>+/*
>+#helpHomeButton[disabled="true"] {
>+  -moz-image-region: rect(0 119px 29px 90px) !important;
> }
>+*/
I'm happy for this to be uncommented even though we never disable it.

>+  skin/classic/messenger/icons/mailicons.png                            (messenger/icons/mailicons.png)
I'd prefer to call this messengericons.png (and similarly for navigatoricons.png if you hadn't already guessed).

>+  skin/classic/messenger/addressbook/icons/im.png                       (messenger/addressbook/icons/im.png)
I think this file should be merged into messengericons.png (or possibly the addressbook should use addressbookicons.png).

>+.toolbarbutton-1 .toolbarbutton-icon {
>+  -moz-margin-end: 0px;
> }
Why is this rule not in button.css above? And why -moz-margin-end anyway?

>+#button-getmsg[buttonstyle="text"] > stack > .toolbarbutton-menubutton-dropmarker {
>+  margin: 0px 2px 0px 55px !important;
>+}
Is this rule necessary?

Comment 7

[Manuel Reimer]

(In reply to comment #6)
> >+        skin/classic/communicator/home.png   (skin/classic/home.png)
> I think this file should be merged into navigatoricons.png (and the stop
> button removed, as it's available in printicons.png).

Sure?
The stop icon in navigatoricons.png comes from modern. I took a look at the classic icon set and there is no stop icon.
Maybe it's a bad decision at all to place icons, which are used in more than one application, in the directory of only one application? Perhaps it's better to rename "printicons.png" into something like "globalicons.png" and place all the stuff used in more than one app (also "Save", "Check spelling", ...) there?

So far we also have icons in two files. For example the "New"-Icon in Composer is the same icon as the "New Message"-Icon in Mail/News. I have to copy this icon everytime I changed it.

But of course I could also "cross-link" from Composer to messengericons.png here. Or maybe better cross-link from Mail/News to editoricons.png? We already do this to get "Check spelling" or "Save"...

> >+/*
> >+#helpHomeButton[disabled="true"] {
> >+  -moz-image-region: rect(0 119px 29px 90px) !important;
> > }
> >+*/
> I'm happy for this to be uncommented even though we never disable it.

... Yes, why not...

> >+  skin/classic/messenger/icons/mailicons.png                            (messenger/icons/mailicons.png)
> I'd prefer to call this messengericons.png (and similarly for
> navigatoricons.png if you hadn't already guessed).

Yes, good idea.

> >+  skin/classic/messenger/addressbook/icons/im.png                       (messenger/addressbook/icons/im.png)
> I think this file should be merged into messengericons.png (or possibly the
> addressbook should use addressbookicons.png).

addressbook uses messengericons.png. Addressbook also uses icons used by Mail/News, so separating them would be difficult.

Sure that the IM-Button will stay? Will we be able to cover more than AIM with this button?

> >+.toolbarbutton-1 .toolbarbutton-icon {
> >+  -moz-margin-end: 0px;
> > }
> Why is this rule not in button.css above? And why -moz-margin-end anyway?

This one was copied from Firefox. I tink I've had added it to reduce space used by the buttons.

> >+#button-getmsg[buttonstyle="text"] > stack > .toolbarbutton-menubutton-dropmarker {
> >+  margin: 0px 2px 0px 55px !important;
> >+}
> Is this rule necessary?

Hmmm. This may be obsolete.

Comment 8

[Robert Kaiser]

(In reply to comment #7)
> Maybe it's a bad decision at all to place icons, which are used in more than
> one application, in the directory of only one application? Perhaps it's better
> to rename "printicons.png" into something like "globalicons.png" and place all
> the stuff used in more than one app (also "Save", "Check spelling", ...) there?

commonicons.png or communicatoricons.png sounds better, and place them inside the communicator directory. If it's the same icons for multiple components, it sounds like a good idea to have it there.

> But of course I could also "cross-link" from Composer to messengericons.png
> here. Or maybe better cross-link from Mail/News to editoricons.png? We already
> do this to get "Check spelling" or "Save"...

In this case, referencing a different component should be OK - but the other way round: reference composer from messenger, as we have the same dependency in the actual code. Making composer depend on messenger is a bad idea, but messenger already depends on composer, so do it this way (for this that are specific to composing, more common stuff should probably go into communicator).

Comment 9

[Manuel Reimer]

Attachment: Second patch to add new icons

Second try...

Icons are here: http://prefbar.mozdev.org/icons2.tar.gz

Comment 10

[Stefan [:stefanh]]

Hmm, is it possible to use "opacity" or "-moz-opacity" instead of having different images for, lets say, disabled toolbarbuttons? Or does that not work with -moz-image-region?

Comment 11

[Robert Kaiser]

(In reply to comment #10)
> Hmm, is it possible to use "opacity" or "-moz-opacity" instead of having
> different images for, lets say, disabled toolbarbuttons? 

Not if disabled buttons should be "grayed out", i.e. have the colors faded out to a gray value...

Comment 12

[Stefan [:stefanh]]

(In reply to comment #11)
> (In reply to comment #10)
> > Hmm, is it possible to use "opacity" or "-moz-opacity" instead of having
> > different images for, lets say, disabled toolbarbuttons? 
> 
> Not if disabled buttons should be "grayed out", i.e. have the colors faded out
> to a gray value...
> 

Well, imo they're too grayed out to fit in with the native-styled mac toolbar. The fact that the icons are set in one app-specific file for all platforms (not MReimers fault since this is the way it has been in Classic) makes it kind of hard to get a native look and feel...

Comment 13

[[e-mail]]

Comment on attachment 236708 [details] [diff] [review]
Second patch to add new icons

>+  skin/classic/communicator/icons/communicatoricons.png                 (communicator/icons/communicatoricons.png)
Would it be too much to ask to have these in the order back, forward, reload, stop, home, print? (See below about why I mentioned reload)

>+  skin/classic/navigator/icons/navigatoricons.png                       (navigator/icons/navigatoricons.png)
Having just griped that the IM icon was the only one in its file you now do the same thing for navigator ;-)

>+/*.toolbarbutton-1 .toolbarbutton-icon {*/
>+.toolbarbutton-icon {
>+  -moz-margin-end: 0px;
>+}
This doesn't seem to save any space, and I think it looks nicer without.

> #button-file {
As of bug 282189 this button has its own native dropmarker; I'll let you decide how you want to resolve this.

>+.toolbarbutton-1 .toolbarbutton-icon {
>+  -moz-margin-end: 0px;
> }
And again as above.

>-#button-newim {

>+#button-newim {
Was there a reason that you moved this block of rules?

There were apparently six lines that you added with trailing whitespace. The jst-review simulacrulm http://beaufour.dk/jst-review/?patch=236708 also lists other things that it considers errors but that's because it's not designed to review CSS.

Otherwise this is almost ready to go!

Comment 14

[[e-mail]]

Attachment: Display issue in 256 colours

As you can see, alpha transparency doesn't work too well in 256 colours (it's a good thing I wasn't using a cairo build, those don't work in 256 colours at all)

Comment 15

[Manuel Reimer]

(In reply to comment #13)
> Would it be too much to ask to have these in the order back, forward, reload,
> stop, home, print? (See below about why I mentioned reload)

Of course this would be possible, even if this doesn't have any effect on display or functionality. Just again some modifications in all the CSS files using communicatoricons.png

> Having just griped that the IM icon was the only one in its file you now do
> the same thing for navigator ;-)

Maybe, but there is a good reason to keep navigatoricons.png. There will be definetly more icons in future and there is no reason to place all icons, which are only used in navigator, in communicatoricons. Currently there is only *one* icon used only by navigator, but I don't think this is a problem. As soon as we start to include some Firefox features, we will get much more of them and I don't like to move the icons from communicatoricons to navigatoricons at a later date and modify all my CSS again.

> > #button-file {
> As of bug 282189 this button has its own native dropmarker; I'll let you
> decide how you want to resolve this.

Hmmmmm... Yes, that's true. And I wondered why I have two arrows now ;-) The solution is easy: I'll just remove the arrow in my icons.

> >-#button-newim {
> 
> >+#button-newim {
> Was there a reason that you moved this block of rules?

I want to have this one separated from the other icons. As long as it's only possible to enter AIM messenger information for this button, I think this one is a good candidate for removal.

Comment 16

[Manuel Reimer]

(In reply to comment #14)
> As you can see, alpha transparency doesn't work too well in 256 colours (it's a
> good thing I wasn't using a cairo build, those don't work in 256 colours at
> all)

Hmmm, yes, this looks bad, but I don't think I'm able to do anything to get this looking better.

Comment 17

[[e-mail]]

(In reply to comment #15)
>There will be definetly more icons in future
Even without the firefox features, I guess we might want to move the Home and Bookmarks buttons to the navigator toolbar. Well, Home was a bad choice ;-)

>The solution is easy: I'll just remove the arrow in my icons.
Fine.

>I want to have this one separated from the other icons. As long as it's only
>possible to enter AIM messenger information for this button, I think this one
>is a good candidate for removal.
That's a good explanation for why you made it the last address book icon, but it doesn't justify moving the CSS rules.

Comment 18

[[e-mail]]

Comment on attachment 236708 [details] [diff] [review]
Second patch to add new icons

r+sr=me with the following changes:
1. -moz-margin-end removed everywhere
2. dropmarker removed from the file image
3. #button-newim rules moved back (but image left at the end of the png)
4. trailing whitespace removed everywhere you added it by mistake

Comment 19

[Manuel Reimer]

(In reply to comment #17)
> Even without the firefox features, I guess we might want to move the Home and
> Bookmarks buttons to the navigator toolbar. Well, Home was a bad choice ;-)

Why? "Home" is used by "help", too. So far "help" only needs the communicatoricons.png file. If we would have "Home" in navigatoricons.png, then "help" would have to load all navigator icons just for the "Home" button.

> That's a good explanation for why you made it the last address book icon, but
> it doesn't justify moving the CSS rules.

ACK

Comment 20

[Manuel Reimer]

Attachment: New patch to add icons

(In reply to comment #18)
> 1. -moz-margin-end removed everywhere

I hope I've removed them all

> 2. dropmarker removed from the file image

done

> 3. #button-newim rules moved back (but image left at the end of the png)

done

> 4. trailing whitespace removed everywhere you added it by mistake

I hope I got them all.

I've also modified the "publish"-button as it looked like the globe gets cutted on the bottom.

The icon archive is, again, available here:

http://prefbar.mozdev.org/icons2.tar.gz

I'm sure there will be some future updates for the icons, but the most important thing is to have the "backend" to read the icon files.

@Neil: I've requested review again, to allow you to have a final look at it. Please check in as soon as you decided that everything is OK now. I don't have the permissions to do that.

As soon as this one is checked in I'll continue with the next patch to add some of the small icons, which are still missing.

Comment 21

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Very nice!

Comment 22

[Robert Kaiser]

This first step is in, and I just checked in a one-liner to fix mailcompose icons showing up. Looks quite good :)

So, will we have other bugs for the rest of the icons (those appearing elsewhere than in primary toolbars, or will we continue to do them in this bug until everything is done?

Comment 23

[Igor Velkov]

Why you broke "classic" theme making it looks like "modern"?
You can add new theme if you want strange, large and bright multi-color icons on your screen. 
I don't.
Please, keep old look. Maybe, on some "old-styled", or else called pre-installed theme.

Comment 24

[Robert Kaiser]

Bug reports are not for discussion of "why do we do this at all?" - please go to our newsgroups for discussions.
That said, the decision has been made, the old "Classic" icon set is outdated and will not be pre-installed in future releases. If you want to keep it, do a 3rd-party theme. That decision will not be revised.
Again, as comment #0 says, "Please use the newsgroup mozilla.dev.apps.seamonkey for discussion."

Comment 25

[Ian Neal]

Out of interest, are there any timescales / progress on the rest of the icons?

Comment 26

[Eyal Rozenberg]

I whole-heartedly agree with comment #23 - the new button icons used in the classic theme are a remake of the 'modern' theme button icons. Why not update both themes as such rather than try to 'remix' them?

Regardless, I find the new 'reload' button looking too much like an 'up' button, which is confusing for me.

Comment 27

[ChrisX]

To the people who wrote comments 23 & 26:

I just experimented a bit with the old and new "classic.jar" zip files and I think it's quite simple to restore at least the old forward/backward/stop/reload/home button set if one knows how to handle transparency in png files (which are used instead of gifs now). 

I did a quick and dirty "fix" and got Seamonkey 1.5a running with the old buttons without  bigger problems. If I have the time to debug the smaller problems I'll try and create a third party skin or at least make public the modified classic.jar file as soon as a more or less finished 1.5rc is released (if nobody else has done this by then).
 
I like the new buttons by the way and don't want to bash them, but I'm also fond of the traditional set since this was the only visible thing left over from the good old Netscape times. There should be a choice for the user built into SeaMonkey between Modern, Classic(new) and Classic(old) because I think there are many more people who'll miss the old buttons.

Comment 28

[Manuel Reimer]

(In reply to comment #27)
> I did a quick and dirty "fix" and got Seamonkey 1.5a running with the old
> buttons without  bigger problems. If I have the time to debug the smaller
> problems I'll try and create a third party skin or at least make public the
> modified classic.jar file as soon as a more or less finished 1.5rc is released
> (if nobody else has done this by then).

BTW: It would be better to get a copy of the theme out of Mozilla 1.7.13 as "patching" the current "new classic" will get more and more difficult. My next update is nearly finished. This update will replace all the smaller icons in Mail/News.

> I like the new buttons by the way and don't want to bash them, but I'm also
> fond of the traditional set since this was the only visible thing left over
> from the good old Netscape times. There should be a choice for the user built
> into SeaMonkey between Modern, Classic(new) and Classic(old) because I think
> there are many more people who'll miss the old buttons.

But there is noone in the SeaMonkey team who is able or has the time to create regular updates for the "old" classic. Maintaining the old classic is pretty difficult. If you magnify one of the "old classic" toolbar icons, you will see, that the icon consists of many pixels with just a few colors. I think creating new icons for this theme will be a pretty hard job!

Comment 29

[ChrisX]

>BTW: It would be better to get a copy of the theme out of 
>Mozilla 1.7.13 as "patching" the current "new classic" will 
>get more and more difficult.

Do you mean by this that it is possible to just replace the current classic.jar with an older one in SM 1.5?

Comment 30

[Manuel Reimer]

Attachment: Patch for the "smaller stuff" in Mail/News

This patch will modify the CSS files for messenger to get the new folder pane icons.

Comment 31

[Manuel Reimer]

Attachment: ... and the icons

The icons, to be placed in mozilla/themes/classic/messenger/icons

Comment 32

[[e-mail]]

Comment on attachment 255687 [details] [diff] [review]
Patch for the "smaller stuff" in Mail/News

>+  skin/classic/communicator/communicatorBindings.xml                    (communicator/communicatorBindings.xml)
You don't seem to have included this file anywhere. (For testing I copied modern/global/globalBindings.xml in its entirety.)

>+  skin/classic/messenger/icons/check.png                                (messenger/icons/check.png)
>+  skin/classic/messenger/icons/dot.png                                  (messenger/icons/dot.png)
I'm not too keen on these icons, they look too cartoony.

> .acctCentralRowTitleBox {
>-  background-color: #C7D0D9;
>+/*  background-color: #C7D0D9;*/
>+  background-color: -moz-dialog;
>   font-size: 150%;
>   font-weight: bold;
>   color: #000000;
> }
The background colour change is OK but only if you also change the text colour to a system colour. Obviously you need to get rid of the comment ;-)

>+  list-style-image: url("chrome://messenger/skin/icons/readcol-unread.png");
This file is also missing.

I notice that there don't seem to be as many images as there used to be, has Modern always had fewer images?

Some of your images had execute bits, which is incorrect.

Comment 33

[[e-mail]]

(In reply to comment #32)
>(From update of attachment 255687 [details] [diff] [review])
>>+  list-style-image: url("chrome://messenger/skin/icons/readcol-unread.png");
>This file is also missing.
Also while readcol-read.png exists it isn't listed in jar.mn ...

Comment 34

[[e-mail]]

Comment on attachment 255687 [details] [diff] [review]
Patch for the "smaller stuff" in Mail/News

Apart from the comments I've already made I liked the rest of the changes.

Comment 35

[Manuel Reimer]

Attachment: The file "communicatorBindings.xml" which goes to "mozilla/themes/classic/messenger/communicator"

Attaching communicatorBindings.xml as "cvs diff" doesn't like the -N switch...

Comment 36

[Manuel Reimer]

Attachment: The updated patch for the "smaller stuff"

Comment 37

[Manuel Reimer]

Attachment: ... and the new icon archive

Comment 38

[Manuel Reimer]

(In reply to comment #32)
> You don't seem to have included this file anywhere. (For testing I copied
> modern/global/globalBindings.xml in its entirety.)

"cvs diff -N" doesn't seem to work so I attached the file now...

> >+  skin/classic/messenger/icons/check.png                                (messenger/icons/check.png)
> >+  skin/classic/messenger/icons/dot.png                                  (messenger/icons/dot.png)
> I'm not too keen on these icons, they look too cartoony.

What should be changed in the icons? Is moving back to the old icons a solution?

> The background colour change is OK but only if you also change the text colour
> to a system colour. Obviously you need to get rid of the comment ;-)

done

> I notice that there don't seem to be as many images as there used to be, has
> Modern always had fewer images?

They differ between open and closed threads, which isn't the case in modern.

> Some of your images had execute bits, which is incorrect.

fixed

Comment 39

[[e-mail]]

Comment on attachment 255779 [details]
The file "communicatorBindings.xml" which goes to "mozilla/themes/classic/messenger/communicator"

>          xmlns="http://www.mozilla.org/xbl"
>          xmlns:html="http://www.w3.org/1999/xhtml"
>          xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>          xmlns:xbl="http://www.mozilla.org/xbl">
Technically only the xmlns= and the xmlns:xul are necessary here.

Comment 40

[[e-mail]]

Comment on attachment 255780 [details] [diff] [review]
The updated patch for the "smaller stuff"

(In reply to comment #38)
>They differ between open and closed threads, which isn't the case in modern.
Thanks, that makes sense.

Comment 41

[Robert Kaiser]

(In reply to comment #32)
> >+  skin/classic/messenger/icons/check.png                                (messenger/icons/check.png)
> >+  skin/classic/messenger/icons/dot.png                                  (messenger/icons/dot.png)
> I'm not too keen on these icons, they look too cartoony.

They look OK to me in use - but could we make the thread pane use the same dot icon for consistency?

Comment 42

[[e-mail]]

Comment on attachment 255781 [details]
... and the new icon archive

OK, so we remove the readcol-read.png image and replaces its references with dot.png and we're all set!

Comment 43

[Matthias Bockelkamp]

Attachment: Screenshot with new column headers

I just downloaded and tested SM with the new icons. I found one issue, that in fact existed before, but this might be a good time to fix it. When compared, not all coloumn headers have the same width, with would look a bit nicer. For better visibility i copied them below each other in the screeshot. Also i think the icons in the headers might be moved up 1 pixel, so they don't touch the border. And, but perhaps that's only my impression, the attachment symbol looks antialiased, but the flag not.

I once submitted a patch for this issue using the old icons. If desired, i'll try to update that patch for the new icons. But i won't be able to begin working on that before next thursday.

Comment 44

[Robert Kaiser]

(In reply to comment #43)
> I just downloaded and tested SM with the new icons. I found one issue, that in
> fact existed before, but this might be a good time to fix it.

Please file a separate bug for that. This one gets too overloaded, the icon set change alone is enough for this one...

Comment 45

[Manuel Reimer]

Attachment: Replaced "readcol-read.png" with "dot.png"

Removed readcol-read.png from CSS and jar.mn and replaced with dot.png

Now the file "readcol-read.png" of the tar.bz2-file doesn't have to be checked into CVS!

Comment 46

[Manuel Reimer]

... sorry for the spam. Didn't know that the new stuff is already in CVS...

Comment 47

[Andy Boze]

I noticed that new mail icons appeared in the 20070222 trunk nightly build. Starting with that build, the "folder with new message" icon wasn't displaying. As soon as I click the folder, the "closed folder" icon shows. Perhaps this has something to do with chrome://messenger/skin/folderPane.css rule treechildren::-moz-tree-imagefolderNameColnewMessages-true with {list-style-image: url(chrome://messenger/skin/icons/folder-new.gif);} . Image file name should be chrome://messenger/skin/icons/folder-new.png ?

Comment 48

[Eyal Rozenberg]

I'm sorry to criticize the artistic efforts of others, but I just downloaded a nightly and am using it with the new mail icons (on Win XP with the 'Windows Classic' theme for XP's UI). I must say I don't like them:

- They don't seem to me less 'dated' than the old ones.
- The grayish-brown tint is somehow sickly-looking; it doesn't go well in my opinion with the gray of the toolbars, menubars and pane separators.
- The unread 'dot' is now a star, and being a very small star, it sort of draws your attention to what seems to the eye like a black scribble over a green disc.

Actually, I didn't like the browser icon change either, because it looks like an invasion of the classic theme by 'modern' theme elements. What I had hoped to be seeing is more of an 'update' on the classic theme - taking the same icon styles, but using gradients instead of dithering, smoothing the edges, increasing the resolution, perhaps slight color variations but no more than that.

Just my $0.02 .

Comment 49

[Robert Kaiser]

Eyal:
OK, so you were wrong in your belief of what this will be. Correcting the summary to state what this effort is really about. "Classic" icons are basically dead on trunk, they get replaced with a contemporary icon set - which is to some extent based on Modern, but completely reworked, with stronger, more distinct coloring and more distinct icon shapes in the browser.

And please take discussion of design decision into the mozilla.dev.apps.seamonkey newsgroup, this bug report is getting too overloaded with misc comments.

Comment 50

[Olivier Vit (just a reporter)]

When using filters to move incoming emails to subfolders, these subfolders should have a folder icon with a green arrow indicating new messages in that subfolders.
Since the change in the icon set, these subfolders don't have any icon at all, until they get activated (then the green arrow is supposed to disappear):  the regular folder icon appears.

Would this mean that some icon for this specific case is missing ?
(the problem doesn't occur on the Inbox for example, but this is not a subfolder)

Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.9a3pre) Gecko/20070302 SeaMonkey/1.5a

Comment 51

[Olivier Vit (just a reporter)]

sorry for the spam, this is actually bug 371560

Comment 52

[Manuel Reimer]

Attachment: First patch to add the browser icons

First try for a patch, to get the new icons into browser.

I had to remove some stuff from global/ and move this stuff into navigator/

As soon as we add toolkit-global to the theme, we may have to modify those changes again.

Comment 53

[Manuel Reimer]

Attachment: ... and the icons

Comment 54

[[e-mail]]

Comment on attachment 257511 [details] [diff] [review]
First patch to add the browser icons

>-  skin/classic/communicator/bookmarks/home.gif                          (communicator/bookmarks/home.gif)
>+  skin/classic/communicator/bookmarks/home.png                   (communicator/bookmarks/home.png)
Wrong indentation here (communicator/bookmarks/home.png)

>+/* Tab icons */
Please can you hold off on the tab icons until we get the tabbrowser sorted?

Comment 55

[Robert Kaiser]

(In reply to comment #52)
> I had to remove some stuff from global/ and move this stuff into navigator/

Please also look into bug 350221 for that, as this move is done there as well. We probably should go right away and put the new icons in the new place instead of moving the old icons in the other bug.

> As soon as we add toolkit-global to the theme, we may have to modify those
> changes again.

I don't think so, we should continue to use our own files there.

Comment 56

[[e-mail]]

Comment on attachment 257511 [details] [diff] [review]
First patch to add the browser icons

>+  list-style-image: url("chrome://communicator/skin/bookmarks/home.png");
>+  -moz-image-region: rect(0 19px 19px 0);
These regions look wrong... shouldn't the size be 20px?
(Either that or delete the unused pixels from the image?)

Comment 57

[Manuel Reimer]

(In reply to comment #56)
> (From update of attachment 257511 [details] [diff] [review])
> >+  list-style-image: url("chrome://communicator/skin/bookmarks/home.png");
> >+  -moz-image-region: rect(0 19px 19px 0);
> These regions look wrong... shouldn't the size be 20px?
> (Either that or delete the unused pixels from the image?)

No, the regions should be all right.

Pixels are zero-based and so 0 == pixel 1 and 19 == pixel 20

Have a look at:

http://lxr.mozilla.org/mozilla/source/themes/classic/navigator/navigator.css#85

Here, I have a size of 30x30 for the icons.

Comment 58

[Eyal Rozenberg]

(In reply to comment #49)
> classic icons ...get replaced with a contemporary icon set - which
> is to some extent based on Modern, but completely reworked, with stronger, 
> more distinct coloring and more distinct icon shapes in the browser.

Ok, basing my consideration on this stated intention, I believe that, as far as the mailnews icons are concerned:

- icons are less strong rather than stronger
- coloring is less rather than more distinct
- icon shapes are less distinct rather than more distinct

I find it significantly more difficult to tell apart icons now than I had with the old classic icons. The most horrid example is the message icon vs message-with-attachment icon, whose single difference is a 1-px-thin black line sticking out of the back of the envelope - which I never fail to miss without focusing for long periods of time on the column of icons. But discerning which icon you're looking at is also a problem with the folder and mail account icons.

Now, I know, this problem is inherited from the Modern theme, which has basically the same issues - but that was the reason I never used Modern... I won't argue over the 'artistic' choice, but at least make the smaller icons legible enough. Even Thunderbird 2's washed-out-color folder icons are better than this.

Comment 59

[Robert Kaiser]

(In reply to comment #58)
> but at least make the smaller icons legible enough.

Could you please file separate, new bugs for such issues with icons that are already checked in? Discussing issues of single icons in a bug report that already has more almost 60 comments becomes hard to follow fast.
Let's leave this bug report here for getting the rest of the icon set in and work on detailed issues with single icons or (small!) subsets of icons in new followup bugs. Thanks.

Comment 60

[[e-mail]]

(In reply to comment #57)
>Pixels are zero-based and so 0 == pixel 1 and 19 == pixel 20
Ah, but rectangles exclude their bottom and right borders.

>Have a look at:
>http://lxr.mozilla.org/mozilla/source/themes/classic/navigator/navigator.css#85
>Here, I have a size of 30x30 for the icons.
They're not. DOM Inspector verifies that they're 29x29.
Unfortunately Modern has always had this bug. Sigh.

Comment 61

[Manuel Reimer]

(In reply to comment #60)
> Ah, but rectangles exclude their bottom and right borders.

... and now *all* icons need fixing ...

*AAARGH*...

Is there any written documentation which says that the bottom/right borders are excluded?

Comment 62

[Manuel Reimer]

(In reply to comment #59)
> (In reply to comment #58)
> > but at least make the smaller icons legible enough.
> 
> Could you please file separate, new bugs for such issues with icons that are
> already checked in?

... and perhaps he also wants to add an example on how he would like to have the icons. It's easy to criticise something but it's surely more difficult to make it better! I'm doing all this as good as I'm able to. In my spare time!! And I'm no professional artist! And currently my primary target is to get this important task, of adding the new icons, finished as soon as possible, so we are able to add this theme to SeaMonkey 1.5.

So please go to http://www.gimp.org/ and get GIMP. Then imporove the icons and *then* file the bug and attach the images there. Please attach images as XCF files if you used multiple layers, so this important information isn't lost! Only GIMP and its format XCF! Not Photoshop or something like that!

Comment 63

[Robert Kaiser]

(In reply to comment #61)
> (In reply to comment #60)
> > Ah, but rectangles exclude their bottom and right borders.
> 
> ... and now *all* icons need fixing ...

BTW, I believe it would be good to even split off fixing already checked-in parts of this into a separate followup bug.
This bug report here is simply getting too long, and we should restrict it to getting all the replacement icons into the tree. All fixups to that (either further improving icons, or fixing other problem with the reviewed and checked-in patches) warrant separate, one-issue-per-bug followups.

Comment 64

[Eyal Rozenberg]

Attachment: illustration of icon illegibility

(In reply to comment #62)

I don't want to get acrimonious, my criticism is certainly not personal and having done a (very small) bit of icon design myself, I know very well this is not at all an easy task.

Still, Manuel, I really think you shouldn't dismiss criticism the way you do. Most of us do this on our spare time (I know I do). SeaMonkey can have a relaxed release cycle anyway, and very few people use it for the beauty of the icons, so I don't see why we should be hasty in accepting a new icon theme; this is doubly the case considering the fact that the new theme is very similar to the Modern theme, which people can use instead of the old Classic if they want to. I would even go as far as saying that a new theme can wait until someone who is a (semi-)professional artist can redo it, the way the new Seamonkey logo was chosen.

Now about this attachment. You'll see on the top the new checked-in icons for the classic theme. The differences between the attachment and no-attachment are quite minimal. Below them you see the old message-mail.gif and message-mail-attach.gif  ; since these are high-contrast icons, the difference stands out more clearly. If we wish to use the envelope shape in the first line, the _least_ we can do is make the paperclip more pronounced, like I've done in the third line.

I want to emphasize that this is an illustration of the general issue, it's not just one icon that's problematic. The whole icon set is quite illegible. The way things stand, I don't think the new icon set is ready to replace the old Classic theme icon set. That's why I'm not opening a new bug.

Comment 65

[Robert Kaiser]

Eyal, you're still cluttering this bug with stuff that belongs IN A SEPARATE BUG. I urge Manuel NOT TO REACT to this here. This BUG IS TOO LONG already.
PLEASE file followups for ANY issues caused by checkins for this bug. Thanks.

Comment 66

[Eyal Rozenberg]

(In reply to comment #65)

Look, Robert, I didn't want to reply to your claims on the bug, but since after e-mailing you're still making meta-comments about the bug on bugzilla, I'll have to reply here.

I've already told you this is not a followup issue or a side-issue, I'm claiming the checkins of (at least) the mail icons should be backed out since these icons are significantly flawed and are not a suitable replacement for the old Classic theme. You can _disagree_ with my opinion, but it is not for you to _forbid_ me from expressing it. 

Also, the demand not to comment on a bug because it is 'too long' is quite unacceptable. This bug is not long. It has less than 6500 words, and as many of them are yours as are mine, if not more. And even if it had been 65000 due to people's comments - so what? Nobody's making you copy it on paper. In long bugs you read the latest comments rather than the whole page.

Comment 67

[Robert Kaiser]

Eyal, please read this before further not following requests of us here: http://home.kairo.at/blog/2007-03/effective_work_with_a_bug_reporting_syst

This bug is and should stay focused on getting the new icons into place - NOT in perfecting them. Small, specific followup bugs are faster for improving the icons than cluttering this bug and distracting people from getting the remaining ugly, old icons replaced in the first place.
And, BTW, 1) Bugzilla is not for opinions but for fixes and 2) noone's forbidding you pointing to a problem caused here, I'm just trying to help you getting it fixed correctly by pointing out how developers can deal with it more easily.

Please send ANY replies to that comment to the m.d.a.seamonkey newsgroup, my blog, or me personally (wherever it fits best). Buzgilla is not where this discussion should take place, as it distracts people like Manuel from actually working on e.g. improved icons.

Comment 68

[Manuel Reimer]

Anything about improving the new Mail/News icons should go to Bug 373452 from now on! Thanks!

Comment 69

[Eyal Rozenberg]

Look, don't be ridiculous. I'm claiming the icon set is seriously flawed, and you're trying to get me to drop this claim by referring me to a 'fine-tuning' bug.  And Robert is putting on a sock puppet and asking me to take its advice about the situation. I could also write a post on my blog repeating my opinions, and ask Robert to read it so as to learn about 'effective' work with bug reporting systems. How would you feel if I did that?... 

I'm not upset because you think the icons are good enough, I'm just annoyed that you can't accept the possibility that someone thinks differently and suggests a back-out or an overhaul.

Comment 70

[[e-mail]]

(In reply to comment #61)
>(In reply to comment #60)
>  h, but rectangles exclude their bottom and right borders.
>... and now *all* icons need fixing ...
Well, Modern managed with this mistake for years, so I'm retracting my denial.

Comment 71

[[e-mail]]

Attachment: Approved parts of attachment 257511

As well as the previously mentioned changes, the location bar change was wrong.

Comment 72

[Robert Kaiser]

(In reply to comment #71)
> Created an attachment (id=258213)
> Approved parts of attachment 257511 [details] [diff] [review]

So, does this part need anything else than checkin now?

Comment 73

[[e-mail]]

Comment on attachment 258213 [details] [diff] [review]
Approved parts of attachment 257511 [details] [diff] [review]

>-  skin/classic/communicator/bookmarks/bookmark-item.gif                 (communicator/bookmarks/bookmark-item.gif)
Tabbed browser was using this image. Oops ;-)

Comment 74

[Jürgen 'JiHa' Harter]

The autoscroll icon is not used correctly in the browser window. It is just a white square.

In MailNews first you get the same square. But when clicking a second time without moving the mouse in between it becomes a round correct icon (the new transparent one for suiterunner).

Build identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/2007060301 Mnenhy/0.7.5.0 SeaMonkey/2.0a1pre 
self compiled from cvs

Comment 75

[John A.]

Any chance of getting this bug switched to replacing Modern instead of Classic? I happen to like Classic.

Comment 76

[Tony Mechelynck [:tonymec]]

(In reply to comment #75)
> Any chance of getting this bug switched to replacing Modern instead of Classic?
> I happen to like Classic.

Hm. Modern already has an icon set with window icons (at top left and in the taskbar) based on the SeaMonkey logo. I "happen to like" Modern's other icons, including the window icons at left end of the status bar.

Comment 77

[Stefan [:stefanh]]

Attachment: Fix addrbook.gif

I think the addrbook.gif ison needs some love. We could at least add some transparency to the existing icon, that's better than nothing. Currently, it looks quite bad if the icon is against a non-white background.

I made a png of the existing one and manually made the white pixels in the outer areas transparent.

Comment 78

[Stefan [:stefanh]]

Attachment: addrbook.png

Here's the icon as it looks with the patch applied.

Comment 79

[Robert Kaiser]

Sorry, you're on the wrong bug with this. Making the same crappy icon transparent doesn't qualify as "new icon set", like the summary here says. It's a good step in the right direction, albeit a small one, but it's on the wrong bug :P

Comment 80

[Stefan [:stefanh]]

(In reply to comment #79)
> Sorry, you're on the wrong bug with this. Making the same crappy icon
> transparent doesn't qualify as "new icon set", like the summary here says. It's
> a good step in the right direction, albeit a small one, but it's on the wrong
> bug :P

I didn't bother to file a new bug, but sure, I guess I could do that

Comment 81

[Stefan [:stefanh]]

Filed bug 510786.

Comment 82

[Robert Kaiser]

Here's the list of icons that still need work here:

communicator/bookmarks/bookmark-folder-button.gif
communicator/bookmarks/location-act.gif
communicator/bookmarks/location-dis.gif
communicator/bookmarks/location.gif
communicator/bookmarks/location-hov.gif
communicator/directory/file.gif
communicator/directory/folder-clsd.gif
communicator/directory/folder-open.gif
communicator/profile/migrate.gif
communicator/profile/profileicon-large.gif
communicator/search/category.gif
communicator/search/result.gif
communicator/sidebar/sbtab-twisty.gif
communicator/sidebar/sbtab-twisty-open.gif
communicator/taskbar/addressbook-16.gif
communicator/taskbar/addressbook.gif
communicator/taskbar/addressbook-hov.gif
communicator/taskbar/composer-16.gif
communicator/taskbar/composer.gif
communicator/taskbar/composer-hov.gif
communicator/taskbar/mail-16.gif
communicator/taskbar/mail.gif
communicator/taskbar/mail-hov.gif
communicator/taskbar/mailnew.gif
communicator/taskbar/mailnew-hov.gif
communicator/taskbar/navigator-16.gif
communicator/taskbar/navigator.gif
communicator/taskbar/navigator-hov.gif
communicator/toolbar/tbgrip-arrow-clps.gif
communicator/toolbar/tbgrip-arrow.gif
communicator/toolbar/tbgrip-texture.gif
editor/icons/btn2.gif
editor/icons/editmode-html.gif
editor/icons/editmode-normal.gif
editor/icons/editmode-preview.gif
editor/icons/editmode-tags.gif
editor/icons/img-align-bottom.gif
editor/icons/img-align-left.gif
editor/icons/img-align-middle.gif
editor/icons/img-align-right.gif
editor/icons/img-align-top.gif
editor/icons/progress-busy.gif
editor/icons/progress-done.gif
editor/icons/progress-failed.gif
messenger/addressbook/icons/abcard.gif
messenger/addressbook/icons/ablist.gif
messenger/addressbook/icons/addrbook.gif
messenger/addressbook/icons/remote-addrbook-error.gif
messenger/addressbook/icons/remote-addrbook.gif
messenger/addressbook/icons/secure-remote-addrbook.gif
messenger/smime/icons/hdrCryptoNotOk.gif
messenger/smime/icons/hdrCryptoOk.gif
messenger/smime/icons/hdrSignNotOk.gif
messenger/smime/icons/hdrSignOk.gif
messenger/smime/icons/hdrSignUnknown.gif
messenger/smime/icons/sbCryptoNotOk.gif
messenger/smime/icons/sbCryptoOk.gif
messenger/smime/icons/sbSignNotOk.gif
messenger/smime/icons/sbSignOk.gif
messenger/smime/icons/sbSignUnknown.gif
navigator/btn1/first-disabled.gif
navigator/btn1/first.gif
navigator/btn1/first-hover.gif
navigator/btn1/last-disabled.gif
navigator/btn1/last.gif
navigator/btn1/last-hover.gif
navigator/btn1/next-disabled.gif
navigator/btn1/next.gif
navigator/btn1/next-hover.gif
navigator/btn1/previous-disabled.gif
navigator/btn1/previous.gif
navigator/btn1/previous-hover.gif
navigator/btn1/top-disabled.gif
navigator/btn1/top.gif
navigator/btn1/top-hover.gif
navigator/btn1/up-disabled.gif
navigator/btn1/up.gif
navigator/btn1/up-hover.gif
navigator/icons/popup-blocked.png
navigator/icons/tab-drag-indicator.gif
navigator/icons/tab-new.gif

Comment 83

[Robert Kaiser]

Attachment: task icons, v1

Here's a patch for the task icons, both in the component bar and the tools menu. The approach is similar to what we have in modern now, but with all component icons being the same size, and with the icons taken from what we have in main toolbar icons so far - the globe for browser is from Composer's publish, the mail envelope is from "next" in mailnews, the edit icon is mixed from the "compose" icon in mailnews and the browser globe, the addressbook icon is mixed from the "new list" and "properties" icons of abook.

Comment 84

[Stefan [:stefanh]]

I think it's hard to preserve details when you're re-using toolbar icons - especially the browser icon seems to suffer a bit from the resize.

Comment 85

[Robert Kaiser]

(In reply to comment #84)
> I think it's hard to preserve details when you're re-using toolbar icons -
> especially the browser icon seems to suffer a bit from the resize.

It's the best I can do, I worked 4 or 5 hours the even get the icons where they are, and it's surely better than what we have in now. If someone comes along who can update the icons to fit within the style of the toolbar icons _and_ be better in the smaller size, I'm all for taking them, but for the time being, I think we need to go with what we get together at all.

Comment 86

[[e-mail]]

(In reply to comment #83)
> Here's a patch for the task icons, both in the component bar and the tools
> menu. The approach is similar to what we have in modern now, but with all
> component icons being the same size,
Why? That looks awful :-(

> and with the icons taken from what we have in main toolbar icons so far -
And here was me thinking you'd just applied the Classic toolbar icon colouring to what we have in Modern.

Was the globe's hover effect copied from Composer's Publish icon?

Comment 87

[[e-mail]]

Comment on attachment 401677 [details] [diff] [review]
task icons, v1

> #mini-nav {
>-  list-style-image: url("chrome://communicator/skin/taskbar/navigator.gif");
>+  list-style-image: url("chrome://communicator/skin/taskbar/taskbar.png");
>+  -moz-image-region: rect(0 23px 13px 0);
Needs 3px removed from the left and 5px from the right.

> #mini-mail {
>-  list-style-image: url("chrome://communicator/skin/taskbar/mail.gif");
>+  list-style-image: url("chrome://communicator/skin/taskbar/taskbar.png");
>+  -moz-image-region: rect(0 46px 13px 23px);
Needs 2px removed from the right (includes "NewMail" version).

> #mini-addr {
>-  list-style-image: url("chrome://communicator/skin/taskbar/addressbook.gif");
>+  list-style-image: url("chrome://communicator/skin/taskbar/taskbar.png");
>+  -moz-image-region: rect(0 115px 13px 92px);
Needs 2px removed from the left and 4px removed from the right.

> #mini-comp {
>-  list-style-image: url("chrome://communicator/skin/taskbar/composer.gif");
>+  list-style-image: url("chrome://communicator/skin/taskbar/taskbar.png");
>+  -moz-image-region: rect(0 92px 13px 69px);
Needs 2px removed from the left and 5px removed from the right.

Comment 88

[[e-mail]]

You might be better off adding
.taskbutton > .toolbarbutton-icon {
  margin: 0px !important;
}

And then restoring 2px from the values above (so for instance the mail icon would not need its size changed at all in that case.)

Comment 89

[[e-mail]]

I made the mistake of looking at the raw image pixels (both the taskbar.png added here and the one we currently have in the Modern theme).

Both images have some regions with an alpha transparency of 98.4%, and a base colour of #FFD0D9; which is a rather fetching salmon shade ;-)

The Modern image has two areas which have the wrong colour and are transparent when they shouldn't be. I can fix these by copying from the GIFs.

The Classic image has "shadows" of coloured pixels with 100% transparency.

Comment 90

[Robert Kaiser]

Attachment: task icons, v2

OK, here's a new patch for the task icons. I left a right 2px empty room on the mailnews icons as else they would look very unbalanced when hovered. I should have removed the salmon though ;-)

Comment 91

[[e-mail]]

Attachment: taskbar.png

Whoops. Because the icons are a different order in the image than they are in the CSS, you adjusted the images incorrectly :-( Here is a corrected image. I also include a minor correction for some bogus transparency in Modern's image.

Comment 92

[[e-mail]]

Comment on attachment 402439 [details] [diff] [review]
task icons, v2

>+  -moz-image-region: rect(0 100px 13px 82px);
>+  -moz-image-region: rect(0 82px 13px 63px);
So if I've got my sums right both these 82px should really be 81.

Comment 93

[[e-mail]]

Comment on attachment 402439 [details] [diff] [review]
task icons, v2

When using 81px as above.

Comment 94

[Robert Kaiser]

81px is better with your image, that's true - with mine, 82px was better ;-)
Pushed with your image and 81px as http://hg.mozilla.org/comm-central/rev/a064d91f0544

The modern image wasn't recognized by ma hg as being changed, but I'd rather put that discussion into a bug that deals with modern - this bug is already 94 comments long...

Comment 95

[Robert Kaiser]

The bulk of the work in here has been done, so I'm resolving this bug. I filed bug 523274 to track the remaining icons from the list in comment #82 and this report in here is just getting too long and intertwined to be still useful for further work. And after all, a huge part of the icons has been replaced for 2.0 and made it look much better now, thanks everyone for the work! :)

Please put all further comments into the followups.

Comment 96

[[e-mail]]

Comment on attachment 258213 [details] [diff] [review]
Approved parts of attachment 257511 [details] [diff] [review]

>Index: themes/classic/jar.mn
...
>-  skin/classic/communicator/icons/offline.gif                           (communicator/icons/offline.gif)
>-  skin/classic/communicator/icons/online.gif                            (communicator/icons/online.gif)
>+  skin/classic/communicator/icons/offline.png                           (communicator/icons/offline.png)
>+  skin/classic/communicator/icons/online.png                            (communicator/icons/online.png)
[suite/themes/classic/*/messenger/accountManager.css is still looking for offline.gif]