827092 - Don't hardcode the Chat icons.

Status: NEW

(Thunderbird :: Instant Messaging, defect)

Description

[Richard Marti (:Paenglab)]

Hardcode icons in JS, XUL etc. isn't good for 3rd party themes because they can't set their own icons with different paths and/or names.
Also for the OSX Retina HiDPI support it's impossible to set HiDPI icons.

This bug is to set all icons through CSS there we have then the possibility to set other icons.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: proposed fix

Florian, what do you think about this solution? For the 16px icons I already set the HiDPI icons. For the bigger protocol icons I need 64px and 96px icons. Do you have this icons or should I ask Andreas for them?

I've set for all icons a default- and a unknown icon. Is this okay like this? Maybe they are not needed everywhere (like in "Add Buddy", "Join Chat" "new Account" etc.) and I can remove some definitions.

Comment 2

[Florian Quèze [:florian]]

Comment on attachment 698412 [details] [diff] [review]
proposed fix

(In reply to Richard Marti [:Paenglab] from comment #1)

First, thanks for looking at this, it's not an easy problem!

> Florian, what do you think about this solution?

I understand that from a theme developer point of view it's important to have all the icons set from theme files so that they are easy to replace when switching to another theme, but from a chat protocol developer point of view, it's important to not have hard coded and duplicated lists of icon URLs, as more protocols can be added by add-ons.

Would it work for you if the unknown and default icons were set through CSS, and the protocol specific icons were still set by the JS code? Wasn't there a trick with a special suffix after the png extension to have high dpi icons automatically recognized? Would it work for protocol icons?

Would it be reasonable to assume that most themes don't want to change all the protocol-specific icons? Could the themes that really want to do it use something like this?
  .protoIcon[src="chrome://prpl-gtalk/skin/icon32.png"] {
    list-style-image: url("chrome://themename/skin/gtalk32.png") !important;
  }

> For the 16px icons I already
> set the HiDPI icons. For the bigger protocol icons I need 64px and 96px
> icons. Do you have this icons or should I ask Andreas for them?

I may have the SVG source files for some of the icons, and I may have some more sizes for a few of the other icons, but I'm afraid I don't have all the files you will need.

> I've set for all icons a default- and a unknown icon. Is this okay like
> this? Maybe they are not needed everywhere (like in "Add Buddy", "Join Chat"
> "new Account" etc.) and I can remove some definitions.

The "unknown" case shouldn't happen for the "Add Buddy" and "Join Chat" dialogs. The "default" case can happen everywhere (the "default" icon is used for a protocol added by an add-on that doesn't provide its own icons).

Comment 3

[Richard Marti (:Paenglab)]

(In reply to Florian Quèze [:florian] [:flo] from comment #2)
> Comment on attachment 698412 [details] [diff] [review]
> proposed fix
> 
> (In reply to Richard Marti [:Paenglab] from comment #1)
> 
> First, thanks for looking at this, it's not an easy problem!
> 
> > Florian, what do you think about this solution?
> 
> I understand that from a theme developer point of view it's important to
> have all the icons set from theme files so that they are easy to replace
> when switching to another theme, but from a chat protocol developer point of
> view, it's important to not have hard coded and duplicated lists of icon
> URLs, as more protocols can be added by add-ons.

The protocol developer can add his own stylesheet in his Add-on to add his protocol
icon.

> Would it work for you if the unknown and default icons were set through CSS,
> and the protocol specific icons were still set by the JS code? Wasn't there
> a trick with a special suffix after the png extension to have high dpi icons
> automatically recognized? Would it work for protocol icons?

I have no Retina Mac and can't test. But you can try to put to the standard icons in the protocol directories double sized icons with @2x added: 16x16 = icon.png, 32x32 = [e-mail]. For the icon32.png the 64x64 icon would be [e-mail]. If this works, great, but if not, then we need the CSS way.

When you try it, please report if it works or not.

> Would it be reasonable to assume that most themes don't want to change all
> the protocol-specific icons? Could the themes that really want to do it use
> something like this?
>   .protoIcon[src="chrome://prpl-gtalk/skin/icon32.png"] {
>     list-style-image: url("chrome://themename/skin/gtalk32.png") !important;
>   }

This wouldn't work because through src="" set icons have always precedence to the icons set by list-style-image, also with !important.

What if the stylesheet would be in content with the definitions and if a theme wants to override them, it could do it in the theme? Then the protocol icons are correctly set also when the theme hasn't set them. This would be the safer way to always show the icons.

> > For the 16px icons I already
> > set the HiDPI icons. For the bigger protocol icons I need 64px and 96px
> > icons. Do you have this icons or should I ask Andreas for them?
> 
> I may have the SVG source files for some of the icons, and I may have some
> more sizes for a few of the other icons, but I'm afraid I don't have all the
> files you will need.

It would be great if you could attach what you have.

Comment 4

[Florian Quèze [:florian]]

(In reply to Richard Marti [:Paenglab] from comment #3)

> The protocol developer can add his own stylesheet in his Add-on to add his
> protocol
> icon.

If the protocol add-on includes a css file containing the icon urls, then the UI code using these icons needs to include one CSS file per available protocol.

On the other hand, if the add-on includes its icons by using "style" directives in its chrome.manifest file, it needs to include one line in that file for each UI window (in both Instantbird and Thunderbird) that needs these icons. That's tedious, would cause the add-on to be obsolete each time a new window starts using protocol icons, and would make add-ons using protocol icons in new windows quite difficult to do.

> > Would it work for you if the unknown and default icons were set through CSS,
> > and the protocol specific icons were still set by the JS code? Wasn't there
> > a trick with a special suffix after the png extension to have high dpi icons
> > automatically recognized? Would it work for protocol icons?
> 
> I have no Retina Mac and can't test. But you can try to put to the standard
> icons in the protocol directories double sized icons with @2x added: 16x16 =
> icon.png, 32x32 = [e-mail]. For the icon32.png the 64x64 icon would be
> [e-mail]. If this works, great, but if not, then we need the CSS way.
> 
> When you try it, please report if it works or not.

Ok, I set the needinfo flag to ensure I won't forget this.

Comment 5

[Florian Quèze [:florian]]

Looks like this is how it is handled for search engine icons in Firefox: http://hg.mozilla.org/mozilla-central/rev/5e615c0e6b30#l8.12

This seems to be using the favicon service, I'm not sure the same solution can apply to chrome:// urls.

Comment 6

[Wayne Mery (:wsmwk)]

Richard, up for round 2?

https://support.mozilla.org/en-US/questions/1086284#answer-788214

Comment 7

[Richard Marti (:Paenglab)]

I have this still on focus but with low priority. I think best would be to use SVG to be better scalable.