Page MenuHomePhabricator

Re-evaluate (and disable) whitespace and newline removal in MobileFrontend/MinervaNeue
Open, Needs TriagePublic

Description

Currently SVGO settings and svg_check.sh are both including removal of whitespace and newline in SVG source code.
SVG icons are not render-blocking anymore.
On the other hand, removing whitespace from the SVGs is seriously impacting readability and maintainance. Making it very hard for humans (devs) to read the source code and identify issues aside from pure visual comparison.

Comparison of several gzipped SVG icons in bytes

iconwithout whitespace/newlinesw whitespace/newlinesdifference in bytes
communityportal-ltr63364512
recentchanges-ltr3023108
magnifying-glass3553638
issue-severity-low.svg59761821

As you can see the byte savings are not worth the extra effort and burden on developers. Note that the biggest of currently available icons in MinervaNeue is 'issue-severity-low.svg' and it is not perfectly optimized. Manually optimizing it (removing duplicated <g> and unncessary ids would save more than the few bytes saved by removing whitespace/newlines.
Even if we take into account all icons above the fold and in the main menu, 19 altogether, the bytes saved wouldn't make up for half a kb after gzipping.
With more icons coming from central OOUI library's dist directory, the already minimal overhead will be declining even further.

Proposal

  • Disable whitespace/newline removal and align MF/MN with other Wikimedia projects in SVG coding guidelines and add pretty print as expected SVGO output.
  • Recursively update all icons via svgo --config=.svgo.yml -r -f resources/

Event Timeline

I'd be more interested in reducing the number of image modules in MobileFrontend and Minerva. We have 8 of those in Minerva for example. I have no qualms with this as long term I think the plan is to use the OOUI icon packs in core and move these icons out of MobileFrontend/Minerva.

Change 570518 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/skins/MinervaNeue@master] Update '.svgo.yml' with pretty print rule and re-crush all icons

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

Change 570519 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/MobileFrontend@master] Update '.svgo.yml' with pretty print rule and re-crush all icons

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

Can we generate SVGs as a build product like we do in Popups? This allows for developers to have a development friendly version that is built by automation into the shipped user version.

@Niedzielski We do in OOUI. Again, the question is how much the effort for the in the end (after merging as many of the MF/MN specific icons "upstream" OOUI) of saving then 300 bytes across two dozen icons is really worth our time.

Can we generate SVGs as a build product like we do in Popups? This allows for developers to have a development friendly version that is built by automation into the shipped user version.

We don't do this in Popups. In Popups confusingly we use svg-inline-loader to load a single SVG mask but that has nothing to do with displaying icons. We have not yet experimented with using webpack to generate ResourceLoader CSS. I think the readability benefits outweigh the performance benefits here.

Change 570518 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Update '.svgo.yml' with pretty print rule and re-crush all icons

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

Change 570519 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Update '.svgo.yml' with pretty print rule and re-crush all icons

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