CSS-Tricks/MovingBoxes

Memory leak if one of the images is not loaded

d-u-a-l opened this issue · 9 comments

It's in base.imagesLoaded function

ic = ('fileSize' in t[i] && t[i].fileSize < 0 && t[i].count > 10) ? true : t[i].complete;

must be like

ic = (('fileSize' in t[i] && t[i].fileSize < 0) || t[i].count > 10) ? true : t[i].complete;

because in your case ic would never be true, even after 10 iterations it would be t[i].complete, which is false, because img is not loaded

Hi @d-u-a-l!

Were you experiencing a problem with this?

IE is actually the only browser that would set the image complete property to false, other browsers would set it to true even if the image failed to load. Also, IE is the only browser with a fileSize property, and it returns -1 until the file has actually loaded.

Try it yourself with this demo. Click the "Load Images" button, then click on any image and it will show the image properties of complete, fileSize and count in the console.

Tried this just now - Opera does not set property to true, if this image is not loaded. I especially set src attr of one of images to non-existent file and value of property complete of this image remained false.
My corrections were wrong, I admit, but you have to change something, because when this happens in Opera I got heavy load on processor and dramatic increase in memory consumption

oh, by the way, I'm using Opera 12.16

One more problem - I'm experiencing problems with navigation arrows in IE10. They seems to be not working, but sometimes, when you click at them few times, they begin to work.

ok, try this code:

ic = ('fileSize' in t[i] && t[i].fileSize < 0 && t[i].count > 10) || t[i].count > 10 ? true : t[i].complete;

I just tried it in Opera, but since I'm on a Windows platform, I have version 17.0 and it does set the complete property to true when an image isn't found.

If that code works, I'll include it with the next update, thanks!

Edit: And I'll look into that IE issue.

Guess it will work. Opera 17 works on Webkit engine, like Chrome and Safari, so it sets complete to true. Opera 12 has it's own engine

I've made a website, where navigation is not working, it's on russian language, thou. It may help finding problem on IE
http://al.devep.ru/

Well, it looks like the problem is that base.initialized isn't set to true.... which is set when all images are loaded, which doesn't seem to be triggering the callback function (which sets the base.initialized flag), even with the above change.

I haven't had a chance to test it, but I think this additional change is needed:

if (c || img.length === 0) {
    // all complete, run the callback
    if (typeof callback === "function") { callback(); }
}

it didn't help, because img.length stays the same, as it was after first iteration, because you only push elements to array, but not delete them from it. The thing is, in my case, there's many images, that are hidden, so their height equals 0. That's why code
c = (c && ic && t[i].height !== 0)
always returns false after it checks height of hidden images
why is it so important to check height of images, if we know, that they already loaded?

There's another inconvenience - that is a period after page loaded, when you can't use navigation (something like timeout 2*animation speed, o.speed * 2) - but it's not that bad (even thou I use a fairly long animation speed)

Hmm, you're right... I didn't think about hidden images. Even failed images have a height (alt text), so yeah that height doesn't really do anything, so it should be removed as well.

You can always set the o.speed to a low number then elevate it after initialization:

$('#slider').on('initialized.movingBoxes', function(event, mb){
    mb.options.speed = 2000; // or whatever
});

Thanks!