Closed Bug 608261 Opened 14 years ago Closed 9 years ago

'complete' property stays true when image src changes

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla38
Tracking Flags:
Tracking Status
firefox38 --- fixed

People

(Reporter: skonstantinov, Assigned: bzbarsky)

References

Details

(Keywords: testcase)

Attachments

(2 files)

testcase
513 bytes, text/html
Details
Set img.complete to false while the image has a pending request
2.70 KB, patch
sicking
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b6) Gecko/20100101 Firefox/4.0b6
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b6) Gecko/20100101 Firefox/4.0b6

'complete' property stays true when image src changes

Reproducible: Always

Steps to Reproduce:
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
    <title>test</title>
    <script type="text/javascript">
		window.onload = function () {
			var img = document.getElementById('image');
			
			img.src = 'http://www.mozilla.org/images/home/projects/thunderbird.png';
			alert(img.complete);
		}
    </script>
</head>

<body>
	<img src="http://www.mozilla.org/images/home/projects/firefox.png" id="image"/>
</body>

</html>
Actual Results:  
img.complete == true

Expected Results:  
img.complete == false

FF3 & FF4 beta affected
Status: UNCONFIRMED → NEW
Component: DOM → DOM: Core & HTML
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Attached file testcase — 
Returning true might be correct if the image was cached I guess.

Opera returns 'false' when the image isn't cached and 'true' otherwise.
Keywords: testcase
(In reply to comment #1)
> Opera returns 'false' when the image isn't cached and 'true' otherwise.

Webkit has the same behavior than Opera (Safari 5 and Chrome 8).
When you change an image via .src, we continue painting the old image until the new one is fully loaded.  During this time, .complete reports the state of the _old_ image, as does any other information you query.

This sounds like a spec issue to be raised.
In particular, we'd be painting the old image on screen; if complete returned false then the canvas and screen behavior would not be consistent.  Something to at least keep in mind when speccing this.
'complete' field in Opera, Webkit indicates that image was fetched from cache and no onload event will be fired. FF behavior differs: FF fills both 'complete' field and 'onload' events. It seems confusing.
Again, in Firefox "complete" in the situation in this bug is referring to the already-loaded image, not the newly-loading one.

So the real problem here is that there are two images in play, but only one boolean, and no spec that says which of the images the boolean applies to.  This really needs to be sorted out in HTML5.

And in particular, if you hit the stop button during the new load, last I checked we just keep showing the old image.  I don't know what opera/webkit do.

Not firing the onload event there is definitely a bug per the html5 spec, last I checked.
A few thoughts...

It seems to me that attributes like img.complete are intended for (and only useful for) indicating the current state of an element.  Not for indicating some historical state from a previous version of the element.

I expect the attributes of an element to be consistent with one another.  If at any given time, a script reads both img.complete and img.src, and one has been updated to reflect the latest changes to the element but the other has not, the two values might as well have been read from two completely separate elements.  This seems broken to me.

I can't think of any need for leaving img.complete in its previous state after a script changes img.src.  If a script wants to know the previous state, it is perfectly capable of saving that state before it makes a change.  On the other hand, there is absolutely a need to expose the current "complete" state.  A script is pretty much helpless if it wants to know the current state and the DOM won't reveal it.

My reading of the spec seems to support updating img.complete when img.src is changed, since img.complete is defined in terms of img.src (and tasks triggered by img.src):
http://dev.w3.org/html5/spec/embedded-content-1.html#dom-img-complete
So you don't have a problem with the image width and height coming from the "old" image while the new one is loading?  Or with the fact that if you paint it to a canvas the old image will be painted (well, unless complete gets set to false, in which case nothing will be painted at all).

The spec is not compatible with either Gecko, Presto, or WebKit.

It's not even clear that the spec is compatible with the web, given that.  So it might well have to change.  Someone who cares about this issue should probably spend some time investigating what behavior here the web does or does not depend on...
(In reply to Boris Zbarsky (:bz) from comment #8)
> So you don't have a problem with the image width and height coming from the
> "old" image while the new one is loading?

I don't remember suggesting any such thing.

> It's not even clear that the spec is compatible with the web, given that.

That may be true.  I'm not trying to excuse the spec.  I'm just offering some thoughts on what behavior could be more useful than the current behavior.
OK.  Well, web compat requires that when src is changed the old image continue showing until the new one is done loading, last I checked.  So that's a given.  That means having the width and height of the old image, etc.

Now given that, what is the sanest behavior for .complete?
I would think the sanest behavior would be to keep interdependent attributes consistent with one another.  That is, if the value of attribute A is defined as depending on the value of attribute B, and a script changes B then reads A, the only sane expectation I can imagine is that A reflect that change.  I suppose that could be the newly-calculated value, or whatever placeholder value normally lives in A until it can be calculated.  Allowing the script to retrieve a stale value that has nothing to do with B, when it is specifically supposed to depend on B, seems like lying to the script and the programmer.  I don't see any value in that.

After changing img.src, I think I expected img.complete, img.width, and img.height, to be false, 0, and 0, until the final values could be determined.

(Of course, in suggesting this, I have to consider whether it would break some app somewhere.  I don't know the answer off the top of my head, but WebKit appears to use this beavior, so the fact that most web apps load images just fine with it would seem to indicate that it's relatively safe.)
> After changing img.src, I think I expected img.complete, img.width, and img.height, to be
> false, 0, and 0,

Those last two are certainly not web-compatible last I checked...  It's interesting that WebKit makes those be the case.  Does it also collapse the image in the display, or does it just make width/height not reflect the actual size of the image?
I really would like to see this fixed and maybe also to block the respimg pref on. In all tested browsers (IE, Safari, Chrome, Opera) the complete property is set to false if a new, not cached src is added, only FF behaves differently. 

For this reason developers are using the following workaround to check wether an image is loaded:


function isLoaded(img, cb){
    var nImg = document.createElement('img');
    var onLoad = function(){
      cb();
      nImg.onload = null;
      nImg.onerror == null;
      nImg = null;
      img = null;
    };
    nImg.onload = onLoad;
    nImg.onerror = onLoad;
    nImg.src = img.src;
    if(nImg && nImg.complete){
        onLoad();
    }
}

A popular script, that uses this pattern can be seen here: https://github.com/desandro/imagesloaded

But there are a lot. Know with responsive image the thing becomes even more bad. Do we really need to rebuild the hole picture markup only to check know wether an image is already loaded or will be loaded?

Please fix this.
This is really a spec-level question.  In the current spec world where there is a pending and current request, how exactly should "complete" be defined?  That's what comment 10 was about.

This is the biggest problem to fixing this: what should the behavior actually be?  I'd really rather not change behavior multiple times back and forth here...

I filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=27484 to track this.  Jonas, how do you feel about the proposal there of just making complete be false if there is a pending request?
Flags: needinfo?(jonas)
@Boris
Just wanted say thanks for being so quick. (and sorry for my english)
I believe the behavior should be this: Set img.complete to false when img.src is changed, and set it to true again according to the spec's requirements.

http://www.w3.org/html/wg/drafts/html/master/embedded-content.html#dom-img-complete

The only argument I've seen against this has to do with the state of pixels on the display, between the time when img.src has changed and the time when the new image has fully loaded. However, there is (as far as I can see) no requirement that img.complete represent those pixels. It is defined in terms of the img.src attribute and the networking tasks triggered by it. Even that flimsy argument makes no sense at all when the img is hidden, as is common when javascript code is waiting for img.complete to become true before it makes the image visible (e.g. for a fade-in effect).
HTML5 was released in the four years since I filed this bug report. Here's an updated link:
http://www.w3.org/TR/html5/embedded-content-0.html#dom-img-complete
Having .complete return false any time there's a pending request sounds good to me.

So if you do

myimageElement.src = x;
console.log(myimageElement.complete);

then the second line would log 'false' false no matter what state the element was in before the above line was executed. Only exception would be if the image source is in the per-document image cache. I.e. if the image has previously been loaded into this document.

Does that make sense? Or is that too strict and there are other situations when we can let .complete be true? Like if it's cached in some other in-memory cache?
Flags: needinfo?(jonas)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/1b6bfd97153f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
See Also: → 1304663
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: