Page MenuHomePhabricator

Implement ResourceLoader module to ship CSS for sets of SVG+PNG icons to the user
Closed, ResolvedPublic

Description

We want to implement ResourceLoader module to ship CSS for sets of SVG+PNG icons to the user. This has been in the works for a while now at https://gerrit.wikimedia.org/r/#/c/165922/ , I'm filing this task to have a place to dump assets and tests and such, and point people to.

Related Objects

Event Timeline

matmarex created this task.Dec 2 2014, 2:35 PM
matmarex raised the priority of this task from to Needs Triage.
matmarex updated the task description. (Show Details)
matmarex changed Security from none to None.

Trevor's rasterization test: https://github.com/trevorparscal/colorize (check out the rsvg branch too: https://github.com/trevorparscal/colorize/tree/rsvg).

Effect: the default behavior sucks. Using ImageMagick and rsvg, respectively, we get icons mangled in different funny ways:

ImageMagickrsvg

However, it can be easily worked around. ImageMagick just needs some command-line options, rsvg needs massaged input data.

ImageMagick, tweakedrsvg, tweaked

Problem: SvgHandler doesn't allow us to do either of these things. Needs some refactoring.

Change 165922 had a related patch set uploaded (by Bartosz Dziewoński):
ResourceLoaderImageModule for icons

https://gerrit.wikimedia.org/r/165922

Patch-For-Review

Bawolff added a subscriber: Bawolff.Dec 2 2014, 6:01 PM

When testing please keep in mind image magick quality varries considerably depending on if it was compiled against rsvg or not.

Jdforrester-WMF triaged this task as High priority.
Jdforrester-WMF moved this task from Backlog to Doing on the OOUI board.

Also, if the goal here is to have these images rasterized dynamically, I'm not sure if that's a good idea:

*Not all people have either image magick or rsvg installed
*I believe I've seen people who have image magick installed, with it compiled to use rsvg, but don't have the shared librsvg library installed, resulting in an error whenever image magick is fed an svg
*Hard to control for quality when different users will have different software, and different versions/compile time options of the software installed

Huh, so ImageMagick can act like a wrapper for rsvg? Curious, I didn't know that.

The rasterized images are only used by browsers that can't handle SVG, and there aren't that many of them anymore. Without dynamic rasterizatiom we'd have to take care of somewhere between one and ten additional copies for each image.

Jdforrester-WMF lowered the priority of this task from High to Normal.Dec 3 2014, 7:15 PM
Jdforrester-WMF moved this task from Unsorted to Doing… on the UI-Standardization board.

Hmm. If its just obscure browsers that need the png fallback, then its probably fine to assume that mediawiki can render them dynamically.

Hmm. If its just obscure browsers that need the png fallback, then its probably fine to assume that mediawiki can render them dynamically.

Well, IE7 isn't "obscure" but it's not widely used. :-) Do we have a list of the browsers that would need PNGs and a rough number for their usage?

ImageMagick's use of RSVG is cool - but the reason we were trying to get ImageMagick to output acceptable quality (using super sampling and sharpening) was to support the minimum requirements of MediaWiki for "rendering thumbnails", as stated on mediawiki.org. RSVG support is indeed a great thing to have both for performance and quality reasons.

Do we have a list of the browsers that would need PNGs and a rough number for their usage?

The ones that are not supported by http://pauginer.com/post/36614680636/invisible-gradient-technique – that post mentions IE<=9, Firefox<=3.5 and some old Android browsers. The full list of browsers that need PNG is the browsers that don't support http://caniuse.com/#feat=multibackgrounds or http://caniuse.com/#feat=css-gradients .

Do we have a list of the browsers that would need PNGs and a rough number for their usage?

The ones that are not supported by http://pauginer.com/post/36614680636/invisible-gradient-technique – that post mentions IE<=9, Firefox<=3.5 and some old Android browsers. The full list of browsers that need PNG is the browsers that don't support http://caniuse.com/#feat=multibackgrounds or http://caniuse.com/#feat=css-gradients .

So, this would be mostly IE8 and below users – maybe 3% of worldwide usage, and less by the time MediaWiki 1.25 ships. Theoretically we could modify the MW release tarballs to ship with the PNGs available if it's a problem, but I'm not convinced it's needed. Let's address that come the RCs if someone finds it problematic in the real world.

Change 178560 had a related patch set uploaded (by Bartosz Dziewoński):
Demo for ResourceLoaderImageModule

https://gerrit.wikimedia.org/r/178560

Patch-For-Review

Change 178560 abandoned by Bartosz Dziewoński:
Demo for ResourceLoaderImageModule

Reason:
Proof of concept, not intended to be merged.

https://gerrit.wikimedia.org/r/178560

Change 165922 merged by jenkins-bot:
ResourceLoaderImageModule for icons

https://gerrit.wikimedia.org/r/165922

So Idf6ff4eb8e is merged; can this be marked as Resolved? If so, are the "blocking" tasks not really blocking, or is there a subsequent implicit part of this task ("… and use it") which isn't yet fixed?

The "blocking" tasks are blocking releasing this as part of a new MediaWiki version. They make the module unusable by third-parties without replicating WMF configuration.

matmarex lowered the priority of this task from Normal to Lowest.Dec 12 2014, 10:20 PM
matmarex raised the priority of this task from Lowest to Normal.
matmarex closed this task as Resolved.Feb 5 2015, 8:03 PM

I think this is sufficiently complete now. Tech debt like T76477 remains.

Jdforrester-WMF moved this task from Doing to Reviewing on the OOUI board.Mar 26 2015, 8:59 PM
Danny_B moved this task from Doing… to OOUI on the UI-Standardization board.May 20 2016, 8:53 PM