Fix #864: don't reject createImageBitmap's promise with null #884
Conversation
| @@ -90531,7 +90540,8 @@ interface <dfn>ImageBitmapFactories</dfn> { | |||
|
|
|||
| <li><p>If the image data is not in a supported file format (e.g. it's not actually an image at | |||
| all), or if the image data is corrupted in some fatal way such that the image dimensions cannot | |||
| be obtained, then reject the promise with null, and abort these steps.</p></li> | |||
| be obtained, then reject the promise with an <code>EncodingError</code>, and abort these | |||
| steps.</p></li> | |||
There was a problem hiding this comment.
EncodingError was originally for the Encoding Standard's API, but I guess it's fine to use it for other things.
There was a problem hiding this comment.
Yeah, "The encoding operation (either encoded or decoding) failed" seemed general enough.
|
The two exceptions should be followed by " exception" I think. |
domenic
force-pushed
the
createimagebitmap-promise
branch
from
520e698
to
99c6b6e
Compare
|
Fixed and also fixed a number of other places my regexp found. |
|
Testing in Firefox they don't reject with null, but instead with one of those weird nonstandard Firefox exceptions: NS_ERROR_NOT_AVAILABLE. |
| @@ -90484,7 +90493,8 @@ interface <dfn>ImageBitmapFactories</dfn> { | |||
|
|
|||
| <li><p>If the image data is not in a supported file format (e.g. it's not actually an image at | |||
| all), or if the image data is corrupted in some fatal way such that the image dimensions cannot | |||
| be obtained, then reject the promise with null, and abort these steps.</p></li> | |||
| be obtained, then reject the promise with an <code>EncodingError</code> exception, and abort | |||
There was a problem hiding this comment.
I think it would make sense to have more consistency between the handling of blobs and img elements (described above). Currently, with an img, when we encounter a file that is decodable but does not represent bitmap media, an InvalidStateError is thrown. Suggestion: perhaps we should separate these two cases: unsupported media type (e.g. vector graphics) -> InvalidStateError; undecodable media -> EncodingError
domenic
force-pushed
the
createimagebitmap-promise
branch
from
99c6b6e
to
f11dee4
Compare
|
@junov rebased and separated out the two conditions. |
| graphic or it's not an image at all), then reject the promise with an | ||
| <code>InvalidStateError</code> exception, and abort these steps.</p></li> | ||
|
|
||
| <li><p>If the image data is corrupted in some fatal way such that the image dimensions cannot |
There was a problem hiding this comment.
In the section "If image is an img element", should we have something equivalent to this? Example: If the img is in the broken state (data-x="img-error"), the promise should be rejected with an EncodingError exception.
Right now, the the broken state case gets handled by a catch-all clause for handling imgs that are not in the completely available state. It rejects with an InvalidStateError.
There was a problem hiding this comment.
Hmm. But won't broken also be the case for e.g. <img src="http://non-existent.com/non-existent-url">?
Maybe we should just collapse these all into InvalidStateError. (Including both of my newly-introduced NotReadableError and EncodingError instances.) I don't anticipate code being able to usefully branch on the type of the resulting error, and that seems simpler.
|
@junov pushed two commits representing two possibilities:
|
|
IMHO, not rejecting with null and using more specific exceptions are two separate issues, which should perhaps be dealt with separately. If we start using EncodingError in this spec, we should probably use it everywhere where it makes sense, and not just in the place we're looking at right now. WDYT? |
|
That makes sense to me, and is probably a better way of phrasing my misgivings. So you think that the diff master...132cf49 which just switches from null to InvalidStateError is good? |
|
yep, lgtm |
|
Great! Will let @annevk give it a look-over to be sure. |
As a drive-by fix, be more consistent about suffixing exception names.
| occurs during reading of the object</span>, then reject the promise with null, and abort these | ||
| steps.</p></li> | ||
| occurs during reading of the object</span>, then reject the promise with a | ||
| <code>InvalidStateError</code> exception, and abort these steps.</p></li> |
annevk
force-pushed
the
createimagebitmap-promise
branch
from
132cf49
to
d09b045
Compare
I wonder which version you've tested. @domenic so, please use Nightly ;) |
We should file bugs (and ideally web-platform-tests) for this.
/cc @junov @xidachen