Page MenuHomePhabricator

OOUI's svgo-optimized SVG files are not compatible with rsvg's broken SVG parsing
Closed, ResolvedPublic

Description

OOUI's svgo-optimized SVG files are not compatible with rsvg's broken SVG parsing.

svgo emits SVG like <path d="M-1-2 …"> or <path d="M.3.5 …"> which rsvg is unable to parse (this is valid path data fragment that represents a points (-1; -2) and (0.3; 0.5), and the SVG 1.1 spec even explicitly discusses this syntax). The same issue affects ImageMagick when compiled with rsvg support.

This means that some of OOUI's and VisualEditor's icons will not appear correctly e.g. when uploaded to Commons, and blocks T76473: Implement ResourceLoader module to ship CSS for sets of SVG+PNG icons to the user since we're unable to rasterize the SVGs to PNG.

This can be worked around using some search-and-replace, but this leaves some questions:

  • Can this be fixed in rsvg? Gotta file a bug, even if it'll probably be ignored, and even if not no one is going to upgrade their copy.
  • Where do we apply the workaround? To the files in git? During build process? In MediaWiki's RL module from T76473?
  • Should we maintain the workaround ourselves somewhere, or maybe it could be upstreamed to svgo? (As a command-line option for people who want rsvg compatibility at the cost of a few bytes, like us?)

Event Timeline

matmarex created this task.Dec 5 2014, 12:23 PM
matmarex raised the priority of this task from to Needs Triage.
matmarex updated the task description. (Show Details)
matmarex added a project: OOUI.
matmarex changed Security from none to None.
matmarex added a subscriber: matmarex.

The workaround goes like this: https://github.com/trevorparscal/colorize/pull/1/files – in the simplest form, we insert some spaces and zeroes to create path data that is readable to rsvg.

This has already been implemented in ResourceLoaderImageModule for icons for T76473 (see ResourceLoaderImage::massageSvgPathdata()).

I think we should keep the workaround there (for potential third-party users) and apply the fix to SVG files in our repos, for the benefit of people wishing to have copies of the files on Commons. It costs us little and isn't just theoretical, but has already been complained about: https://lists.wikimedia.org/pipermail/wikitech-l/2014-November/079532.html

Haha, I just learned that we have just deployed a rsvg patch to fix this on Wikimedia wikis last week: https://gerrit.wikimedia.org/r/#/c/173639/ (80dd6ad665610979af294aeff4db417599363250). rsvg upstream bug is https://bugzilla.gnome.org/show_bug.cgi?id=620923, patch is https://git.gnome.org/browse/librsvg/commit/?id=5ba4343bccc7e1765f38f87490b3d6a3a500fde1.

So let's forget applying to fix to existing files or bothering svgo people, and just keep the one in ResourceLoaderImage patch for third-parties who don't have the most recent rsvg.

matmarex closed this task as Resolved.Dec 5 2014, 12:57 PM
matmarex claimed this task.
Jdforrester-WMF moved this task from Backlog to Reviewing on the OOUI board.Mar 26 2015, 9:00 PM