whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.17k stars 2.69k forks source link

Contradiction in CanvasRenderingContext2D.createPattern for images with bad useability #8149

Closed mysteryDate closed 2 years ago

mysteryDate commented 2 years ago

According to https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-createpattern:

  1. Let usability be the result of checking the usability of image.
  2. If usability is bad, then return null.
  3. Assert: usability is good.

Some tests in wpt explicitly check for this behavior:

https://github.com/web-platform-tests/wpt/blob/master/html/canvas/element/fill-and-stroke-styles/2d.pattern.image.zerowidth.html

However, in checking the usability of image, it states:

HTMLCanvasElement: OffscreenCanvas: If image has either a horizontal dimension or a vertical dimension equal to zero, then throw an "InvalidStateError" DOMException.

The issue is that once "checking the usability" throws the DOMException js execution stops and we cannot return null. I vote we just take out step 2 from create pattern and not expect it to return null and just keep the error messages. If Assert: usability is good becomes step 2 then it seems to me like everything will work fine.

Kaiido commented 2 years ago

But this particular test is against HTMLOrSVGImageElement's third step:

If image has an intrinsic width or intrinsic height (or both) equal to zero, then return bad.

Passing a zero width HTMLCanvasElement does throw as specced in every UA. Firefox and Chrome also do throw on broken images (though they seem to disagree on what makes a broken image...)

A few quick tests: https://jsfiddle.net/6wn1rof3/

I'm not sure why the specs need to be changed here? But maybe I'm missing something.

mysteryDate commented 2 years ago

But this particular test is against HTMLOrSVGImageElement's third step:

You mean this test? https://github.com/web-platform-tests/wpt/blob/master/html/canvas/element/fill-and-stroke-styles/2d.pattern.image.zerowidth.html#L23

It's checking that the result is null, not that an error is thrown. As I read https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-createpattern it certainly looks like null is the expected result. We can't both return null and throw an error, right?

Kaiido commented 2 years ago

I think the confusion is that In this test an HTMLImageElement is passed to createPattern(), not an HTMLCanvasElement.
This HTMLImageElement has its width set to 0. So when we enter check the usability of the image argument, in the first step we get in the first bullet HTMLOrSVGImageElement.
There we don't meet the first point, our image request isn't broken, so we don't throw.
We're not either in the second one, the image is fully decodable.
However we do hit the third one, we have a zero width.
So bad is returned from this algorithm.
Back in createPattern(), we see that usability is bad, we return null.

mysteryDate commented 2 years ago

Oh, of course 🤦‍♀️. I got confused by chromium code that was conflating the different sources for patterns. Thanks for your explanation.

So for createPattern, if the source is a zero width canvas element checking the useability will throw and we will not get a null for the pattern and that's fine?

Kaiido commented 2 years ago

So for createPattern, if the source is a zero width canvas element checking the useability will throw and we will not get a null for the pattern

Yes, that's what happens in every UA and what is in the specs.

and that's fine?

I don't have much to say on this, but yes I guess it's fine. It's anyway throwing or getting null. I suppose that treating a zero width canvas differently than a zero width <img> may be argued over, but I think it's like this since a long time, and authors have somehow more control over a canvas than over what's in an <img>.