Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes required "to" and "from" keyframe in animation #448

Closed
SethFalco opened this issue Dec 25, 2021 · 1 comment
Closed

Removes required "to" and "from" keyframe in animation #448

SethFalco opened this issue Dec 25, 2021 · 1 comment

Comments

@SethFalco
Copy link

SethFalco commented Dec 25, 2021

When an SVG has key frames with to or from, CSSO will remove them, which changes the animation.

Expected Behavior

The to and from frames should be preserved/converted, otherwise it modifies the animation.

For example: 0%,to{clip-path:inset(84%0 0)}, should not become 0%{clip-path:inset(84%0 0)}.

Minimal Working Example

The SVG I'm optimizing in SVGO.

<svg width="48" height="48" viewBox="0 0 12.7 12.7" xmlns="http://www.w3.org/2000/svg"><style>@keyframes a{0%,to{clip-path:inset(84%0 0)}4%,96%{clip-path:inset(74%0 0)}16%,52%,84%{clip-path:inset(16%0 0)}20%,48%{clip-path:inset(19%0 0)}24%,44%{clip-path:inset(26%0 0)}28%,64%,72%{clip-path:inset(29%0 0)}32%{clip-path:inset(45%0 0)}36%,40%{clip-path:inset(35%0 0)}60%,76%{clip-path:inset(23%0 0)}68%{clip-path:inset(32%0 0)}}@keyframes b{0%,32%,40%,68%,to{clip-path:inset(13%0 0)}4%{clip-path:inset(20%0 0)}76%,8%{clip-path:inset(23%0 0)}16%,84%{clip-path:inset(29%0 0)}28%,72%,96%{clip-path:inset(19%0 0)}36%{clip-path:inset(16%0 0)}48%{clip-path:inset(45%0 0)}52%{clip-path:inset(58%0 0)}56%{clip-path:inset(48%0 0)}60%{clip-path:inset(35%0 0)}64%,92%{clip-path:inset(26%0 0)}88%{clip-path:inset(39%0 0)}}@keyframes c{0%,to{clip-path:inset(52%0 0)}16%{clip-path:inset(0 0 0)}20%,84%{clip-path:inset(10%0 0)}24%,52%{clip-path:inset(26%0 0)}28%{clip-path:inset(39%0 0)}32%,44%{clip-path:inset(48%0 0)}36%{clip-path:inset(55%0 0)}40%{clip-path:inset(68%0 0)}48%,92%{clip-path:inset(32%0 0)}56%{clip-path:inset(16%0 0)}60%,76%,88%{clip-path:inset(23%0 0)}68%{clip-path:inset(29%0 0)}72%{clip-path:inset(35%0 0)}80%{clip-path:inset(13%0 0)}}</style><path d="M2.117 2.249h2.38v8.202h-2.38z" fill="#FFF" style="animation:a 1.25s linear infinite"/><path d="M5.292 2.249h2.38v8.202h-2.38z" fill="#FFF" style="animation:b 1.25s linear infinite"/><path d="M8.467 2.249h2.38v8.202h-2.38z" fill="#FFF" style="animation:c 1.25s linear infinite"/></svg>

A minimal example (using my actual use-case) of how it's being passed to CSSO.

const { minify } = require('csso');
const fs = require('fs');

const src = '@keyframes a{0%,to{clip-path:inset(84%0 0)}4%,96%{clip-path:inset(74%0 0)}16%,52%,84%{clip-path:inset(16%0 0)}20%,48%{clip-path:inset(19%0 0)}24%,44%{clip-path:inset(26%0 0)}28%,64%,72%{clip-path:inset(29%0 0)}32%{clip-path:inset(45%0 0)}36%,40%{clip-path:inset(35%0 0)}60%,76%{clip-path:inset(23%0 0)}68%{clip-path:inset(32%0 0)}}@keyframes b{0%,32%,40%,68%,to{clip-path:inset(13%0 0)}4%{clip-path:inset(20%0 0)}76%,8%{clip-path:inset(23%0 0)}16%,84%{clip-path:inset(29%0 0)}28%,72%,96%{clip-path:inset(19%0 0)}36%{clip-path:inset(16%0 0)}48%{clip-path:inset(45%0 0)}52%{clip-path:inset(58%0 0)}56%{clip-path:inset(48%0 0)}60%{clip-path:inset(35%0 0)}64%,92%{clip-path:inset(26%0 0)}88%{clip-path:inset(39%0 0)}}@keyframes c{0%,to{clip-path:inset(52%0 0)}16%{clip-path:inset(0 0 0)}20%,84%{clip-path:inset(10%0 0)}24%,52%{clip-path:inset(26%0 0)}28%{clip-path:inset(39%0 0)}32%,44%{clip-path:inset(48%0 0)}36%{clip-path:inset(55%0 0)}40%{clip-path:inset(68%0 0)}48%,92%{clip-path:inset(32%0 0)}56%{clip-path:inset(16%0 0)}60%,76%,88%{clip-path:inset(23%0 0)}68%{clip-path:inset(29%0 0)}72%{clip-path:inset(35%0 0)}80%{clip-path:inset(13%0 0)}}';

const optimized = minify(src, {
  usage: {
    tags: ['svg', 'style', 'path']
  }
});

fs.writeFile('minified.css', optimized.css, 'utf8', () => {
  console.log('done');
});

I think the issue is caused by to/from being interpreted as a TypeSelector in the AST, and then being stripped out because it's not in the whitelisted array of tags.

"prelude": {
  "type": "SelectorList",
  "loc": null,
  "children": [
    {
      "type": "Selector",
      "loc": null,
      "children": [
        {
          "type": "Percentage",
          "loc": null,
          "value": "0"
        }
      ]
    },
    {
      "type": "Selector",
      "loc": null,
      "children": [
        {
          "type": "TypeSelector",
          "loc": null,
          "name": "to"
        }
      ]
    }
  ]
},

Before I do anything else, I'm wondering, is CSSTree correct to report that as a TypeSelector? 🤔

This might be a bug in CSSTree, since to and from aren't type selectors. (https://www.w3.org/TR/selectors-3/#type-selectors)
If they had to be named, it'd probably fall under keyframe selectors, or KeyframeSelector. (https://www.w3.org/TR/css-animations-1/#typedef-keyframe-selector)

Related

@SethFalco SethFalco changed the title Removes required "to" key frames in animation Removes required "to" and "from" key frames in animation Dec 25, 2021
@SethFalco SethFalco changed the title Removes required "to" and "from" key frames in animation Removes required "to" and "from" keyframe in animation Dec 25, 2021
@lahmatiy
Copy link
Member

That's a pretty interesting bug. There is no a big deal that css-tree treated from/to as TypeSelector but CSSO should avoid applying usage filtering to @keyframe rules as well as for other special at-rules like @page (not the case for SVGO but still). I think to fix it we need add an additional check here that a rule don't belong to a special at-rule like @keyframe, @page etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants