Skip to content

[web-animations-1] Web animations should read prefixed/aliased properties too #3948

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

Open
birtles opened this issue May 21, 2019 · 9 comments

Comments

@birtles
Copy link
Contributor

birtles commented May 21, 2019

Background: Mozilla bug 1551818

In Firefox 68 and prior it was possible to animate -moz-user-select using { MozUserSelect: ['text', 'none'] } but that stopped working recently since we made -moz-user-select an alias of user-select and we ignore aliases when reading keyframes.

Web Animations doesn't specifically say not to read aliases or prefixed properties on keyframes but that was the intention in the past and in my testing neither Chrome nor Safari read Webkit... prefixes.

I believe Shane was of the view that we shouldn't support prefixed properties because they are on the way out. Since then, however, we've specified a number of these properties in the compatibility spec and elsewhere and some properties still don't have un-prefixed equivalents. Furthermore, when exposing CSS animations / transitions that use these properties we have to return something and whatever we return from getKeyframes() should be able to be round-tripped using setKeyframes().

(The round-tripping for CSS animations/transitions works in Gecko simply because we resolve aliases when parsing the @keyframes / transitions so while we might return a prefixed property from getKeyframes() we'll never return an alias. Furthermore, our implementation of setKeyframes() allows prefixed properties, just not aliases.)

I suggest Web Animations should accept prefixed properties (to support properties which don't yet have an unprefixed version) and should accept aliased properties (so that content using the prefixed version continues to work once it is un-prefixed).

When an aliased property is specified along with the property it reflects, the property it reflects should win (i.e. transform trumps MozTransform). This would be resolved in the procedure to calculate computed keyframes, i.e. the same place we specify that longhands override shorthands, physical overrides logical etc.

When multiple aliases are specified (e.g. WebkitTransform and MozTransform) without the property they refer to (i.e. transform) then we would just use the same procedure as for overlapping shorthands -- sort by unicode codepoints of the property IDL names.

Thoughts?

@emilio
Copy link
Collaborator

emilio commented May 21, 2019

Is there any sorting order between JS object keys? I think we shouldn't let the aliases get more through the system, as soon as they're parsed they should be transformed to the aliased property IMO.

@birtles
Copy link
Contributor Author

birtles commented May 21, 2019

Is there any sorting order between JS object keys?

I'm not sure which part you're referring to but both the order in which the keys are read off the objects is specified, as is the order in which they are resolved when they overlap.

I think we shouldn't let the aliases get more through the system, as soon as they're parsed they should be transformed to the aliased property IMO.

My concern is that will mean that code like the following won't work:

const anim = elem.animate({ MozTransform: 'translate(100px)' }, 1000);
// Update target translation to 200px
anim.effect.setKeyframes({
  ...anim.effect.getKeyframes()[0],
  MozTransform: 'translate(200px)'
});

(since the initial MozTransform will be transformed into transform which will override the new MozTransform).

It also means that code like the following will stop working when we unprefix '-moz-user-select' (as just happened):

const initialValue = anim.effect.getKeyframes()[0].MozUserSelect;

@emilio
Copy link
Collaborator

emilio commented May 22, 2019

(since the initial MozTransform will be transformed into transform which will override the new MozTransform).

That's kind of what I'd expect, fwiw, and what would happen with elem.style.MozTransform = " ... ";.

const initialValue = anim.effect.getKeyframes()[0].MozUserSelect;

That seems a bit more annoying indeed. That doesn't happen on the OM because you can keep the getters. I guess it's too late for making getKeyframes() return some sort of interface :)

I don't recall what shorthands do, I guess they don't get expanded on getKeyframes()?

@emilio
Copy link
Collaborator

emilio commented May 22, 2019

And, fwiw, it may be ok not to fix this, if the "longhand that is later changed to be a shorthand" works well... I suspect animating -moz-user-select is not something that people are doing on the wild.

If other browsers already do not support animating prefixed properties, that makes me a lot less scared than when I filed the gecko bug about this :)

@birtles
Copy link
Contributor Author

birtles commented May 22, 2019

That doesn't happen on the OM because you can keep the getters. I guess it's too late for making getKeyframes() return some sort of interface :)

I'd rather not if we can avoid it but it's probably not impossible.

I don't recall what shorthands do, I guess they don't get expanded on getKeyframes()?

Right, they don't get expanded. (For keyframes from CSS animations we actually build the keyframes from the computed values so we don't get shorthands for them, but for Web Animations we basically preserve the keyframes as passed-in.)

And, fwiw, it may be ok not to fix this, if the "longhand that is later changed to be a shorthand" works well

Right, the longhand → shorthand change works fine. The property → alias change doesn't. Assuming the list of prefixed properties without an unprefixed version is steadily shrinking and is expected to reach ~0 in the nearish future then that might be ok. In fact it would add an extra incentive to spec unprefixed versions of such properties. I'm not sure what that list currently looks like, however.

@emilio
Copy link
Collaborator

emilio commented May 22, 2019

You can get an idea of the list of prefixed properties we support here:

https://searchfox.org/mozilla-central/search?q=%22-moz-&case=false&regexp=false&path=mako.rs

Of those a bunch are internal only (not web-exposed), and the XUL / legacy flexbox ones are unlikely to be ever unprefixed. So I think the only potential risks would be:

  • -moz-text-size-adjust
  • -moz-appearance
  • -moz-tab-size
  • Maybe -moz-user-{input,modify,focus}? I suspect not though, the whole concept behind those is a bit broken.

I think I could live with that. But not sure how that list looks for WebKit / Blink, or whether they support animating prefixed non-alias properties at all. Do Chromium / Safari allow animating e.g. -webkit-appearance?

@birtles
Copy link
Contributor Author

birtles commented May 22, 2019

You can get an idea of the list of prefixed properties we support here:

https://searchfox.org/mozilla-central/search?q=%22-moz-&case=false&regexp=false&path=mako.rs

That doesn't include -webkit- ones though. I think there are a few of those already in Gecko like -webkit-line-clamp, -webkit-text-fill-color, -webkit-text-stroke-color, and -webkit-text-stroke-width. I wonder how many more there will be in future.

For these properties we can either:

  1. Never unprefix them
  2. Not allow them to be animated until it they are unprefixed
  3. Let them break when they are unprefixed (at which point the prefixed one becomes an alias for the unprefixed one)

I believe Gecko is doing 3 while Blink and Safari are doing 2.

@birtles
Copy link
Contributor Author

birtles commented May 22, 2019

Do Chromium / Safari allow animating e.g. -webkit-appearance?

Last I checked, they don't support animating WebkitTransform (presumably an alias) or WebkitAppearance (not an alias).

@emilio
Copy link
Collaborator

emilio commented May 22, 2019

Ok... Then we should probably just fix Gecko to not allow animating these prefixed properties either.

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