Page MenuHomePhabricator

Reduce output size of css-icon mixin
Closed, ResolvedPublic1 Estimated Story Points

Description

The fix for T356540 changed the cdx-mixin-css-icon mixin to use mask-image instead of background-image. As part of that change, the output is now much larger, in part because it contains three copies of the icon SVG, instead of one.

Before (532 bytes):

.minerva-icon--download {
    background-position: center;
    background-repeat: no-repeat;
    background-size: calc(max(1.25em, 20px));
    min-width: 20px;
    min-height: 20px;
    width: 1.25em;
    height: 1.25em;
    display: inline-block;
    vertical-align: text-bottom;
    background-image: url("data:image/svg+xml;utf8,<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"20\" height=\"20\" viewBox=\"0 0 20 20\" fill=\"%2354595d\"><path d=\"M17 12v5H3v-5H1v5a2 2 0 002 2h14a2 2 0 002-2v-5z\"/><path d=\"M15 9h-4V1H9v8H5l5 6z\"/></svg>")
}

After (1977 bytes):

.minerva-icon--download {
    min-width: 20px;
    min-height: 20px;
    width: 1.25em;
    height: 1.25em;
    display: inline-block;
    vertical-align: text-bottom
}

@supports not ((-webkit-mask-image:none) or (mask-image:none)) {
    .minerva-icon--download {
        background-position: center;
        background-repeat: no-repeat;
        background-size: calc(max(1.25em, 20px))
    }
}

@supports (-webkit-mask-image:none) or (mask-image:none) {
    .minerva-icon--download {
        -webkit-mask-size: calc(max(1.25em, 20px));
        mask-size: calc(max(1.25em, 20px));
        -webkit-mask-repeat: no-repeat;
        mask-repeat: no-repeat;
        -webkit-mask-position: center;
        mask-position: center;
    }
}

@supports not ((-webkit-mask-image:none) or (mask-image:none)) {
    .minerva-icon--download {
        background-image: url("data:image/svg+xml;utf8,<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"20\" height=\"20\" viewBox=\"0 0 20 20\" fill=\"%23000000\"><path d=\"M17 12v5H3v-5H1v5a2 2 0 002 2h14a2 2 0 002-2v-5z\"/><path d=\"M15 9h-4V1H9v8H5l5 6z\"/></svg>");
        filter: invert(0);
        opacity: 0.87
    }

    .cdx-button:not(.cdx-button--weight-quiet):disabled .minerva-icon--download,
    .cdx-button--weight-primary.cdx-button--action-progressive .minerva-icon--download,
    .cdx-button--weight-primary.cdx-button--action-destructive .minerva-icon--download {
        filter: invert(1)
    }
}

@supports (-webkit-mask-image:none) or (mask-image:none) {
    .minerva-icon--download {
        -webkit-mask-image: url("data:image/svg+xml;utf8,<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"20\" height=\"20\" viewBox=\"0 0 20 20\" fill=\"%23000000\"><path d=\"M17 12v5H3v-5H1v5a2 2 0 002 2h14a2 2 0 002-2v-5z\"/><path d=\"M15 9h-4V1H9v8H5l5 6z\"/></svg>");
        mask-image: url("data:image/svg+xml;utf8,<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"20\" height=\"20\" viewBox=\"0 0 20 20\" fill=\"%23000000\"><path d=\"M17 12v5H3v-5H1v5a2 2 0 002 2h14a2 2 0 002-2v-5z\"/><path d=\"M15 9h-4V1H9v8H5l5 6z\"/></svg>");
        background-color: #54595d
    }
}

The output size necessarily increases because we now have to have rules for both background-image and mask-image, but we should try to reduce this as much as possible.

Possible ideas for reduction

  • The same data URI repeats three times (for background-image, -webkit-mask-image, and mask-image). Maybe it could be deduplicated by putting it in a CSS variable?
    • Evaluation: This would actually increase the size after gzip as we'd introduce new code. Aside of that, there's also the bigger question if such custom properties weren't better off as global ones and not internally scoped.
  • The cdx-button-related rules should not be output if param-is-button-icon is set to false, as it is here
  • There are 2 @supports blocks and 2 @supports not blocks, perhaps they could be consolidated into 1 block each? (This may be difficult to do given the internal structure of the mixin)
    • Evaluation: It seems really difficult to reach this given the current code complexity of the mixin. Also this problem will be reduced massively with T383943 enacted.
  • @thiemowmde brought forward the great idea of removing xmls:link from svg root element and only provide it on elements within in the currently 4 icons that need it

Before we get too deep into optimizations, we should check whether the proposed optimization would actually matter after gzip.

Acceptance criteria for done

  • Output cdx-button-related rules only if param-is-button-icon is set to false
  • Bring mask-* properties in same order as background ones
  • Remove xmls:link from svg element (and provide in the inner elements in currently 4 icons that actually make use of it

Event Timeline

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald Transcript

Let's keep in mind that T367821: Discovery: Deprecation of TLS 1.2 would elevate our needs from providing the background fallback for Firefox v39-52.

Volker_E added a subscriber: thiemowmde.

On the open questions from the task:

The same data URI repeats three times (for background-image, -webkit-mask-image, and mask-image). Maybe it could be deduplicated by putting it in a CSS variable?

Putting the icon into a var might be slightly preferable from developer experience – not having to weed through SVG background-image URLs over and over again, but from a performance gain perspective it's unsurprisingly even increasing the whole output by a few bytes (5 from an unsolicited testing).
It also brings the disadvantage of having a local CSS var, the Codex icons should probably –if as Custom Properties– be available in a central module on :root.

The cdx-button-related rules should not be output if param-is-button-icon is set to false

This seems like a feasible, small and useful refinement.

There are 2 @supports blocks and 2 @supports not blocks, perhaps they could be consolidated into 1 block each?

Combining the @supports and not @supports rules brings down one icon output by lame 3 bytes after gzip (from 578 bytes to 575 bytes) per icon in my testing. I don't think this is worth to work with plugging nested mixins into the already very complex 'css-icon' mixin for this gain. Waiting for TLS task above for Firefox <=52 fallback removal a few more weeks or few months seems a better choice.

Clear bigger impact has (slightly surprising to me as gzip tester for years) @thiemowmde's great idea on limiting the xmlns:xlink attribute to be specific to only icons where it's needed. In my testing it brings down the size by 10 bytes after gzip on one icon.

Also "fun": reducing the longform hex code #000000 to #000 is saving us another 3 bytes.

Change #1111884 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] mixins: Reduce to shorthand hex color in 'css-icon' mixin

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

Change #1111649 had a related patch set uploaded (by VolkerE; author: Thiemo Kreuz (WMDE)):

[design/codex@main] icons: Remove `xmlns:xlink` namespace where unnecessary

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

Change #1111914 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] mixins: Reduce output of 'css-icon' when not a button icon

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

Another optimization technique I learned over the years is to re-arrange the code in a way so that it still does the same as before (often the exact execution order makes no difference) but the gzip algorithm has an easier time finding repeating patterns. Here I was able to reduce the example given in the task description from 567 to 562 bytes (gzipped).

.minerva-icon--download {
	min-width: 20px;
	min-height: 20px;
	width: 1.25em;
	height: 1.25em;
	display: inline-block;
	vertical-align: text-bottom;
}

@supports not ((-webkit-mask-image:none) or (mask-image:none)) {
	.minerva-icon--download {
		background-repeat: no-repeat;
		background-position: center;
		background-size: calc(max(1.25em, 20px));
	}
}

@supports (-webkit-mask-image:none) or (mask-image:none) {
	.minerva-icon--download {
		-webkit-mask-repeat: no-repeat;
		mask-repeat: no-repeat;
		-webkit-mask-position: center;
		mask-position: center;
		-webkit-mask-size: calc(max(1.25em, 20px));
		mask-size: calc(max(1.25em, 20px));
	}
}

@supports not ((-webkit-mask-image:none) or (mask-image:none)) {
	.minerva-icon--download {
		filter: invert(0);
		background-image: url("data:image/svg+xml;utf8,<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"20\" height=\"20\" viewBox=\"0 0 20 20\" fill=\"%23000000\"><path d=\"M17 12v5H3v-5H1v5a2 2 0 002 2h14a2 2 0 002-2v-5z\"/><path d=\"M15 9h-4V1H9v8H5l5 6z\"/></svg>");
		opacity: 0.87;
	}

	.cdx-button--weight-primary.cdx-button--action-progressive .minerva-icon--download,
	.cdx-button--weight-primary.cdx-button--action-destructive .minerva-icon--download,
	.cdx-button:not(.cdx-button--weight-quiet):disabled .minerva-icon--download {
		filter: invert(1);
	}
}

@supports (-webkit-mask-image:none) or (mask-image:none) {
	.minerva-icon--download {
		-webkit-mask-image: url("data:image/svg+xml;utf8,<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"20\" height=\"20\" viewBox=\"0 0 20 20\" fill=\"%23000000\"><path d=\"M17 12v5H3v-5H1v5a2 2 0 002 2h14a2 2 0 002-2v-5z\"/><path d=\"M15 9h-4V1H9v8H5l5 6z\"/></svg>");
		mask-image: url("data:image/svg+xml;utf8,<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" width=\"20\" height=\"20\" viewBox=\"0 0 20 20\" fill=\"%23000000\"><path d=\"M17 12v5H3v-5H1v5a2 2 0 002 2h14a2 2 0 002-2v-5z\"/><path d=\"M15 9h-4V1H9v8H5l5 6z\"/></svg>");
		background-color: #54595d;
	}
}

However, I'm not sure if this scales in real-world scenarios and is worth it.

What also helps is to just be consistent, which is what e.g. https://gerrit.wikimedia.org/r/1111883 is about.

Change #1111884 merged by jenkins-bot:

[design/codex@main] mixins: Reduce to shorthand hex color in 'css-icon' mixin

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

Change #1111914 merged by jenkins-bot:

[design/codex@main] mixins: Reduce output of 'css-icon' when not a button icon

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

Change #1112309 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] build: Disallow setting xlink:namespace on the `svg` element

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

Change #1111649 merged by jenkins-bot:

[design/codex@main] icons, docs: Remove `xmlns:xlink` namespace where unnecessary

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

Change #1112309 merged by jenkins-bot:

[design/codex@main] build: Disallow setting xlink:namespace on the `svg` element

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

Change #1113883 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from 1.19.0 to 1.20.0

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

Change #1113883 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from 1.19.0 to 1.20.0

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

Thanks again @thiemowmde for the excellent optimization idea.

I must do something wrong. But when I test this now the xmlns:xlink appears in both places: in the <svg> header and in the individual element (a <g> for example). Can anyone reproduce this? Could it be that SVGO puts it back?