Page MenuHomePhabricator

Unify and optimize SVG markup across Foundation products
Open, MediumPublic

Description

SVGs in our products feature all kinds of markup, in parts with editor specific metadata, resulting in unnecessary inconsistency and bloat.
We're using SVGO for optimization and go for manual QA, both on code and visual level.
Optimizations achieved are between 3%–27% file size on average before gzipping, in a few cases, like MMV even up to 78%.

Among changes:

  • Unify XML declaration, preferable (altough maybe slighly esoteric) with uppercase UTF-8: <?xml version="1.0" encoding="UTF-8"?>
    • Removing standalone="no" attribute as its default
  • Removing additional DOCTYPE
  • Setting of width and height attributes together with viewport for widest cross-browser compatibility, example: <svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20">
    • Removing px value unit as it's default
    • Removing non-standard enable-background attribute, removed from SVG standard altogether
  • Reduction of ids where applicable
  • Changing indentation to use tabs & reduce whitespace where applicable, example: "/> not " />
  • Minify and lowercase colors, example: #fff not #FFFFFF
  • Collapsing useless groups g
  • Merging paths where applicable
  • Removing editor (InkScape, Illustrator etc.) data
  • Removing unnecessary style attributes where applicable
  • Removing unused defs
  • Removing raster images where applicable
  • Sorting attributes

Above optimizations are a mix of SVGO's available plugins and manual optimizations. They've been collected now at https://www.mediawiki.org/wiki/Manual:Coding_conventions/SVG


Products optimized:

Themes:

Extensions:

Projects:

Details

Related Gerrit Patches:
mediawiki/extensions/MultimediaViewer : masterUnify SVG markup
mediawiki/extensions/RelatedArticles : masterUnify SVG markup
mediawiki/extensions/TwoColConflict : masterUnify SVG markup
mediawiki/skins/MinervaNeue : masterUnify SVG markup
mediawiki/extensions/Flow : masterUnify SVG markup
wikimedia/portals : masterSVG Sprites - Adding new SVG files
mediawiki/extensions/VisualEditor : masterUnify SVG markup
mediawiki/extensions/Cite : masterUnify SVG markup
mediawiki/extensions/Echo : masterUnify SVG markup
mediawiki/core : masterOptimize SVGs and align colors to WikimediaUI palette
mediawiki/extensions/UniversalLanguageSelector : masterAlign SVGs to WikimediaUI color palette and optimize
oojs/ui : mastericons: Unify SVG markup
mediawiki/extensions/CentralNotice : masterAlign colors to WikimediaUI color palette
mediawiki/extensions/Kartographer : masterUnify SVG markup
mediawiki/skins/Vector : masterOptimize SVGs and align to WikimediaUI color palette

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Volker_E updated the task description. (Show Details)Nov 4 2017, 6:01 AM

Change 386254 merged by jenkins-bot:
[mediawiki/core@master] Optimize SVGs and align colors to WikimediaUI palette

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

Volker_E updated the task description. (Show Details)Nov 4 2017, 7:09 PM
Volker_E updated the task description. (Show Details)Nov 4 2017, 8:17 PM

Change 389230 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/Echo@master] Unify SVG markup

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

Volker_E updated the task description. (Show Details)Nov 4 2017, 9:24 PM

Change 389290 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Unify SVG markup

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

Volker_E updated the task description. (Show Details)Nov 4 2017, 11:24 PM

Change 389230 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Unify SVG markup

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

Volker_E updated the task description. (Show Details)Nov 5 2017, 10:17 PM

Change 388161 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Unify SVG markup

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

Change 388933 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Unify SVG markup

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

We're touching quite a lot of SVG files here @Nirzar and @ovasileva so I'd like this to go through our normal QA process to avoid any icon regressions. In particular we'll need some time-constrained support from @ABorbaWMF

@Jdlrobson I've also asked @Etonkovidova for QA support on Contributors side of things, especially on the larger patches of Echo and StructuredDiscussions (Flow, still to be done).

I will speak with @ABorbaWMF about this and update here but I agree this needs QA

@Volker_E can we add Acceptance Criteria and steps to verify the patches so @ABorbaWMF can check reading parts when the time comes?

Change 387192 merged by jenkins-bot:
[wikimedia/portals@master] SVG Sprites - Adding new SVG files

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

Volker_E updated the task description. (Show Details)Nov 7 2017, 8:04 PM

@Jdlrobson The team inferred that you are doing the code review for the Web team stuff. Is that right?

Volker_E updated the task description. (Show Details)Nov 9 2017, 11:50 PM

Change 390557 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/Flow@master] Unify SVG markup

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

Volker_E updated the task description. (Show Details)Nov 11 2017, 5:12 AM

Per discussion with Nirzar, I'll review https://gerrit.wikimedia.org/r/389290 tomorrow after the train is cut. This will maximise testing time.

ovasileva triaged this task as Medium priority.Nov 13 2017, 7:14 PM

@ABorbaWMF @Nirzar Following are the MinveraNeue patch icons where more than just simple reordering of attributes was done (additional viewBox attributes for example):

  • magnifying glass,
  • external link icon & its RTL version,
  • userAnonymous
  • watch & watched
  • edit & editLocked
  • error
Volker_E updated the task description. (Show Details)Nov 14 2017, 12:50 AM

Change 390557 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Unify SVG markup

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

Change 389290 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Unify SVG markup

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

Over to you @ABorbaWMF to get those icons tested. We'll need to review icons in the mobile site on the beta cluster.

So far things look good. I am not sure if I have the right icons for external link icon & its RTL version. I also don't think we have an edit-locked article on beta right now.







Volker_E claimed this task.Nov 16 2017, 5:11 PM
Volker_E updated the task description. (Show Details)
ovasileva closed this task as Resolved.Nov 17 2017, 12:14 PM

all done!

Volker_E reopened this task as Open.Nov 17 2017, 6:04 PM

@ovasileva In this task there's are still products that will see patches/need code review. Only the MinervaNeue part was resolved recently. StructuredDiscussions, MMV and the Style Guide are among those TBD.

Insightful read on further optimization, in case we want to go crazier on SVG optimization and gzip/brotli preparation:
https://blog.usejournal.com/of-svg-minification-and-gzip-21cd26a5d007

Featuring

  • unclosing paths (remove end z) as they're auto-closing or
  • making use of more backreferences (we already go for example for only hex color values

Most of the presented steps generally don't scale as it needs per-file dev supervision. In some cases like the Wikipedia logo it could be useful though.

Volker_E updated the task description. (Show Details)Nov 28 2017, 5:41 AM

After more detailed feedback on XML declaration necessity in SVG here, reciting:

GreLI commented on #836:
AFAIK, Firefox 3.6 doesn't support SVG except in <object>, and Android 2.3 doesn't support SVG at all. These are surely “issues”, but I think, it doesn't worth supporting SVG for them—a fallback should be provided in a such case.
I've heard nothing about something couldn't render SVG because of absent prolog. The only case was, that some primitive utils like file couldn't get MIME-type right, discussed in #306.

See also https://www.w3.org/TR/REC-xml/#charencoding

Should we also get rid of the XML declarations globally and would we be on the safe side not running into a weird encoding hell?
Opinions, @Krinkle or @Esanders ?

Krinkle added a comment.EditedDec 1 2017, 12:27 AM

@Volker_E The main concern I'd have is mime-type checking (e.g. https://github.com/svg/svgo/issues/306).

Many mime-type tools wrongly interpret files starting with <svg as something different.

With <?xml header: image/svg+xml.
Without:

  • PHP 7.1 mime_content_type on macOS: text/html
  • file command on Linux: text/plain
  • file command on macOS: image/svg (promising!)

I know that within our systems, ResourceLoader (CSSMin) and MediaWiki more generally prefer mime-type guessing based on file name in many cases, but not all cases. So I can't confidently say nothing will break when we remove the header.

Volker_E updated the task description. (Show Details)Dec 1 2017, 8:58 PM
Volker_E updated the task description. (Show Details)Dec 2 2017, 4:37 AM
mxn added a subscriber: mxn.Dec 9 2017, 6:04 AM

@Krinkle For the OOUI icons via RL @embed XML processing information should be considered then… Where would be the best place of removal then, probably in CSSMin.php?!

Volker_E updated the task description. (Show Details)Dec 15 2017, 5:15 AM

@Krinkle For the OOUI icons via RL @embed XML processing information should be considered then… Where would be the best place of removal then, probably in CSSMin.php?!

[..] within our systems (ResourceLoader, CSSMin) [..] generally prefer mime-type guessing based on file name [..]

This means they'd return image/svg+xml based on the filename ending in .svg, regardless of what is inside the file.

I'm fairly sure that within ResourceLoader nothing would be affected by your proposed change. However, we do use mime_content_type() in many other places in MediaWiki, and even CSSMin does use it as a fallback, after checking a fixed set of common file extensions. I can't say for certain whether or not one of those other places will scan an OOUI file at some point.

Also, at Wikimedia production, serving static files (e.g. debug mode) will touch code beyond ResourceLoader (Varnish, Nginx, Apache, wmf-config static.php). Paths like /static/current are handled by Apache I believe, not MediaWiki PHP. I don't know if that uses content-based mime checking.

Paths like /w/resources/ are nowadays proxied by static.php (in wmf-config), which uses StreamFile::contentTypeFromPath (MediaWiki PHP), and works similar to CSSMin (guess type based on file name).

From a quick glance it should be fine, at least for ResourceLoader and related Wikimedia production infrastructure. But I'm not sure if we want to depend on that.

I agree that it might be best to keep the XML header in the files and strip it only as an optimisation when embedding. For that, CSSMin is indeed the appropiate place. I'd recommend elaborating on the existing encodeStringAsDataURI method, with a case specific to the $type value for SVG files.

Volker_E updated the task description. (Show Details)

Change 399133 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/RelatedArticles@master] Unify SVG markup

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

Volker_E updated the task description. (Show Details)Dec 19 2017, 3:48 AM

Change 399136 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/TwoColConflict@master] Unify SVG markup

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

Change 399136 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Unify SVG markup

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

Change 399133 merged by jenkins-bot:
[mediawiki/extensions/RelatedArticles@master] Unify SVG markup

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

Volker_E updated the task description. (Show Details)Dec 19 2017, 3:11 PM
Volker_E updated the task description. (Show Details)Dec 19 2017, 4:03 PM
Krinkle removed a subscriber: Krinkle.
Volker_E updated the task description. (Show Details)Jan 4 2018, 9:10 PM

Change 388833 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] Unify SVG markup

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

Volker_E updated the task description. (Show Details)Jan 25 2018, 1:35 AM

Checked in betalabs (including Saucelabs IE11 Windows combination), checked different skins - as far as I could see, no regressions of any sort are present.