Fix several HTMLImageElement#complete bugs #2967
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
strager
force-pushed
the
img-complete
branch
2 times, most recently
from
07cccad
to
4f01e47
Compare
jsdom implements HTMLImageElement#complete poorly. I noticed this when I observed .complete giving false despite the onerror callback being called. To address this, perform the following changes: * Make .complete true if src is unset or is empty. * Make .complete true if an error occurs loading the image. * Make .complete false if .src is reset to an unloaded URL. This commit fixes most (but not all) test cases in html/semantics/embedded-content/the-img-element/img.complete.html. This commit does not address srcset or the image list.
domenic
requested changes
Jul 3, 2020
| @@ -5,7 +5,16 @@ const HTMLElementImpl = require("./HTMLElement-impl").implementation; | |||
| const { Canvas } = require("../../utils"); | |||
| const { parseURLToResultingURLRecord } = require("../helpers/document-base-url"); | |||
|
|
|||
| const REQUEST_STATE_BROKEN = "broken"; | |||
| const REQUEST_STATE_COMPLETELY_AVAILABLE = "completely available"; | |||
| const REQUEST_STATE_UNAVAILABLE = "unavailable"; | |||
There was a problem hiding this comment.
Let's eliminate these and just use the strings directly.
| @@ -0,0 +1,43 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
Can you name this -dont-upstream.html since it's a copy of an existing file and we need to remember not to upstream it?
There was a problem hiding this comment.
Hmm I see it's not a copy of an existing file... I'll give this some thought.
|
I'm trying to push fixes to this branch but I cannot do so. Can you ensure that "Allow edits from maintainers" is checked, per https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork ? |
domenic
added a commit
that referenced
this pull request
Aug 8, 2020
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
jsdom implements HTMLImageElement#complete poorly. I noticed this when I observed .complete giving false despite the onerror callback being called. To address this, perform the following changes:
This commit fixes most (but not all) test cases in html/semantics/embedded-content/the-img-element/img.complete.html. This commit does not address srcset or the image list.