Proposal: deprecate silk icons theme

Development
1

Hi everyone,

this topic is a fork of the proposal originally made by @CharpentierLucas on Proposal: add icons to the XWiki Icon Set.
As part of this proposal Lucas said:

Unfortunately, silk was created in 2005 and not updated for a long time, so there’s some huge holes in its icon theme cover. When checking out a list of the icons in silk, I could not find anything closer to those we would expect for those actions.

So I proposed that we deprecate the Silk theme in future version of XWiki:

Regarding silk shouldn’t we just start deprecating it? As you said it’s starting to get old, and I think we’re all more relying on FA those days than on silk. IMO it would make sense to deprecate it and stop bundling it in the future, maybe when we move to 16.x or 17.x.

@vmassol then reacted with:

Sounds good generally speaking, but could you explain precisely what you mean by deprecating? I think we need to make sure that contrib extensions (or custom code) continue to work if they use Silk. Does deprecating mean that when using Silk in future XWiki versions, the UI would get broken? IMO, one simple solution that could be acceptable would be that each icon set could provide a default icon when no mapping exists for a specific name. That would allow Silk to continue “working” (even if it would be with icons not nice) in the future, at a low cost.

So to be clear this proposal is about:

  1. documenting that Silk should not be used anymore in new code, in favor of FA theme: this documentation should go in dev best practices, and in the documentation regarding the extension theme / icon extension. That’s what I meant first by “deprecating”
  2. open a vote for stop bundling silk in a future version of XWiki and move it to contrib: as part of this change we might decide to introduce a default icon as fallback to avoid problems on older extensions

wdyt?

2

You mean when an entire explicitly referenced theme does not exist ?

Is that really the right recommendation ? Should it be to not reference a specific theme and use generic icons ?

3

  1. +1 for the idea. But like @tmortagne I think that we should not recommend another theme, to avoid making it difficult to move on from this theme when eventually needed. I’d rather have the best practice reference the Icon Theme Application :slight_smile:

  2. +1

Thanks!
Lucas

4

I thought that was what Vincent was proposed but I misread and it actually doesn’t really make sense, so I just withdraw this proposal. No fallback.

+1

5

By new code, I guess you mean XS at least and also Contrib extensions. Not using it is one thing, but we continue to map new icons into the Silk icon set, right?

BTW this is already the case (not to use Silk) and we’ve already been migrating to FA over the years. I don’t think we have used Silk anywhere else since we introduced FA, as, when we introduced FA we effectively deprecated Silk back then. So for me we don’t need a proposal for this (but it’s ok to reiterate it, no harm done!).

Sounds good.

Thanks

PS: I find it interesting to do the mapping for other icon sets when we want to introduce new icons for FA. It allows us to make sure the new icons are generic enough across icon sets.

6

What I meant is that if there’s no mapping defined for a specific icon set then use a default icon.

7

+1

+1

8

We’re currently using silk for resolving icon: image references. If we want to replace silk by Font Awesome, we need to first add a version of Font Awesome to XWiki that actually provides images. This is possible with SVGs but currently not available.

Further, silk is the default, fallback icon theme that is bundled in the WAR while Font Awesome is only included as a XAR in the distribution. So in an empty wiki, Font Awesome is currently simply not available but we also need icons in an empty wiki.

So I suggest before we deprecate Silk, we first add a proper replacement. This means an icon theme that provides images and is available in an empty wiki.

In general, +1, but there are many steps before we can remove silk. Also, note that we have many explicit references to silk in the code, just search for “silk”.

Also, regarding best practices, I think it is already a best practice that no icon theme should be explicitly referenced but instead the current icon theme should be used. If this is not documented, it should be added.

1 Like
9

We need to think this through. How / where is Silk currently explicitly used?

  • As an icon theme:
    • As the configured icon theme in the administration. Low probability, but possible.
    • Explicitly passed to $services.icon.* methods (as the icon set). Low probability, but possible.
    • Explicitly passed to {{displayIcon iconSet="silk" /}}. Low probability, but possible.
    • As the default icon theme in an empty wiki (as Michael pointed out)
  • As an icon resource reference (for the image syntax, as Michael pointed out): image:icon:someIconNameSpecificToSilk (e.g. image:icon:cog)
  • As an XWiki WAR resource:
    {{velocity}}
image:path:$xwiki.getSkinFile('icons/silk/cog.png')
{{/velocity}}

    background-image: url("$xwiki.getSkinFile('icons/silk/cross.png')");

Deprecating the Silk icon theme should be relatively easy, because we’re supposed to use the icon names from the XWiki icon set. The biggest problem is with the places where Silk icon names are used (i.e. for icon: and getSkinFile()). This will cause a lot of breakage, especially for old extensions. It will also break upgrades from old XWiki versions (released before the icon themes were introduced). Basically the Silk icon names can be seen as an API… In order to avoid breaking this API we need to:

  • provide another image (file) based icon set (as Michael suggested)
  • and also provide a mapping between the Silk icon names and the new image icon set, so that both icon: and getSkinFile() return semantically similar image icons (from the new icon set).

Thanks,
Marius

10

Isn’t the image:icon: syntax using the current icon theme? Does it mean it’s a syntax only supporting the Silk icon set? If so, that’s bad and we need to do something about this syntax (fixing it in xwiki/2.2, etc).

The doc at https://www.xwiki.org/xwiki/bin/view/Documentation/UserGuide/Features/XWikiSyntax/?syntax=2.1&section=Images does say:

  • icon: A required string identifying the image reference as an icon from the XWiki Icon Set.

And the link is to the Silk icon set…

I’m just surprised we didn’t generalize the syntax to the current icon theme (without breaking backward compat which probably essentially means deprecating the syntax and introducing a new one, or introducing a xwiki.properties config option to specify different modes, or something else).

Thx

11

No, and I’m not sure if it can work that easily with the current icon theme because the icon:* part is just a resource reference that the image syntax is using to generate an image element. I think ATM it can work only with image-based icon sets. Adding support for font-based icon sets probably requires some changes in the rendering (e.g. to render a span and not an image, and to be able to parse the span as an image block). Or, alternatively, as Michael suggested, to generate an SVG image when a font-icon set is used. But in any case, that won’t solve the fact that we currently use icon: with Silk icon names.

ATM yes, that’s my understanding.

Because font-icon sets usually generate a SPAN and not an actual image element. And probably also because icon: was introduced before the icon themes, so we were already using it with Silk icon names.

Thanks,
Marius

12

The problem has two sides:

  1. This is an image syntax that is defined by xwiki-rendering. We just resolve the reference to an actual image in xwiki-platform. As this is an image syntax, this would require quite some hacks to support icon fonts. Afaik SVG is only a workaround if the icon theme supports SVG, we can’t simply produce SVG images for an icon font.
  2. CKEditor expects that when you use an image syntax you get an image that has, e.g., a width and height that can be modified or an alternative text. All of this doesn’t work with an icon font.

As a solution for this, I introduced the icon macro some time ago. It allows displaying an icon from the current or a chosen icon set. My suggestion back then was to deprecate this icon reference syntax, i.e., to still support it but to not offer the option in the UI when adding or editing an image that is not already an icon.

13

I’m not sure I understand the difference between the icon image syntax and the displayIcon macro. They both generate XDOM/listener events. When the XWiki HTML Renderer receives an ImageBlock (or onImage() event), why couldn’t it generate a <span class="fa fa-*"></span> content? (we could introduce a, IconResourceReference).

Note that we would also need to have the XWiki HTML parser recognize <span class="fa fa-*"></span> as an icon and generate the XWiki syntax for it.

Is your problem just with the fact that the syntax starts with image:? If that’s the case, I don’t think it’s a problem since from the user POV, it’s an “image” being displayed (not some text).

There’s ofc the issue with backward compatibility that we would need to fix, but first I wanted to understand the problem.

From CK’s POV it wouldn’t get an IMG tag but a SPAN one with a class that it would need to understand.

Thanks

14

The XWiki HTML renderer doesn’t know anything about icon themes as it lives in xwiki-rendering but icon themes are in xwiki-platform. It just outputs an image with the URL that it received, see, e.g., this code where the icon reference is resolved to a URL. You’re right that parsing is a similar problem, but it could be solved by ignoring the HTML and relying solely on the metadata in the comments when there is no image. Note that the produced HTML is specific to the icon theme, icon themes can produce arbitrary HTML.

We already have icon resource references. However, our APIs only allow resolving this icon reference to an image URL and not to arbitrary HTML.

No, you cannot assume that an icon theme produces a span with a class. It could also be, e.g., an SVG tag as here. Basically, what you’re suggesting here is that CKEditor would need to understand all possible icon themes and adapt its image dialog accordingly to show the options that are possible with this icon theme (e.g., you cannot set an alternative text, width or height with most icon themes).

Also, how would you handle images with icon references that don’t resolve to an image URL in other syntax like LaTeX?

15

ok that makes sense. But rendering is not only in xwiki-rendering. We extend it in xwiki-platform for all rendering features that require the concept of a wiki (color themes is a case like this).

16

+1 to drop Silk in order to be able to add more icons to the XWiki Icon set.

its lack of maintenance hinders development of the XWiki product - see, as an example, the like UI improvements discussions, Loading... where the empty heart was missing.

I think the risk that contributions are breaking because silk is not present is rather low-ish: we have icon sets in XWiki since 7.x (or 6.x?) and if an extension would hardcode silk it would mean that either:

  • its code was was written before version 6 (almost 10 years ago?) and never updated. In this case I think there may be other problems with it
  • its code was updated but still hardcoding silk, in which case it would not work right on the latest XWiki
  • if the icon dissapears it may not be such a big problem as icon is supposed to be mostly decorative.

Now, about handling icon: references from the syntax, it would indeed make sense to replace that with emojis or with actual icons from the XWiki icon set (which means that we may not actually generate an actual image but some other form of HTML). For the icons there is a macro (?), so I am not sure why we need a syntax, while for fast-typing decorations in the content, modern standards are to use emojis for that. I’m not sure how relevant this syntax is today…

Thanks,
Anca

17

FTR, we’re still hard-coding Silk icons in XWiki Platform in many places, Code search results · GitHub , so I wouldn’t be surprised if supported extensions are doing the same. Now, I agree that a broken icon shouldn’t prevent you from using the extension normally, so it’s not a blocker issue.