515091 - main-toolbar.png and main-toolbar-small.png inconsistent in themes qute, pinstripe, and gnomestrip

Status: RESOLVED WORKSFORME

(Thunderbird :: Theme, enhancement)

Description

[Joachim Herb]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.4pre) Gecko/20090905 Shredder/3.0b4pre

The files main-toolbar.png and main-toolbar-small.png contain different numbers of icons for the three different themes. If you want to use the file in an extension you cannot reference a certain icon within a css file.
The qute version contains:
get mail
compose
addressbook
reply
reply all
forward
delete
junk
print
stop
archive
last(?)
first(?)
mark
tag
forward(?)
backward(?)
delete all(?)

Altogether 18 icons in three versions.

In the pinstrip theme an extra reply list icon is there. Then an extra icon between archive and first(?), perhaps flag(?). So altogether 19 icons at different positions in three version.

Finally the gnomestrip theme contains only 14 icons in two versions.

If an add-on should use the default theme icons depending on the operating system there is no obvious work-around: The add-on cannot bring its own icons as they should differ for the different operating systems. It also cannot reference e.g. (junk in qute, icon 8)
chrome://messenger/skin/icons/mail-toolbar-small.png 
-moz-image-region: rect(0px 128px 16px 112px);
as in pinstripe this would be the icon for "delete" and in gnomestrip also(?) the icon for "delete"(?). When gnomestrip is the default theme the icon may even be empty as there are only two versions instead of the three used by the other themes.

Reproducible: Always

Comment 1

[Joachim Herb]

Just as quick reference:
http://mxr.mozilla.org/comm-central/source/mail/themes/qute/mail/icons/mail-toolbar.png
http://mxr.mozilla.org/comm-central/source/mail/themes/pinstripe/mail/icons/mail-toolbar.png
http://mxr.mozilla.org/comm-central/source/mail/themes/gnomestripe/mail/icons/mail-toolbar.png

Do the icons really have different sizes. Then everything get even more complicated.

Comment 2

[rsx11m]

It would certainly be helpful to keep the themes as synchronized as possible to minimize development efforts. Another example is compose-toolbar.png where qute has a horizontal arrangement of the icons and gnomestripe a vertical one (e.g., apparent in bug 496375). Other examples can be found. It's somewhat confusing while during the development of the gnomestripe theme in bug 406742 such major differences to the qute theme were introduced.

An issue obviously making this difficult is the use of stock icons in the Linux theme, thus there would be "holes" in the icon image for those (and padding pixels if sizes are different, yes).

I'm not confirming this as I'm uncertain if an extension is supposed to use the icon images directly. It appears that every button has an associated CSS entry which could be utilized (disclaimer: having no clue how to program extensions myself). Even if the default themes are synchronized, when using a third-party theme, you cannot necessarily assume that they would obey such schemes.

Comment 3

[Andreas Nilsson (:andreasn)]

Regardless if icons are supposed to be used by extensions or not, I think it makes sense to clean these up a bit to make the code more consistent.

The sizes for gnomestripe and qute *should* be 16x16 and 24x24 (there exists a couple of places where some are 22x22, but we should fix those to be 24x24 instead). For some reason, pinstripe makes use of 24x24 and 32x32, but I'm going to look into making them 16x16 and 24x24 in Thunderbird 3.1 (or whatever the version is going to be called).

I won't have time to take this on right now, but if someone wants to rearrange some icons and create a patch, that would be great!

Comment 4

[Joachim Herb]

At the moment the icons are assigned to the individual buttons by their xul id, e.g.
toolbar[iconsize="small"] .toolbarbutton-1 {
  list-style-image: url("chrome://messenger/skin/icons/mail-toolbar-small.png");
}

toolbar[iconsize="small"] #button-getmsg {
  -moz-image-region: rect(0px 16px 16px 0px);
}

Would it make sense to assign the icons to classes which in turn are used for the buttons, something like this (.css of the theme):

.button-getmsg-class  {
  -moz-image-region: rect(0px 16px 16px 0px);
}

and in the xul file:
    <toolbarbutton id="button-getmsg"
                   type="menu-button"
                   class="toolbarbutton-1 button-getmsg-class"
                   ...

Then the class could be used in the add-on.

Comment 5

[Markus Stange [:mstange]]

This sounds like a much better approach to me!

Comment 6

[Joachim Herb]

I would suggest a two step approach:

1. Add classes to the css file which could be used by e.g. add-ons.
2. Remove the IDs from the css file and update the xul files using these buttons.

Is there a chance to get step 1 or even 2 into Thunderbird 3.0? This would be quite a motivation to supply a patch ;-)

Comment 7

[Mark Banner (:standard8)]

Confirming based on Andreas's comment 3. Although I can see this is problematic for themes, we wouldn't block shipping Thunderbird 3 based on it.

We would accept patches for it, though we have limited time in which to do that now - in approx 3-4 weeks time we'll be locking down quite a bit (although theme changes like this may still make it in).

Comment 8

[Magnus Melin [:mkmelin]]

Is extension reuse of icons really that usual?
If an extension want's an image, its certainly easy enough to just pack it within the extension. Or is making that match default platform what you're after? 

For themes i suspect it would often be broken anyway, which makes me wonder if reuse is a good idea to start with.

Comment 9

[Joachim Herb]

I would like to use the icons of some toolbar buttons for the newly introduced menu buttons in the header pane. There is a bug about adding a toolbar to this header pane. But as long as this has not happened referencing the icons would be a work around. (At the moment I add an invisible toolbar with all needed icons to "steal" there icon because you only can access buttons really present at the toolbar.) 

I think this would be useful also for other add-ons.

Comment 10

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

This is really annoying and I know it's a bit of extra work for extensions but here is the work-around to this issue.

Make per OS CSS files
http://hg.mozilla.org/users/clarkbw_gnome.org/ovidiu-header/file/56af22d4da53/src/chrome.manifest

Include your base CSS file as well as the OS specific ones
http://hg.mozilla.org/users/clarkbw_gnome.org/ovidiu-header/file/56af22d4da53/src/content/ovidiuheader.xul#l39

Then just match the chrome.manifest structure in your skin directory
http://hg.mozilla.org/users/clarkbw_gnome.org/ovidiu-header/file/56af22d4da53/src/skin

Comment 11

[Ludovic Hirlimann [:Usul]]

Mass move to the new theme component.