jQuery.imagesLoaded does not work with FireFox #191

Closed
justnorris opened this Issue Apr 30, 2015 · 22 comments

Comments

10 participants
@justnorris

screen shot 2015-04-30 at 12 11 07 pm

Note: I didn't just "quickly take a screenshot". I waited for about a minute or so (in the meantime I moved the "about windows" in view). The images are never loaded

Note 2: In the screenshot it looks like no images are loaded at all. That's not true either. I just moved the "about windows" on top of the loaded images accidentally.

On not that good networks ( that's what the network link conditioner is there for ) imagesLoaded just stops trying to load the images at all. No 'fail', no 'always', it just stops and does nothing.

I used this for the screenshot: http://codepen.io/desandro/pen/bIFyl

imagesLoaded vanilla version works just fine with firefox and problematic network. For some reason the jQuery version is what causes problems.

@desandro

This comment has been minimized.

Show comment
Hide comment
@desandro

desandro May 3, 2015

Owner

Thanks for reporting this bug. I'm not able to reproduce this bug in Firefox v38. Sounds like a networking problem. Can you clarify that this problem only happens:

  • with poor networking
  • in Firefox
  • with jQuery
Owner

desandro commented May 3, 2015

Thanks for reporting this bug. I'm not able to reproduce this bug in Firefox v38. Sounds like a networking problem. Can you clarify that this problem only happens:

  • with poor networking
  • in Firefox
  • with jQuery
@justnorris

This comment has been minimized.

Show comment
Hide comment
@justnorris

justnorris May 3, 2015

  1. Yes
  2. As far as I've tested - Yes.
  3. Yes. edit: Tested: Vanilla version is broken too.

Did you try with "Network Link Conditioner" ? It only happens to me with that thing turned on. Stumbled on this because users reported the site not loading properly ( I'm using imagesLoaded to fade the site in ).

  1. Yes
  2. As far as I've tested - Yes.
  3. Yes. edit: Tested: Vanilla version is broken too.

Did you try with "Network Link Conditioner" ? It only happens to me with that thing turned on. Stumbled on this because users reported the site not loading properly ( I'm using imagesLoaded to fade the site in ).

@justnorris

This comment has been minimized.

Show comment
Hide comment
@justnorris

justnorris May 3, 2015

FireFox 38b9
screen shot 2015-05-03 at 9 45 39 pm

It doesn't always freeze. To reproduce click "add images" about 3 times, then wait for half of them to load, then click twice again. If that doesn't work - clear cache and try again.

It's a rather random error, but when many images are involved - it has a high risk of hanging in FireFox.

FireFox 38b9
screen shot 2015-05-03 at 9 45 39 pm

It doesn't always freeze. To reproduce click "add images" about 3 times, then wait for half of them to load, then click twice again. If that doesn't work - clear cache and try again.

It's a rather random error, but when many images are involved - it has a high risk of hanging in FireFox.

@justnorris

This comment has been minimized.

Show comment
Hide comment
@justnorris

justnorris May 3, 2015

Update: Found that it's broken on vanilla version too.
screen shot 2015-05-03 at 9 50 05 pm

Update: Found that it's broken on vanilla version too.
screen shot 2015-05-03 at 9 50 05 pm

@justnorris

This comment has been minimized.

Show comment
Hide comment
@justnorris

justnorris May 3, 2015

Tested Safari 8 with same network settings - works. Pushed Chrome to the limit with 44% packet drop and 800ms DNS delay - it took about 10 minutes to load, but everything worked out fine.

It's a FireFox Thing.

Another hint of what might the issue be - when it's stuck - if I "add more images", the images that were stuck previously are "unstuck" and everything loads properly.


Edit:
Managed to reproduce on my 100mbps home-office network. I just had to click "Add Images" rapidly a couple of times. - Turns out the problem is not isolated to bad networks ( but they do help to reproduce the issue ).

screen shot 2015-05-03 at 10 04 21 pm

Tested Safari 8 with same network settings - works. Pushed Chrome to the limit with 44% packet drop and 800ms DNS delay - it took about 10 minutes to load, but everything worked out fine.

It's a FireFox Thing.

Another hint of what might the issue be - when it's stuck - if I "add more images", the images that were stuck previously are "unstuck" and everything loads properly.


Edit:
Managed to reproduce on my 100mbps home-office network. I just had to click "Add Images" rapidly a couple of times. - Turns out the problem is not isolated to bad networks ( but they do help to reproduce the issue ).

screen shot 2015-05-03 at 10 04 21 pm

@justnorris

This comment has been minimized.

Show comment
Hide comment
@justnorris

justnorris May 7, 2015

beep beep
Any updates here ? It's kind of a big issue for me.

beep beep
Any updates here ? It's kind of a big issue for me.

@desandro

This comment has been minimized.

Show comment
Hide comment
@desandro

desandro May 7, 2015

Owner

Sorry, I won't be able to dig into this one. It's a deep edge case. I'll label this as help wanted.

Owner

desandro commented May 7, 2015

Sorry, I won't be able to dig into this one. It's a deep edge case. I'll label this as help wanted.

@desandro desandro added the help wanted label May 7, 2015

@justnorris

This comment has been minimized.

Show comment
Hide comment
@justnorris

justnorris May 8, 2015

In my mind - 7% of browser market-share isn't a deep edge case, considering that Flickity, Masonry, Packery are all based on imagesLoaded. It's a critical instability issue at the moment.

Even your demo site breaks on a FireFox and a poor internet connection. Yes, it's likely not an issue in California and Europe, but what about the developing world ? Does this mean that imagesLoaded shouldn't be used for "developing countries" that use FireFox ?

I'd help, but I have no idea even where to begin. Any ideas on what might be wrong with FireFox ?

In my mind - 7% of browser market-share isn't a deep edge case, considering that Flickity, Masonry, Packery are all based on imagesLoaded. It's a critical instability issue at the moment.

Even your demo site breaks on a FireFox and a poor internet connection. Yes, it's likely not an issue in California and Europe, but what about the developing world ? Does this mean that imagesLoaded shouldn't be used for "developing countries" that use FireFox ?

I'd help, but I have no idea even where to begin. Any ideas on what might be wrong with FireFox ?

@justnorris

This comment has been minimized.

Show comment
Hide comment
@justnorris

justnorris May 8, 2015

Here is a fully loaded "webster hall" homepage, found in your masonry demo links- with the same issue:
screen shot 2015-05-08 at 3 58 37 pm

Here is a fully loaded "webster hall" homepage, found in your masonry demo links- with the same issue:
screen shot 2015-05-08 at 3 58 37 pm

@crazoter

This comment has been minimized.

Show comment
Hide comment
@crazoter

crazoter May 14, 2015

I'm currently on Firefox 37.0.2 & I'm also experiencing the issue. I don't think the issue is isolated to only slow networks (although it does help reproduce the error) because I managed to reproduce the error consistently whilst having a download speed of 3.77 Mbps.

I'm no expert so I took a rather thorough (and possibly quite difficult to read) approach to this issue. The details can be found below.

I believe I have found a solution to this issue:
In the Resource.prototype.check function, I changed

var proxyImage = new Image();

to

var proxyImage = this.img;

where this.img is the actual Image element we are trying to track. I simply passed it into the Resource constructor when it was created in the LoadingImage.prototype.check method and modified the Resource constructor to store the variable.

//modified constructor (this is in LoadingImage.prototype.check)
var resource = cache[ this.img.src ] || new Resource( this.img.src,this.img );
//modified constructor (Resource)
function Resource( src,img ) {
    this.src = src;
    this.img = img;
    // add to cache
    cache[ src ] = this;
  }

Limitations of this fix:
I'm not very sure about this - this technique may not work when attempting to detect reloading images e.g. if an image is loaded using the Image element that has already previously loaded an image. I have not tested this to see if it will still work then. If this hypothesis happens to be true, an alternate fix would be to append an image element to the body and attach the events to it.

This fixed the issue on firefox. I did not test extensively on other browsers, but chrome still worked fine.

Details:
First, I have to understand the process, so I will state my understanding:
How I understand imagesLoaded works:

  1. an imagesLoaded object is created with the dom elements to be tracked (or a query selector string which is later used to obtain the elements).
  • getImages() is called, which is basically getting all image elements to be tracked.
  • imagesLoaded.prototype.check is called (with a slight time delay, 4-10ms) which basically involves looping through every image, checking if they are complete and binding the complete eventhandler on them.
  • LoadingImage.prototype.check is called for each image (to check if they are complete).
    • First, it checks if the image was loaded before (check if cached). If not, it initializes a new Resource object for that image.
    • Usually that returns false, so it then uses the image.complete and naturalwidth to check if it is complete manually. This is form of checking is only performed once, 4-10ms after the imagesLoaded object is created.
    • Unless your image was already loaded when the imagesLoaded object was created, that will return false and it will call Resource.prototype.check which contains the image's src.
      • At this point, resource determines if the image is loaded by simulation - it creates a new Image, binds the 'load' and 'error' to it using eventie and sets the src so that it begins loading.

Observations:

  • I don't think the issue lies in LoadingImage.prototype.check because all images are correctly obtained and checked here.
  • I don't think the issue lies in the Firefox Images because I had looped through all the images stored in this.Images after I was certain they were loaded and all of them had true on their complete property and a non-undefined naturalWidth property. Of course, because these properties are only checked at the start, their use is limited in this scenario.
  • However, when an image "fails to load" the event is not caught in either 'load' or 'error', so the error must be related to the binding of the event to detect if the image is loaded / error. This is the method I speak of:
    Resource.prototype.check = function() {
    // only trigger checking once
    if ( this.isChecked ) {
      return;
    }
    // simulate loading on detached element
    var proxyImage = new Image();
    eventie.bind( proxyImage, 'load', this );
    eventie.bind( proxyImage, 'error', this );
    proxyImage.src = this.src;
    // set flag
    this.isChecked = true;
  };
  • I don't think the issue lies in eventie because eventie just binds the event. However, I think the problem lies in creating a new Image() and using it to check if the image is loaded - in Firefox it apparently doesn't succeed all the time for some reason. When I passed the image element into the Resource constructor and did
 var proxyImage = this.img;//this.img is the actual dom Image we are trying to track

I tested this fix 5 times using 20 images from imgur and it worked 5/5 times (no caching), as opposed to the 1/5 times successful with creating a new Image().

I'm currently on Firefox 37.0.2 & I'm also experiencing the issue. I don't think the issue is isolated to only slow networks (although it does help reproduce the error) because I managed to reproduce the error consistently whilst having a download speed of 3.77 Mbps.

I'm no expert so I took a rather thorough (and possibly quite difficult to read) approach to this issue. The details can be found below.

I believe I have found a solution to this issue:
In the Resource.prototype.check function, I changed

var proxyImage = new Image();

to

var proxyImage = this.img;

where this.img is the actual Image element we are trying to track. I simply passed it into the Resource constructor when it was created in the LoadingImage.prototype.check method and modified the Resource constructor to store the variable.

//modified constructor (this is in LoadingImage.prototype.check)
var resource = cache[ this.img.src ] || new Resource( this.img.src,this.img );
//modified constructor (Resource)
function Resource( src,img ) {
    this.src = src;
    this.img = img;
    // add to cache
    cache[ src ] = this;
  }

Limitations of this fix:
I'm not very sure about this - this technique may not work when attempting to detect reloading images e.g. if an image is loaded using the Image element that has already previously loaded an image. I have not tested this to see if it will still work then. If this hypothesis happens to be true, an alternate fix would be to append an image element to the body and attach the events to it.

This fixed the issue on firefox. I did not test extensively on other browsers, but chrome still worked fine.

Details:
First, I have to understand the process, so I will state my understanding:
How I understand imagesLoaded works:

  1. an imagesLoaded object is created with the dom elements to be tracked (or a query selector string which is later used to obtain the elements).
  • getImages() is called, which is basically getting all image elements to be tracked.
  • imagesLoaded.prototype.check is called (with a slight time delay, 4-10ms) which basically involves looping through every image, checking if they are complete and binding the complete eventhandler on them.
  • LoadingImage.prototype.check is called for each image (to check if they are complete).
    • First, it checks if the image was loaded before (check if cached). If not, it initializes a new Resource object for that image.
    • Usually that returns false, so it then uses the image.complete and naturalwidth to check if it is complete manually. This is form of checking is only performed once, 4-10ms after the imagesLoaded object is created.
    • Unless your image was already loaded when the imagesLoaded object was created, that will return false and it will call Resource.prototype.check which contains the image's src.
      • At this point, resource determines if the image is loaded by simulation - it creates a new Image, binds the 'load' and 'error' to it using eventie and sets the src so that it begins loading.

Observations:

  • I don't think the issue lies in LoadingImage.prototype.check because all images are correctly obtained and checked here.
  • I don't think the issue lies in the Firefox Images because I had looped through all the images stored in this.Images after I was certain they were loaded and all of them had true on their complete property and a non-undefined naturalWidth property. Of course, because these properties are only checked at the start, their use is limited in this scenario.
  • However, when an image "fails to load" the event is not caught in either 'load' or 'error', so the error must be related to the binding of the event to detect if the image is loaded / error. This is the method I speak of:
    Resource.prototype.check = function() {
    // only trigger checking once
    if ( this.isChecked ) {
      return;
    }
    // simulate loading on detached element
    var proxyImage = new Image();
    eventie.bind( proxyImage, 'load', this );
    eventie.bind( proxyImage, 'error', this );
    proxyImage.src = this.src;
    // set flag
    this.isChecked = true;
  };
  • I don't think the issue lies in eventie because eventie just binds the event. However, I think the problem lies in creating a new Image() and using it to check if the image is loaded - in Firefox it apparently doesn't succeed all the time for some reason. When I passed the image element into the Resource constructor and did
 var proxyImage = this.img;//this.img is the actual dom Image we are trying to track

I tested this fix 5 times using 20 images from imgur and it worked 5/5 times (no caching), as opposed to the 1/5 times successful with creating a new Image().

dmoreno added a commit to hi-impact/imagesloaded that referenced this issue May 25, 2015

temporal fix to #191
jQuery.imagesLoaded does not work with FireFox
@duynguyen1990

This comment has been minimized.

Show comment
Hide comment
@duynguyen1990

duynguyen1990 May 28, 2015

Hi,

I have a trouble with imagesloaded js, when i try with ctrl + f5 the browser, the issue sometimes happens. I show the console in firebug and notice that one image is checked by false ( "check" event return false), and that image has 744.00KB and load time 7.15s. See Below:

All images is loaded but why a image is check false, so the inner function can't run.

P/s sorry for bad english
Wait for your advice!
imageloaded1
imageloaded2
imageloaded3

Hi,

I have a trouble with imagesloaded js, when i try with ctrl + f5 the browser, the issue sometimes happens. I show the console in firebug and notice that one image is checked by false ( "check" event return false), and that image has 744.00KB and load time 7.15s. See Below:

All images is loaded but why a image is check false, so the inner function can't run.

P/s sorry for bad english
Wait for your advice!
imageloaded1
imageloaded2
imageloaded3

@crazoter

This comment has been minimized.

Show comment
Hide comment
@crazoter

crazoter May 28, 2015

Did you use the imagesloaded.pkgd.js which includes the commit pushed by dmoreno? It should fix the issue
You can get the file from this branch:
https://github.com/hi-impact/imagesloaded/tree/fix-issue-191

the imagesloaded.js file in this branch is not updated to reflect the fix though, so if you use that you may experience the same issue.

Did you use the imagesloaded.pkgd.js which includes the commit pushed by dmoreno? It should fix the issue
You can get the file from this branch:
https://github.com/hi-impact/imagesloaded/tree/fix-issue-191

the imagesloaded.js file in this branch is not updated to reflect the fix though, so if you use that you may experience the same issue.

@tgmweb

This comment has been minimized.

Show comment
Hide comment
@tgmweb

tgmweb Jul 21, 2015

@crazoter - I had the same issue with Firefox and your fix worked for me.

Thanks!

BTW - this issue is not limited to slow networks. Our images are hosted on s3/cloudfront and we're in NYC with high speed FIOS.

Tom.

tgmweb commented Jul 21, 2015

@crazoter - I had the same issue with Firefox and your fix worked for me.

Thanks!

BTW - this issue is not limited to slow networks. Our images are hosted on s3/cloudfront and we're in NYC with high speed FIOS.

Tom.

@tmslnz

This comment has been minimized.

Show comment
Hide comment
@tmslnz

tmslnz Aug 4, 2015

@crazoter Thank you for this; it fixed my issue as well.
@desandro Wonder why this fix isn’t merged in yet (or manually integrated)?

tmslnz commented Aug 4, 2015

@crazoter Thank you for this; it fixed my issue as well.
@desandro Wonder why this fix isn’t merged in yet (or manually integrated)?

@desandro

This comment has been minimized.

Show comment
Hide comment
@desandro

desandro Aug 7, 2015

Owner

@crazoter Thanks for looking deeper into this! This is tremendous work. I'll be incorporating your insights in the next refactor.

Owner

desandro commented Aug 7, 2015

@crazoter Thanks for looking deeper into this! This is tremendous work. I'll be incorporating your insights in the next refactor.

@vejersele

This comment has been minimized.

Show comment
Hide comment
@vejersele

vejersele Aug 12, 2015

@desandro you probably also want to try the following:

Resource.prototype.check = function() {
    // only trigger checking once
    if ( this.isChecked ) {
      return;
    }
    // simulate loading on detached element
    this.proxyImage = new Image();
    eventie.bind( this.proxyImage, 'load', this );
    eventie.bind( this.proxyImage, 'error', this );
    this.proxyImage.src = this.src;
    // set flag
    this.isChecked = true;
  };

It could be that the proxyImage variable is getting garbage collected because it's defined locally. Assigning proxyImage as a class property will prevent this from happening ...

Also: I wouldn't suggest using the actual dom element to check if the image is loaded because when the image is already loaded before the script is executed, it's going to look like the image never loads.

@desandro you probably also want to try the following:

Resource.prototype.check = function() {
    // only trigger checking once
    if ( this.isChecked ) {
      return;
    }
    // simulate loading on detached element
    this.proxyImage = new Image();
    eventie.bind( this.proxyImage, 'load', this );
    eventie.bind( this.proxyImage, 'error', this );
    this.proxyImage.src = this.src;
    // set flag
    this.isChecked = true;
  };

It could be that the proxyImage variable is getting garbage collected because it's defined locally. Assigning proxyImage as a class property will prevent this from happening ...

Also: I wouldn't suggest using the actual dom element to check if the image is loaded because when the image is already loaded before the script is executed, it's going to look like the image never loads.

@m4olivei

This comment has been minimized.

Show comment
Hide comment
@m4olivei

m4olivei Aug 25, 2015

We're also seeing this issue, and the proposed fix from @crazoter and @dmoreno seems to fix the problem. Can't say I understand why that patch fixes things, but seems to. Testing ongoing. Very interested in seeing this make it into a future release. We're relying on imagesLoaded to "reveal" parts of our website.

We're also seeing this issue, and the proposed fix from @crazoter and @dmoreno seems to fix the problem. Can't say I understand why that patch fixes things, but seems to. Testing ongoing. Very interested in seeing this make it into a future release. We're relying on imagesLoaded to "reveal" parts of our website.

@TerribleDev

This comment has been minimized.

Show comment
Hide comment
@TerribleDev

TerribleDev Oct 8, 2015

👍 for this fix What can we do to get this in?

👍 for this fix What can we do to get this in?

@justnorris

This comment has been minimized.

Show comment
Hide comment
@justnorris

justnorris Oct 27, 2015

@desandro I thought you quit twitter to work full-time on your plugins. This seems like a pretty critical issue, isn't it? It's in the core of many of your other libraries, I don't understand why is this issue ignored.

screen shot 2015-10-27 at 09 57 36

@desandro I thought you quit twitter to work full-time on your plugins. This seems like a pretty critical issue, isn't it? It's in the core of many of your other libraries, I don't understand why is this issue ignored.

screen shot 2015-10-27 at 09 57 36

@desandro desandro closed this in bcbb8ca Oct 28, 2015

@desandro desandro reopened this Oct 28, 2015

@desandro

This comment has been minimized.

Show comment
Hide comment
@desandro

desandro Oct 29, 2015

Owner

🎉 imagesLoaded v3.2.0 integrates @crazoter's fix for Firefox 🎉 Try it out and report back if this issue has been resolved or not 🍊

Owner

desandro commented Oct 29, 2015

🎉 imagesLoaded v3.2.0 integrates @crazoter's fix for Firefox 🎉 Try it out and report back if this issue has been resolved or not 🍊

@gsmeira

This comment has been minimized.

Show comment
Hide comment
@gsmeira

gsmeira Oct 30, 2015

Just in time! :D

It's working for me here in firefox, and now the plugin is detecting reloaded images. I'm using another plugin, ResponseJS, and when the src change, the imagesloaded is detecting that change.

Thanks @desandro for the plugin and thanks @crazoter for the fix!! 👍

gsmeira commented Oct 30, 2015

Just in time! :D

It's working for me here in firefox, and now the plugin is detecting reloaded images. I'm using another plugin, ResponseJS, and when the src change, the imagesloaded is detecting that change.

Thanks @desandro for the plugin and thanks @crazoter for the fix!! 👍

@desandro

This comment has been minimized.

Show comment
Hide comment
@desandro

desandro Nov 30, 2015

Owner

I'm closing this issue as resolved. If you run into an issue with imagesLoaded or Firefox, please open a new issue.

Owner

desandro commented Nov 30, 2015

I'm closing this issue as resolved. If you run into an issue with imagesLoaded or Firefox, please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment