/2dcontext/drawing-images-to-the-canvas/2d.drawImage.broken.html seems to have wrong expectations

[pylbrecht]

From the spec of drawImage():

2. Let usability be the result of checking the usability of image.
3. If usability is bad, then return (without drawing anything).

From the checking the usability of the image argument spec:

If image's current request's state is broken, then throw an "InvalidStateError" DOMException.

Currently 2d.drawImage.broken.html checks that drawImage() just returns and nothing is painted. To me it looks like the test should check that drawImage() raises an InvalidStateError.

References:

• drawImage() spec
• checking the usability of the image argument spec
• Make CanvasRenderingContext2D.createPattern() return null for incomplete images servo/servo#25710

[pylbrecht]

I'm also happy to open a PR for this, but first I want it to be clear that the test's expectation is indeed wrong.

[jdm]

Currently all browsers except Safari pass this test, which suggests that the spec should be updated to match the implementation behaviour.

[pylbrecht]

That would be the more appropriate thing to do then.

The spec <-> test combination is a little confusing, because the test for createPattern() expects an InvalidStateError to be raised in case of a broken image, although the spec links to the same steps as for drawImage(), which is expected to return without drawing anything. See servo/servo#25710 (comment) for reference.

[guest271314]

Why is an InvalidStateError expected for the image requested?

The height and width of the image are both greater than 0, 50 and 100, respectively.

Per https://html.spec.whatwg.org/multipage/images.html#current-request "Broken" is defined as follows

Broken
The user agent has obtained all of the image data that it can, but it cannot even decode the image enough to get the image dimensions (e.g. the image is corrupted, or the format is not supported, or no data could be obtained).

(Emphasis added).

While some image programs, e.g., ristretto image viewer, might have issues reading the PNG, Chromium 81 and Firefox 72 each are able to decode the image enough to get dimensions.

The request for the image itself should not cause any error or exception to be thrown.

[guest271314]

For reference this is what ristretto image viewer at *nix displays when the image is loaded:

Fatal error reading PNG image file: IDAT: CRC error

however, that reason does not appear to be a specified for an error or exception to be thrown per HTML Standard.

[guest271314]

This code does not throw an error or exception at Chromium or Firefox

let canvas = document.createElement("canvas");
canvas.width = 100;
canvas.height = 50;
canvas.style.border = "2px solid orange";
document.body.appendChild(canvas);
let ctx = canvas.getContext("2d");
ctx.drawImage(document.querySelector("img"), 0, 0);

This code will throw an exception

DOMException: "An attempt was made to use an object that is not, or is no longer, usable"
at both browsers

createImageBitmap(document.querySelector("img")).then(console.log, console.error);

yet creating an ImageBitmap of the canvas that the "broken" image was previously drawn onto does not throw an error or exception

createImageBitmap(document.querySelector("canvas")).then(console.log, console.error);
// ImageBitmap {width: 100, height: 50}

[pylbrecht]

Thanks for providing so much information and context. My research was obviously not very thorough. My apologies!
I based my conclusions on the fact that the spec of both createPattern() and drawImage() states that the same steps for checking the usability of the image argument should be performed, but their corresponding tests have different expectations:

• 2d.pattern.image.broken.html: expects an InvalidStateError to be raised
• 2d.drawImage.broken.html: expects drawImage() to return without drawing anything.

Is there something up with that or am I missing something?

[guest271314]

An InvalidStateError is not thrown at either Chromium 81 or Firefox 72 (will test at Nightly 74 shortly).

Would refer to the HTML Standard, where the description of "Broken" does not appear to be applicable to the image being used for the test.

Re the drawImage() test am not sure what is meant by

return without drawing anything

?

Some of the tests re canvas and OffscreenCanvas contain references to exceptions that do not exist in the current specification. See #20886 (comment), where "INDEX_SIZE_ERR" is expected though that is not in the current iteration of HTML Standard, rather "IndexSizeError" is in the current specification.

Also, would carefully review the _assertPixelApprox function; what parameters are passed, how the arguments are used within the body of the function, along with _getPixels function #20886 (comment).

For what specific reason is a InvalidStateError expected for the 2d.pattern.image.broken test?

[guest271314]

No exception is thrown at Nightly 74 for the same code.

Perhaps the image is not "broken" enough within the context of current implementations' capability to decode "broken.png"?

[pylbrecht]

Re the drawImage() test am not sure what is meant by

return without drawing anything

?

From drawImage() spec:

1. If any of the arguments are infinite or NaN, then return.
2. Let usability be the result of checking the usability of image.
3. If usability is bad, then return (without drawing anything).

Since

[...] the description of "Broken" does not appear to be applicable to the image being used for the test.

I agree that 2d.drawImage.broken.html is correct. I'm still puzzled about 2d.pattern.image.broken.html though.

For what specific reason is a InvalidStateError expected for the 2d.pattern.image.broken test?

My guess is due to this statement from checking the usability of image:

If image's current request's state is broken, then throw an "InvalidStateError" DOMException.

But since the image is not "broken" in the sense of the HTML standard, this test should not expect an InvalidStateError to be raised.

[pylbrecht]

No exception is thrown at Nightly 74 for the same code.

Perhaps the image is not "broken" enough within the context of current implementations' capability to decode "broken.png"?

I agree on this. But then I would not expect 2d.pattern.image.broken.html to expect an InvalidStateError either, yet it does (https://github.com/web-platform-tests/wpt/blob/master/2dcontext/fill-and-stroke-styles/2d.pattern.image.broken.html#L23).

[guest271314]

But since the image is not "broken" in the sense of the HTML standard, this test should not expect an InvalidStateError to be raised.

Since

but it cannot even decode the image enough to get the image dimensions

is not applicable to the image being used, at least not with regards to createPattern() as evidenced by the result.

The tests at WPT are not fail-safe in and of themselves, do not spontaneously update corresponding to updates to specifications or implementations. Therefore, here, do not expect any test to be absolutely accurate.

Either an image needs to be used that is "Broken" per the specification or standard being relied on, or the test need to be modified to suit what you are expecting.

[pylbrecht]

The tests at WPT are not fail-safe in and of themselves, do not spontaneously update corresponding to updates to specifications or implementations. Therefore, here, do not expect any test to be absolutely accurate.

Alright, thanks for clarifying!

Either an image needs to be used that is "Broken" per the specification or standard being relied on, or the test need to be modified to suit what you are expecting.

Meaning the modifications should be done in a fork and not upstream?

[guest271314]

Yes.

Would first clearly define what is expected (pass or faile), and why.

Then create various images and test the input and output locally.

Then create a branch corresponding to what is actually in the current specification or standard relied on. During the composition of the tests, reviewing each result, it could be the case that a given specification might need to be updated. Or, you might be able to create objects which do in fact meet the existing published criteria. There are no guarantees that either the specification is in accord with current implementations or that implementations are in accord with specifications. As an example, testing resizing of <video> elements during playback #19030 corresponding to the underlying encoded frames found several bugs in Chromium implementation. Those "bugs" were implementation decisions that had the unintended consequences of crashing the browser https://github.com/guest271314/MediaFragmentRecorder/tree/chromium_crashes when several different configurations and API's were used.

[pylbrecht]

Alright, I see. Thank you so much for your patience and your input. I learned a lot of new things through this!
I'm closing this issue, since there's nothing wrong with 2d.drawImage.broken.html and 2d.pattern.image.broken.html needs a lot more investigation before drawing any backed up conclusions.