Fix several bugs in <img>.complete

In particular, now: * complete returns true if src="" is unset or empty. * complete returns true if an error occurs loading the image. * complete returns false if src="" is reset to a not-yet-loaded URL. Closes #2967 by superseding it. Co-authored-by: Matthew Glazar <strager.nds@gmail.com>
jsdom · Aug 8, 2020 · 3802e13 · 3802e13
1 parent bfa4902
commit 3802e13
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 5 deletions.
diff --git a/lib/jsdom/living/nodes/HTMLImageElement-impl.js b/lib/jsdom/living/nodes/HTMLImageElement-impl.js
@@ -6,6 +6,11 @@ const { Canvas } = require("../../utils");
 const { parseURLToResultingURLRecord } = require("../helpers/document-base-url");
 
 class HTMLImageElementImpl extends HTMLElementImpl {
+  constructor(...args) {
+    super(...args);
+    this._currentRequestState = "unavailable";
+  }
+
   _attrModified(name, value, oldVal) {
     // TODO: handle crossorigin
     if (name === "src" || ((name === "srcset" || name === "width" || name === "sizes") && value !== oldVal)) {
@@ -50,7 +55,11 @@ class HTMLImageElementImpl extends HTMLElementImpl {
   }
 
   get complete() {
-    return Boolean(this._image && this._image.complete);
+    const srcAttributeValue = this.getAttributeNS(null, "src");
+    return srcAttributeValue === null ||
+      srcAttributeValue === "" ||
+      this._currentRequestState === "broken" ||
+      this._currentRequestState === "completely available";
   }
 
   get currentSrc() {
@@ -73,6 +82,7 @@ class HTMLImageElementImpl extends HTMLElementImpl {
       this._image = new Canvas.Image();
     }
     this._currentSrc = null;
+    this._currentRequestState = "unavailable";
     const srcAttributeValue = this.getAttributeNS(null, "src");
     let urlString = null;
     if (srcAttributeValue !== null && srcAttributeValue !== "") {
@@ -101,11 +111,15 @@ class HTMLImageElementImpl extends HTMLElementImpl {
           throw new Error(error);
         }
         this._currentSrc = srcAttributeValue;
+        this._currentRequestState = "completely available";
       };
 
       request = resourceLoader.fetch(urlString, {
         element: this,
-        onLoad: onLoadImage
+        onLoad: onLoadImage,
+        onError: () => {
+          this._currentRequestState = "broken";
+        }
       });
     } else {
       this._image.src = "";

diff --git a/test/to-port-to-wpts/htmlimageelement.js b/test/to-port-to-wpts/htmlimageelement.js
@@ -33,10 +33,11 @@ describe("htmlimageelement", { skipIfBrowser: true }, () => {
     assert.strictEqual(image.height, 0, "before loading, height should be 0");
     assert.strictEqual(image.naturalWidth, 0, "before loading, naturalWidth should be 0");
     assert.strictEqual(image.naturalHeight, 0, "before loading, naturalHeight should be 0");
-    assert.strictEqual(image.complete, false, "before loading, complete should be false");
+    assert.strictEqual(image.complete, true, "before loading or setting src, complete should be true");
     assert.strictEqual(image.src, "", "before loading, src should be an empty string");
     assert.strictEqual(image.currentSrc, "", "before loading, currentSrc should be an empty string");
     image.src = src;
+    assert.strictEqual(image.complete, false, "before loading and after setting src, complete should be false");
     image.onload = () => {
       assert.ok(true, "onload should be triggered when loading from valid URL.");
       assert.strictEqual(image.width, 168, "after loading, width should be 168");

diff --git a/test/web-platform-tests/to-run.yaml b/test/web-platform-tests/to-run.yaml
@@ -633,7 +633,7 @@ remove-element-and-scroll.html: [timeout, scrollIntoView not implemented]
 sizes/**: [timeout, Unimplemented]
 srcset/**: [timeout, Unimplemented]
 update-media.html: [timeout, Unimplemented]
-update-src-complete.html: [timeout, Unknown]
+update-src-complete.html: [needs-canvas]
 update-the-image-data/fail-to-resolve.html: [timeout, Resource loader doesn't catch bad URLs at the right point in the process]
 update-the-source-set.html: [timeout, Unimplemented]
 usemap-casing.html: [fail-with-canvas, Can't seem to handle onload = () => ...]

diff --git a/...orm-tests/to-upstream/html/semantics/embedded-content/the-img-element/img.complete-2.html b/...orm-tests/to-upstream/html/semantics/embedded-content/the-img-element/img.complete-2.html
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>HTMLImageElement#complete</title>
+<link rel="help" href="https://html.spec.whatwg.org/#the-img-element">
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+"use strict";
+
+async_test(t => {
+  const img = document.createElement("img");
+  img.src = "file-does-not-exist.png";
+  assert_false(img.complete);
+
+  img.onload = img.onerror = t.step_func_done(event => {
+    assert_equals(event.type, "error");
+    assert_true(img.complete);
+  });
+
+  // JSDOM specific: when image loading is not supported (i.e. when node-canvas is not installed), skip the test.
+  t.step_timeout(() => {
+    t.done();
+  }, 1000);
+}, "src 404 error");
+
+async_test(t => {
+  const img = document.createElement("img");
+  img.src = "http://malformed:url";
+  assert_false(img.complete);
+
+  img.onload = img.onerror = t.step_func_done(event => {
+    assert_equals(event.type, "error");
+    assert_true(img.complete);
+  });
+
+  // JSDOM specific: when image loading is not supported (i.e. when node-canvas is not installed), skip the test.
+  t.step_timeout(() => {
+    t.done();
+  }, 1000);
+}, "src malformed URL");
+</script>
diff --git a/...sts/to-upstream/html/semantics/embedded-content/the-img-element/remove-attribute-src.html b/...sts/to-upstream/html/semantics/embedded-content/the-img-element/remove-attribute-src.html
@@ -19,7 +19,7 @@
 
     assert_equals(image.width, 0);
     assert_equals(image.height, 0);
-    assert_equals(image.complete, false);
+    assert_equals(image.complete, true);
     assert_equals(image.src, "");
     assert_equals(image.currentSrc, "");