[web-animations-1] Fix invalid error code for KeyfameEffect.pseudoElement

[george-steel]

Since TypeError is not a valid DOMException error name, change it to SyntaxError (the code for this kind of type error).

[george-steel]

@stephenmcgruer

[stephenmcgruer]

I'm OOO currently; can one of the other editors take a look please?

[birtles]

I'm not sure. I think we're trying to match WebIDL semantics here which throws a JS TypeError for an invalid enum value, for example.

[george-steel]

There's definitely a case for a JS TypeError. Are there any other examples of it being used for an invalid value outside an infinite family of strings (SyntaxError seems to be used here a lot), or just for values outside finite enums? pseudoElement can take any syntactically valid pseudo-element selector (such as ::foo) whether the selector is supported or not.

[birtles]

There's definitely a case for a JS TypeError. Are there any other examples of it being used for an invalid value outside an infinite family of strings (SyntaxError seems to be used here a lot), or just for values outside finite enums? pseudoElement can take any syntactically valid pseudo-element selector (such as ::foo) whether the selector is supported or not.

One example is that in the process a keyframes argument procedure right towards the end we have:

If parsing the “easing” property fails, throw a TypeError and abort this procedure.

So at least within Web Animations, we currently always use a TypeError. I don't recall exactly how we ended up there. There was certainly at least one case where we were deciding between SyntaxError or TypeError and the advice we got was something along the lines of, "We normally use TypeError, but it doesn't really matter". Maybe @heycam recalls?

[george-steel]

To me it seems like either will work. When i asked @stephenmcgruer, he referenced me to the DOMException part fo the WebIDL spec defining

"SyntaxError" DOMException is used to report parsing errors in web APIs, for example when parsing selectors. … The string did not match the expected pattern.

[birtles]

Oh, that seems pretty clear then!

[birtles]

That makes me think we should update the parsing of easing to throw a SyntaxError too. Mind fixing that too?

[stephenmcgruer]

That makes me think we should update the parsing of easing to throw a SyntaxError too. Mind fixing that too?

There is the potential for web compat issues there (slight since its still an exception, but technically authors could be catching TypeErrors specifically currently), right? I think it's probably still reasonable, but just something to think about.

[birtles]

There is the potential for web compat issues there

Yes, that's right. I might be being a bit optimistic that no-one is watching specifically for TypeErrors from those procedures.

[george-steel]

It looks looks there are a lot more places (including all of keyframe parsing) besides parsing easing that uses TypeError for infinite families of values instead of the more specific DOMException error types (SyntaxError, RangeError, etc). I now think that this should make the error for pseudo-element selectors a TypeError for consistency and that changing them to the more specific codes should be for another issue.

[george-steel]

@birtles @stephenmcgruer what do you think?

[birtles]

I had a quick skim through the spec yesterday and the only two places where I thought SyntaxError would make sense were pseudoElement and easing (although the easing case appears in about 4~5 different places). Many of the others were more enum-like cases where a TypeError seems to match WebIDL best.

Regarding RangeError, I think there's an explicit note in the spec about why we don't use that for at least one case (since if [EnforceRange] is ever able to handle that case, it will throw a TypeError).

So I would be happy with making pseudoElement throw a SyntaxError and then filing a second issue to make easing throw a SyntaxError if we think Web compat won't be a problem.

But all in all, I don't mind too much either way.

[george-steel]

Sounds good to me. Patch is ready on my end.