Handling of non-finite floats in canvas APIs is even more confusing now

[bzbarsky]

https://html.spec.whatwg.org/multipage/scripting.html#2dcontext says (well hidden; search for it):

Except where otherwise specified, for the 2D context interface, any method call with a numeric argument whose value is infinite or a NaN value must be ignored.

This doesn't actually specify a processing model very well (e.g. does the ignoring happen as soon as you find such an argument, as basically a monkeypatch to IDL processing, or does it happen as part of the algorithm steps after all argument conversions are done?). But that's a longstanding issue.

What's worse is that the refactor into partial interfaces makes it very unclear how this works. A simple example, quoting the spec:

[NoInterfaceObject]
interface CanvasPath {
...
  void moveTo(unrestricted double x, unrestricted double y);
};

CanvasRenderingContext2D implements CanvasPath;
Path2D implements CanvasPath;

When moveTo is invoked on a Path2D object with non-finite arguments, what happens and why? Note that Path2D is not "the 2D context interface".

[annevk]

We should really inline this requirement.

[junov]

Agreed, this needs to be clarified and re-organized. I always interpreted this as meaning that the args are like restricted args that abort instead of throwing. I will have to run some tests on various implementations to very whether that interpretation is interoperable.

[bzbarsky]

Testcase:

<script>
var c = document.createElement("canvas").getContext("2d");
c.moveTo(0, { valueOf: function() { alert("called for finite double"); } });
c.moveTo(NaN, { valueOf: function() { alert("called for NaN"); } });
</script>

Results:

• Chrome dev: shows both alerts
• Firefox nightly: shows only the first alert
• Safari 9: shows both alerts
• WebKit nightly: shows both alerts
• Edge 14: shows both alerts

That shows that everyone except Firefox treats this as an algorithm-steps-level requirement. Firefox treats it the way @junov describes: just like restricted args that immediately return instead of throwing. In particular, Firefox has a WebIDL extension it uses for this case, so this is done as part of normal argument processing (aka the "monkeypatch" approach I mention above).

[junov]

For the longest time I knew blink and webkit implemented the abort downstream of the bindings without ever giving much thought to this script-observable side-effect. I've never seen any bug reports about this interop issue, so I am not too worried about braking the web by going either way. So the question is, what is the right thing to do here?

The idea of WebIDL extension is appealing. Would it make sense to have something like that in the spec?

[annevk]

It's probably best to add that to IDL directly. And that's definitely reasonable.

[domenic]

Hmm, I tend to disagree that it's reasonable to add to IDL. It'd be a kind of strange unprecedented type conversion, which aborts the subsequent algorithm without throwing an exception. Just inlining the steps into the start of each method makes more sense to me.

[bzbarsky]

Fwiw, I agree that aligning on either behavior would likely be web-compatible, and would be fine with adding something to IDL if others want to move in that direction too. // cc @cdumez

What Gecko implements is an annotation on the whole method that says "any restricted float/double arg that's not finite means immediate return". This only applies to arguments whose type is float/double or nullable versions of those, or a sequence of any of those (e.g. sequence<float?>). The sequence thing is needed for setLineDash. I suspect we also make it work for sequence<sequence<float>> by accident, now that I think of it....

In any case, we don't have anything along these lines for unions or dictionary members or anything like that. If we do add something like this to IDL, I would prefer it be very scoped down so it only needs to be supported in the cases where the legacy canvas stuff needs it (e.g. it should not be a new type usable anywhere).

[cdumez]

WebKit (and likely Blink) do an early return in the implementation methods if the input value is non-finite. I personally prefer this compared to adding a Web IDL extension.

[junov]

Alright then. The early exit in the steps is fine by me. In terms of spec readability, I was thinking that each affected method should have an explicitly documented early exit step, as opposed to the kind of blanket statement we have now, which is error prone.

[bzbarsky]

That sounds fine to me too. I'm fairly certain it's not hard to do that in Gecko without changing much (i.e. instead of early returning in the binding set a flag and then right before calling out of the binding check the flag). Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1303099 on that.

[bzbarsky]

Gecko patch to align on the behavior everyone else has landed.