[web-animations] Error handling in KeyframeEffect.pseudoElement

[george-steel]

What should KeyframeEffect.pseudoElement do if it is set in an invalid or unsupported value? I can see a few possibilities here.

1. Throw an exception. This can easily break backwards compatibility as the set of pseudo-elements is not currently fixed. Running an animation with a new pseudoelement on an old browser can crash the whole script. I think it makes sense for inputs that don't start with "::" though.

2. Set pseudoElement to null. This fails without crashing but has the disadvantage of causing animations to happen on the wrong element in the case of an unsupported pseudo-element.

3. Leave pseudoElement unchanged. Again, can cause false positives.

4. Set pseudoElement to the text given and do not animate.

5. Set pseudoElement to s sentinel value (such as ::unknown) which has no effect. Allows for error detection without the possibility of crashes.

My personal preference is for option 5.

[george-steel]

@birtles @stephenmcgruer

[birtles]

There's also the option of making it read-only. (i.e. you can only animate pseudos via CSS animations/transitions.)

Otherwise I'd lean towards 4 with a console warning.

[george-steel]

Is there any way to add error detection to 4?

[george-steel]

In my current patch do far I do 4 with the but it throws an exception if the selector does not start with a double-colon. An unknown selector gets stored and not animated.

[birtles]

Is there any way to add error detection to 4?

Look up the computed style for the pseudo and see if it is being animated? What's the use case where you want to do error detection on a if a pseudo is valid or not?

Given you can't set style on an arbitrary pseudo from JS presently with constructing a stylesheet, I wonder if being able to animate an arbitrary pseudo from Web Animations API is necessary. Can we make this read-only initially and then make it writable later?

[stephenmcgruer]

If we drew a parallel to getComputedStyle, then we should throw for text not prefixed with ':', and silently accept all other input. The intent from getComputedStyle appears to be that anything prefixed with a ':' is a pseudo-element, just maybe not one the browser does anything with. (See step 3.3, which just says "let obj be the given pseudo-element of elt", but this can be called for e.g. ::foo).

Of course, detecting support then becomes a problem; what if the web author wants to animate the ::first-line if possible, but fall back to animating the entire text if ::first-line isn't supported? To be fair, this problem exists today for pseudo-elements; it seems that most authors fall back to either inserting CSS and checking for the style applying, or calling insertRule and seeing if it throws (which I can't find the spec text for, but which apparently works). So perhaps we could say that it is reasonable for an author to do:

const div = document.createElement('div');
const anim = div.animate({ top: ['100px', '100px']}, { pseudoElement: myPseudoElement });
const supported = getComputedStyle(div, myPseudoElement).top === '100px';
div.remove();
return supported;

Given that, I think I'm coming down on 4. Console warning is an interesting question; I tend to lean away from adding them as each one lowers the chance that developers will actually pay attention to the console. But if a given browser felt it was important to warn the developer, they of course could do so?

Given you can't set style on an arbitrary pseudo from JS presently with constructing a stylesheet, I wonder if being able to animate an arbitrary pseudo from Web Animations API is necessary. Can we make this read-only initially and then make it writable later?

It feels weird to me to say that one of the goals of Web Animations is to give authors much finer control over their animations, and then say "but only for animations of non-pseudo elements".

[emilio]

Note that there are resolutions to get getComputedStyle() throw on unknown pseudos, though it may be too late for that. But I'd probably not emulate gCS in this case.

[stephenmcgruer]

Right, I think Amelia's comment here sums gCS up best: #3980 (comment) . However we don't have to worry about the compat questions, we 'just' have to get it right to avoid problems when a given pseudo-element is supported in some browsers but not others or when a new pseudo-element becomes supported in all browsers.

In what way do you think you wouldn't emulate gCS @emilio ?

[emilio]

In the "eating the error" when parsing a pseudo. I think throwing or other kind of error handling is fine :)

[george-steel]

So far in my pending implementation I'm doing as @stephenmcgruer suggests and throwing if it doesn't start with a colon, and eating the error otherwise. I think this will work at least as a transitional form. I share stephen's concerns about rolling out pseudo-elements (also blink's pseudo-element code is a bit flaky so this makes it merely fail to animate instead of crashing the page).

[birtles]

FWIW, we also need to fix the spec text for KeyframeEffectOptions.pseudoElement. It currently says,

The pseudo-element selector (which must be valid or null) used to specify the effect target given the target element.

but it doesn't make any sense to claim that the selector must be valid. It's a dictionary so the author can set it to whatever they like. If we want to restrict the set of valid values we need to define the behavior at the points where it is accepted. Presumably that behavior will match what we do here for the KeyframeEffect.pseudoElement setter (this issue).