Page MenuHomePhabricator
Create Task
Maniphest T178867

[EPIC] Unify and optimize SVG markup across Foundation products
Closed, ResolvedPublic
Actions

Assigned To
Volker_E
Authored By
Volker_E
Oct 24 2017, 7:47 AM
Tags
Referenced Files
F10828305: image.png
Nov 16 2017, 4:53 PM
F10828307: image.png
Nov 16 2017, 4:53 PM
F10828321: image.png
Nov 16 2017, 4:53 PM
F10828325: image.png
Nov 16 2017, 4:53 PM
F10828303: image.png
Nov 16 2017, 4:53 PM
F10828309: image.png
Nov 16 2017, 4:53 PM
F10828332: image.png
Nov 16 2017, 4:53 PM
Subscribers
ABorbaWMF
Aklapper
Edtadros
Esanders
Etonkovidova
Jdlrobson
Jdrewniak
View All 12 Subscribers
Tokens
"Orange Medal" token, awarded by Krinkle."Doubloon" token, awarded by Nemo_bis.

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:

Sign off steps

  • Make sure all subtasks are resolved.

Details

Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
gerritbot added a comment.Nov 7 2017, 7:52 PM

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

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

Jdlrobson moved this task from To Do to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Nov 7 2017, 8:00 PM
Volker_E updated the task description. (Show Details)Nov 7 2017, 8:04 PM
MBinder_WMF subscribed.Nov 9 2017, 6:10 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
gerritbot added a comment.Nov 11 2017, 5:08 AM

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
Jdlrobson moved this task from Needs Code Review to Blocked on Others on the Readers-Web-Kanbanana-Board-Old board.Nov 13 2017, 6:43 PM

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
Volker_E mentioned this in T179361: Move MobileFrontend/Minerva's svg_check.sh script into a reusable, separate library.Nov 13 2017, 9:13 PM
Volker_E added a comment.Nov 14 2017, 12:44 AM

@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
gerritbot added a comment.Nov 14 2017, 3:54 AM

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

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

ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2017-11-14 (1.31.0-wmf.8)); removed MW-1.31-release-notes (WMF-deploy-2017-11-07 (1.31.0-wmf.7)).Nov 14 2017, 4:00 AM
Volker_E created subtask T180427: Align BetaFeature SVGs to each other.Nov 14 2017, 7:23 AM
Jdlrobson moved this task from Blocked on Others to Doing on the Readers-Web-Kanbanana-Board-Old board.Nov 14 2017, 6:12 PM
Jdlrobson moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.
gerritbot added a comment.Nov 14 2017, 9:48 PM

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

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

ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2017-11-28 (1.31.0-wmf.10)); removed MW-1.31-release-notes (WMF-deploy-2017-11-14 (1.31.0-wmf.8)).Nov 14 2017, 10:00 PM
Jdlrobson reassigned this task from Volker_E to ABorbaWMF.Nov 14 2017, 10:55 PM
Jdlrobson moved this task from Needs Code Review to Needs QA on the Readers-Web-Kanbanana-Board-Old board.

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

Volker_E mentioned this in T177006: Optimize the size of the SVG en-wordmark.Nov 16 2017, 6:46 AM
ABorbaWMF moved this task from Needs QA to Ready for Signoff on the Readers-Web-Kanbanana-Board-Old board.Nov 16 2017, 4:53 PM

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.

image.png (1×720 px, 521 KB)

image.png (1×720 px, 521 KB)

image.png (960×540 px, 149 KB)

image.png (1×720 px, 409 KB)

image.png (1×720 px, 179 KB)

image.png (1×720 px, 282 KB)

image.png (1×720 px, 161 KB)

• Nirzar added a comment.Nov 16 2017, 5:02 PM

Macro votecat: looks good

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.

Jdlrobson removed a project: Readers-Web-Kanbanana-Board-Old.Nov 17 2017, 6:50 PM
Volker_E added a comment.Nov 21 2017, 1:09 PM

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
Volker_E added a subscriber: Krinkle.Nov 30 2017, 7:46 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
Nemo_bis awarded a token.Dec 3 2017, 3:39 PM
Volker_E closed subtask T180427: Align BetaFeature SVGs to each other as Resolved.Dec 4 2017, 8:01 PM
mxn subscribed.Dec 9 2017, 6:04 AM
Volker_E added a comment.Dec 15 2017, 4:24 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 added a comment.Dec 18 2017, 5:38 PM
In T178867#3839463, @Volker_E wrote:

@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?!

In T178867#3801679, @Krinkle wrote:

[..] 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 added a project: Front-end-Standards-Group.Dec 19 2017, 2:42 AM
Volker_E updated the task description. (Show Details)
Volker_E added a subtask: T162319: Tooling: Should not be possible to ship un-optimised SVGs.Dec 19 2017, 3:18 AM
gerritbot added a comment.Dec 19 2017, 3:44 AM

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
gerritbot added a comment.Dec 19 2017, 5:08 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

Diffusion mentioned this in rESCCd3f07cd44a83: Unify SVG markup.Dec 19 2017, 5:11 AM
gerritbot added a comment.Dec 19 2017, 9:35 AM

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

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

ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2018-01-02 (1.31.0-wmf.15)); removed MW-1.31-release-notes (WMF-deploy-2017-11-28 (1.31.0-wmf.10)).Dec 19 2017, 10:00 AM
gerritbot added a comment.Dec 19 2017, 11:01 AM

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
Volker_E added a subtask: T175318: Optimize SVGs in data URIs.Dec 19 2017, 4:05 PM
Krinkle awarded a token.Dec 22 2017, 6:17 PM
Krinkle unsubscribed.
Volker_E updated the task description. (Show Details)Jan 4 2018, 9:10 PM
• Tbayer mentioned this in T184227: Measured impact of SVG optimizations.Jan 5 2018, 7:29 AM
Volker_E mentioned this in T185596: Bring SVGO-based automated optimization to all products.Jan 23 2018, 7:39 PM
gerritbot added a comment.Jan 25 2018, 1:35 AM

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
MBinder_WMF unsubscribed.Jan 25 2018, 1:44 AM
ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2018-02-06 (1.31.0-wmf.20)); removed MW-1.31-release-notes (WMF-deploy-2018-01-02 (1.31.0-wmf.15)).Jan 25 2018, 2:00 AM
Etonkovidova added a comment.Jan 30 2018, 12:31 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.

Volker_E removed a project: Patch-For-Review.Jan 31 2018, 11:40 PM
Volker_E mentioned this in T125292: Optimize how we load OOUI in MediaWiki.Feb 5 2018, 10:52 PM
Amire80 mentioned this in T170137: Expand button for compact language links features wrong colored icon.Mar 27 2018, 7:08 AM
• Tbayer closed subtask T184227: Measured impact of SVG optimizations as Resolved.Aug 3 2018, 2:48 PM
Volker_E closed subtask T185596: Bring SVGO-based automated optimization to all products as Resolved.Aug 8 2018, 3:39 PM
Jdlrobson mentioned this in T204260: Consolidate svgmin config and svg / grunt-svgmin checker.Sep 18 2018, 11:51 PM
Krinkle closed subtask T175318: Optimize SVGs in data URIs as Resolved.Sep 19 2019, 6:37 PM
Volker_E added a subtask: T245296: Unify SVG optimization to Wikimedia coding conventions in MobileFrontend and Minerva.Feb 17 2020, 8:34 PM
Volker_E mentioned this in T234070: Consider svgo use (via cssnano).Feb 18 2020, 2:33 AM
Jdlrobson added a comment.Mar 3 2020, 7:00 PM

@Volker_E why is this task still open? I'm a little confused about what's outstanding here. Could you update the description - maybe adding an unchecked checkbox?

Restricted Application added a subscriber: Masumrezarock100. · View Herald TranscriptMar 3 2020, 7:00 PM
Volker_E added a subscriber: Edtadros.Mar 3 2020, 7:02 PM

@Jdlrobson Two unresolved sub-tasks. T162319: Tooling: Should not be possible to ship un-optimised SVGs could/should probably not be a subtask anymore. T245296: Unify SVG optimization to Wikimedia coding conventions in MobileFrontend and Minerva is waiting for @Edtadros sign-off.

Volker_E removed a subtask: T162319: Tooling: Should not be possible to ship un-optimised SVGs.Mar 3 2020, 7:02 PM
Jdlrobson renamed this task from Unify and optimize SVG markup across Foundation products to [EPIC] Unify and optimize SVG markup across Foundation products.Mar 3 2020, 7:07 PM
Jdlrobson added a project: Epic.
Jdlrobson updated the task description. (Show Details)
gerritbot added a comment.Mar 4 2020, 12:24 AM

Change 575812 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Run .svg minifier in Vector skin

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

gerritbot added a project: Patch-For-Review.Mar 4 2020, 12:24 AM
ovasileva closed subtask T245296: Unify SVG optimization to Wikimedia coding conventions in MobileFrontend and Minerva as Resolved.Mar 6 2020, 11:39 AM
Volker_E removed a project: MW-1.31-release-notes (WMF-deploy-2018-02-06 (1.31.0-wmf.20)).Mar 8 2020, 4:14 AM
Volker_E updated the task description. (Show Details)
gerritbot added a comment.Mar 8 2020, 4:47 AM

Change 575812 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Re-crush SVG files with unified SVGO rules

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

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.23; 2020-03-10).Mar 8 2020, 5:00 AM
Volker_E closed this task as Resolved.Mar 8 2020, 5:05 AM
Volker_E removed a project: Patch-For-Review.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits