Skip to content

Commit

Permalink
Fix .complete for errored images
Browse files Browse the repository at this point in the history
According to the HTML specification [1], HTMLImageElement#complete
indicates whether or not an image is loading. It does not indicate
whether or not the image successfully loaded.

When an Image fails to load, we call the onerror callback. However,
Image#complete is false, despite the current request's state being
'broken' (according to the spec). This is not spec-compliant.

Because our Image implementation loads images synchronously (as soon as
the src property is set, which is not spec-compliant), then the current
request's state is only ever 'completely available' or 'broken' [2] (if
src is set). This means that, according to the spec, Image#complete must
always be true. Fix Image#complete to return true unconditionally.

[1] https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-complete
[2] https://html.spec.whatwg.org/multipage/images.html#img-req-state
  • Loading branch information
strager authored and zbjornson committed Jun 5, 2020
1 parent f13efc7 commit 3e80556
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ project adheres to [Semantic Versioning](http://semver.org/).
* Fix signed/unsigned comparison warning introduced in 2.6.0, and function cast warnings with GCC8+
* Fix to compile without JPEG support (#1593).
* Fix compile errors with cairo
* Fix Image#complete if the image failed to load.

2.6.1
==================
Expand Down
3 changes: 1 addition & 2 deletions src/Image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ NAN_METHOD(Image::New) {
*/

NAN_GETTER(Image::GetComplete) {
Image *img = Nan::ObjectWrap::Unwrap<Image>(info.This());
info.GetReturnValue().Set(Nan::New<Boolean>(Image::COMPLETE == img->state));
info.GetReturnValue().Set(Nan::New<Boolean>(true));
}

/*
Expand Down
8 changes: 7 additions & 1 deletion test/image.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ describe('Image', function () {
it('detects invalid PNG', function (done) {
if (process.platform === 'win32') this.skip(); // TODO
const img = new Image()
img.onerror = () => done()
img.onerror = () => {
assert.strictEqual(img.complete, true)
done()
}
img.src = Buffer.from('89504E470D', 'hex')
})

Expand Down Expand Up @@ -156,6 +159,7 @@ describe('Image', function () {
assert.equal(err.code, 'ENOENT')
assert.equal(err.path, 'path/to/nothing')
assert.equal(err.syscall, 'fopen')
assert.strictEqual(img.complete, true)
done()
}
img.src = 'path/to/nothing'
Expand All @@ -165,6 +169,7 @@ describe('Image', function () {
const img = new Image()
img.onerror = err => {
assert.equal(err.message, "JPEG datastream contains no image")
assert.strictEqual(img.complete, true)
done()
}
img.src = `${__dirname}/fixtures/159-crash1.jpg`
Expand Down Expand Up @@ -218,6 +223,7 @@ describe('Image', function () {
img.src = Buffer.alloc(0)
assert.strictEqual(img.width, 0)
assert.strictEqual(img.height, 0)
assert.strictEqual(img.complete, true)

assert.strictEqual(onerrorCalled, 1)
})
Expand Down

0 comments on commit 3e80556

Please sign in to comment.