Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abort/cancel loading triggered lazy images from the loading queue #438

Closed
wzol opened this issue Apr 11, 2020 · 45 comments
Closed

Abort/cancel loading triggered lazy images from the loading queue #438

wzol opened this issue Apr 11, 2020 · 45 comments

Comments

@wzol
Copy link

wzol commented Apr 11, 2020

I'm working on a thumbnail gallery: when you hover the mouse over a thumbnail, the related images are triggered to load for autoplay by the lazyinstance.

The problem is if the gallery A have like 12 images, and all or some of them are triggered to load, but if the user doesn't wait this and scrolls lower viewing gallery B, the images from the previous gallery A are still loading and in the loading queue. So before the images from the gallery B could load he need to wait the previously triggered images to load, even if they are not in view anymore.

Because visually nothing happens (beside a loading progress display), he moves on to gallery C, and now he need to wait gallery A and B images until the images in gallery C could start to load. The longer he tries, the more he would have to wait to actually see something.

So is there a way to abort and likely reset lazy images which are currently loading or pending but not in view anymore?

@verlok
Copy link
Owner

verlok commented Apr 11, 2020

Hey there, thank you for asking.

What you wrote made me think that maybe your thumbnails are not scaled down to be as light as they should be. If you scale them down properly, there wouldn’t be waiting time because the images will be very lightweight.

Anyway, there is a way to do what you asked and it’s to use the callback_exit option to cancel the loading of the image.

@wzol
Copy link
Author

wzol commented Apr 11, 2020

The thumbs are around 60-100KB - so I'd say they are fine, the bigger problem is that the server's response time is not good enough from everywhere. So if I use a 800ms autoplay gallery that time isn't always enough to load the next picture. And if I don't cancel the unwanted pictures, it makes it even worse for the user.

Yes, I think that callback could be helpful, be yet unsure what to do inside:

I have to cancel the actual image loading/pending in the browser, which can be done with manually setting an empty SVG as img src, but the problem is that this way the image will still become "loaded" for LazyLoad - even if actually it isn't.
So I probably need another step to reset its state for LazyLoad. Not sure how to do that: is removing the data enough, or also I need to handle the classes too? And also I probably need to "reobserve" it?

@verlok
Copy link
Owner

verlok commented Apr 11, 2020

You’re right. I was already thinking of releasing a new minor version with a reset method on it. You’d probably need that.

@wzol
Copy link
Author

wzol commented Apr 11, 2020

Sound really nice! Can't wait for it 👍

@wzol
Copy link
Author

wzol commented Apr 11, 2020

Would be possible to add this as an option, example "reset_threshold"? It is something that I call "fast reader - slow connection" problem: in short by the time the user reads the viewport the images aren't yet loaded, and then he scrolls more, but the current viewport couldn't be filled as the network is still busy loading images out of the viewport. So despite of lazy loading the user experience is bad as the user can read the page with holes or he can always wait for the network to catch up. What do you think?

@verlok
Copy link
Owner

verlok commented Apr 11, 2020

I've never thought of that case. 😮 And I like the "fast reader - slow connection"! I think I've also found a good almost-acronym for it: FRESCO! Ok I'm just joking...
So about the FRESCO case, I think it that it could be a very common case and it should be the default behaviour of LazyLoad.
I smell version 16 boiling in the kitchen...

@verlok
Copy link
Owner

verlok commented Apr 11, 2020

Well well, I was doing some research and look what I've found: setting the src attribute of the img tag to the empty string will interrupt the current download. I've tried it on this JsFiddle and it's true, to cancel a download you must set the image src to "".

This would be very tricky to bring to LazyLoad because it would mess with the script users pages, and I think this is a very bad idea. Even if I put that behind a configuration, it would be tricky to manage because of all the possible cases: a simple img tag, one with the src and srcset attribute, or a picture with many source tags, not to mention background images, iframes and videos.

So I think this script should expose the API to use it flexibly and in a smart way, and leave to its users decide what to do with their pages' DOM. And I'm back to my first idea of releasing a new minor version with a reset method on it. And of course I can put on a demo for the FRESCO use case.

@verlok
Copy link
Owner

verlok commented Apr 11, 2020

UPDATE: I was trying to build LazyLoad's restore method that brought the observed element to its initial status, and I started testing it on a demo, like this:

var callback_exit = function (element) {
    logElementEvent("🚪 EXITED", element);
    element.setAttribute("src", "");
    ll.restore(element);
};

When I discovered that setting the src to "" generates an error in the img, so the callback_error is triggered. And usually the callback_error is used to set the src to a fallback image.

var callback_error = function (element, instance, event) {
    logElementEvent("💀 ERROR", element);
    console.log("Event info", event);
    element.src = "https://via.placeholder.com/440x560/?text=Error+Placeholder";
};

So... I'm confused and I don't really know what to do right now. I probably need to get some sleep and, tomorrow, I hope to get a large coffee.

Speak to you tomorrow.

@wzol
Copy link
Author

wzol commented Apr 12, 2020

Well well, I was doing some research and look what I've found: setting the src attribute of the img tag to the empty string will interrupt the current download. I've tried it on this JsFiddle and it's true, to cancel a download you must set the image src to "".

Yes that is the thing I'm currently doing, it also works with empty placeholder svgs - at least in Chromium based browsers.

I was thinking about storing the original setting (example a placeholder) of the src in memory or in a data attribute, and when doing the reset for FRESCO (cool :)) you could not only reset the LazyLoad state, but all of its original attributes - like LazyLoad never even touched it. In case there wasn't any src before, I'd try to set a data png/svg just to cancel network operations and then set it to empty.

So something like:

  1. unobserve (might avoids callback error)
  2. set src based on original setting (to cancel peding actions and show original placeholders)
  3. clear/reset LazyLoad state params
  4. reobserve it (in case it gets into viewport again)

@verlok
Copy link
Owner

verlok commented Apr 13, 2020

Hey there.

The part where you put the original src value in another data attribute, I would leave it to you users, because putting it inside LazyLoad code

it would be tricky to manage because of all the possible cases: a simple img tag, one with the src and srcset attribute, or a picture with many source tags, not to mention background images, iframes and videos.

So I could develop a minor to expose methods to allow you doing what you described, meaning

  • reset lazyload state
  • observe elements again

What do you think?

@wzol
Copy link
Author

wzol commented Apr 13, 2020

While I think a full solution would fix cases when the developer is not even aware of this kind of problem (and partly the cause why some users actually hate lazy loading) I also understand your point and seems like a nice compromise having the tools to help users in not so developed countries, still on 3G, or having ADSL like connections.

At least at the end could you include a recipe/sample too - that might direct some attention to that this problem is real, it exists.

EDIT:

It seems I have missed an important thing in theory - maybe you didn't :) When callback_exit is fired or more specifically when does an item gets the loaded state? - as it won't be wise to reset already loaded items, but in this case I'm thinking about technically loaded items (like complete is true with images). So if it is handled this way I probably only need to check for a loaded or error class inside callback_exit and don't do a reset/restore if any exist, right?

@verlok
Copy link
Owner

verlok commented Apr 26, 2020

It seems I have missed an important thing in theory - maybe you didn't :) When callback_exit is fired or more specifically when does an item gets the loaded state? - as it won't be wise to reset already loaded items, but in this case I'm thinking about technically loaded items (like complete is true with images). So if it is handled this way I probably only need to check for a loaded or error class inside callback_exit and don't do a reset/restore if any exist, right?

Not sure about what you're saying/asking here.
Could you explain it again?

@wzol
Copy link
Author

wzol commented Apr 26, 2020

I was thinking about the images already "loaded" and exiting the viewport: the ones that actually loaded, and the ones are pending loading. It wouldn't be a good thing to reset the ones that are actually loaded, but resetting the ones that are pending to load is the goal. How can I distinguish the two, checking the class is enough?

@verlok
Copy link
Owner

verlok commented Apr 26, 2020

Yes of course.
The elements have their own status that can be observed, loading, loaded, error, native, etc.

@verlok
Copy link
Owner

verlok commented Apr 30, 2020

Hey, I'm here. Yesterday I developed something that touched the callback_exit and I think it can relate to this issue too. I'll build from there.

To recap what we discussed until now:

  • The final goal is to cancel downloading images that are still loading but exited the viewport, to optimize performance for FReSCo (fast reader, slow connection) users.
  • I won't create a way to do that using an option, because I believe it would be a lot of code and complexity to manage inside this library
  • I will give you the instruments to do that by yourfelf, more likely a way to reset lazyload status for a given element
  • I will put on a demo and write a recipe to make it easy to build

@verlok
Copy link
Owner

verlok commented May 1, 2020

I made it! Look!

I added the ability to reset an element status + created a working demo of how to use it to cancel the download of elements that exit while loading.

The new demo is named fresco_optimization.html

Everything works except that the instance loading count is left unhandled when resetting, so when a canceling happens, the callback_finished is called before time.

I'm afraid the reset method should:

  • be changed to an instance method (not static anymore)
  • consider the element's status
  • change or not the istance loading/managed count

I'm also not sure I want to expose a new method isElementLoading just for this purpose...
Maybe I really need to manage everything as an option, except the "cancel loading" part which is tricky.

But I'm tired now, I need to sleep over it.

verlok pushed a commit that referenced this issue May 1, 2020
@wzol
Copy link
Author

wzol commented May 1, 2020

It looks really promising and I can't wait to see how it'll perform on my live site. Considering that (as I know) your lazy is the only one that will handle FReSCo, I'm wouldn't be surprised by some dramatic results on implemented sites.

Oh, and have a great rest! :)

EDIT: the timer in the implementation made me thinking: what do you think about improving this with a threshold and a timeout? So it can be set how far the is the element to be reset, and how much time ago it made the exit. This could allow users to tweak this feature like if it exited 3 sec ago and still not loaded then reset it.

@verlok
Copy link
Owner

verlok commented May 1, 2020

It’s already under a threshold, the same threshold that works as a “look ahead” for the elements entering the viewport works as a “look behind” for their exit.

I had to put that setTimeout because setting the src of the image to "" triggers an asynchronous error, so LazyLoad would set the error status on the element, but I want to reset it to null.

@verlok
Copy link
Owner

verlok commented May 2, 2020

Hey @wzol ,
great news!

I've pushed new commits to that branch.

Now it introduces:

  1. Ability to reset an element status via resetElementStatus, in case you need it (e.g. some users want to change the data-src and make LazyLoad reconsider those images)
  2. Introduced a new option cancel_onexit to cancel the download of the exiting images
  3. Introduced a new callback callback_cancel that you MUST implement to cancel the download of your images
  4. Created a working demo named cancel_onexit to demo point 2. and 3.

The count loading / to load elements works now.
The isElementLoading is not exposed anymore.
General refactoring was applied.

Could you take a look at the cancel_onexit.html demo code and tell me what you think?
It's surely better than before, but...

I'm still thinking if it's REALLY necessary to delegate to the user the implementation via callback_cancel, but probably the answer is yes... too many cases to manage there.

@wzol
Copy link
Author

wzol commented May 2, 2020

I'm happy for the progress, and tried the cancel_onexit demo. Unfortunately for me it doesn't work as expected:

  • not sure if it is related but on page load all elements triggers EXITED - it should trigger only if had an ENTERED before, no?
  • I did set my connection to Slow 3G, fast scrolled through the page, and all images triggered ENTERED and LOADING, none was cancelled in the network pane, and finally all of them loaded and
    there is no CANCEL in the console

@verlok
Copy link
Owner

verlok commented May 2, 2020

Hey @wzol ,

I build the JavaScript on this repo only before releasing a new version.

To make the demos working on that branch you need to run a few commands first:

npm install

And when it's finished:

npm run build

Otherwise the demos would use the previous version of the script.

That’s probably why it didn’t work for you.

@wzol
Copy link
Author

wzol commented May 2, 2020

Ah, ok, I don't know how that works blush, so I'll wait :)

@verlok
Copy link
Owner

verlok commented May 2, 2020

Come on @wzol is not that hard, you just need to open a terminal and write those 2 commands I've posted in my latest comment. At least give it a try?

EDIT: I've just realized that I was giving for granted you have node JS installed on you machine.

@verlok
Copy link
Owner

verlok commented May 2, 2020

Tonight I've worked on the same branch.

I've solved which prevented the callback_finish to be called when the user scrolled up and down fast. It was due to multiple event listeners to the same elements and the callbacks being called twice. Now the event listeners references are stored inside the element itself and they are always removed before adding new ones.

Also I'm starting to seriously consider to include in this library the code to cancel the download, instead of delegating it to the script user via callback_cancel. I'm starting to work on all the cases to consider. Not sure this is the right way, but I'll give it a try.

@wzol
Copy link
Author

wzol commented May 3, 2020

Also I'm starting to seriously consider to include in this library the code to cancel the download, instead of delegating it to the script user via callback_cancel. I'm starting to work on all the cases to consider. Not sure this is the right way, but I'll give it a try.

That is great to hear. :) My secret wish was having something like this enabled by default (with really friendly default options) so everyone who updates or uses the library would spread a solution to FReSCo :) I know it is wild wish, but I think that is what a "wish" is for :)

(Yes, I don't know node JS, so I'll just wait my turn.)

@verlok
Copy link
Owner

verlok commented May 3, 2020

It’s a really good wish.
I’ll release it “behind an option” first, then release it to a wider audience.

@verlok
Copy link
Owner

verlok commented May 3, 2020

Awesome news!

I've just implemented "the wish" even if just for images (next step is to do so for videos and iframes too), and I want to share some performance data with you.

I've tested the same page with and without the cancel_onexit option.
I've set the network speed to "slow 3g" and recorded the page performance using Devtools.
At page landing, I scrolled down fast to the bottom of the page.

🔥 It's 2 times faster 🔥

Time taken to load the images at the bottom:

  • With cancel_onexit: false, 20 seconds
  • With cancel_onexit: true, 8.7 seconds

And there are some screenshot of the network activity.

With cancel_onexit: false
image

With cancel_onexit: true
image

Astonishing performance.
Thank you for your inspiration!

@verlok
Copy link
Owner

verlok commented May 3, 2020

The script is now managing videos and iframes too.
Next up:

  • After canceling the download, restore the original src (evil placeholders!) if there was one before it started loading.

@wzol
Copy link
Author

wzol commented May 4, 2020

Wow, that is really amazing - it is even better than expected, and beside performance imagine how much data it preserves on a mobile connection: I am so excited to play with it :)

As for placeholders I usually use empty SVGs which are small, and with them there is no reflow when an image is done loading example src="data:image/svg+xml,%3Csvg%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20viewBox%3D%220%200%20460%20215%22%3E%3C%2Fsvg%3E"

@verlok
Copy link
Owner

verlok commented May 6, 2020

Hey there! Here's an update on tonight's work.

  • The option was renamed to cancel_on_exit
  • The load cancelling is restricted to the img (including picture) elements
  • When cancelling loading, it restores the original attributes (placeholders?) of the image elements (both src and srcset)
  • Edited the image_ph_inline.html and image_ph_external.html demos to use the cancel_on_exit option (you're covered with src="data:image/svg+xml,...")
  • Improved the event listeners management and removal

Current issues:

  • Cancel loading on exit resets the element status to null instead of "observed". It works, but given that the element is currently being observed by the IntersectionObserver, it's probably more correct to set to "observed", like for other elements. Will check.

Next up:

  • Internal properties namespace refactoring.

Solved issues:

  • Canceling the download was triggering callback_error, now it no longer does.

@wzol
Copy link
Author

wzol commented May 6, 2020

Really awesome progress - big cheers for taking care of inline placeholders :)

@verlok
Copy link
Owner

verlok commented May 6, 2020

Wednesday night update!

  • Removed status "observed" (it was useless), introduced status "delayed" to distinguish exit at landing | exit while a delay is on.
  • Bugfix: delay_load wasn't working anymore, now it works even better
  • Set the cancel_on_exit: true in new demos for cross feature testing
    • image_ph_inline.html,
    • image_ph_external.html,
    • delay_test.html
    • fade_in.html
  • Refactored code in intersection handler functions (on enter | on exit)

It works like a charm now!

Next steps are:

  • a little refactoring (move element.llOriginalAttrs and element.llEventLisnrs to the same namespace, eg element.lazyload.originals, element.lazyload.listeners)
  • release this version! (which means documenting the new options and features, writing the changelog, etc.)

📅 My estimation is to release this by friday night, sunday night at top.

@wzol
Copy link
Author

wzol commented May 7, 2020

  • Removed status "observed" (it was useless), introduced status "delayed" to distinguish exit at landing | exit while a delay is on.

Uh, this is too much in one point for me blush - want to share some details about this?
And I really feel the excitement now :)

@verlok
Copy link
Owner

verlok commented May 7, 2020

want to share some details about this

The status is an attribute (ll-status) that LazyLoad applies to DOM elements to determine what’s going on on that element and ultimately to make things work.

That should not have an impact on LazyLoad users except for... now things work better.

@verlok
Copy link
Owner

verlok commented May 7, 2020

Hey @wzol,

🎉 I've just released 15.2.0 with this feature included. 🎉

Find all the relevant information about this release in the CHANGELOG, and all the details - including a new recipe - in the README.

Let me know what you think in the comments!

☕ And if you're happy about how fast your website performs thanks to the new version of LazyLoad, consider a coffee.

@wzol
Copy link
Author

wzol commented May 9, 2020

You did it! I tried it and it seems to work exactly as it should - can't believe a FReSCo solution made it this far, congrats!

I was wondering if there is a technical limitation to include background images? Usually they are the big full block/full page decoration elements which when cancelled could give a nice relief to the pending connections.

Also you should include the data reserve advantage too in the description, as if either the user is on a mobile, or the server is slow/overloaded both can profit from this feature.

@verlok
Copy link
Owner

verlok commented May 9, 2020

Hey @wzol ,
unfortunately it isn't possible to cancel a background image once it started loading.

I've tried in the LazyLoad script first, and now to clear any doubts I've created this Pen, where you can set and unset a background image using 2 buttons. If you slow down the connection and do set and reset quickly enough, you will see that the download of the image is not canceled.

So this feature will be available only for content images, but it makes sense because they are the most commonly used (and the most appropriate for SEO and A11Y).

@verlok
Copy link
Owner

verlok commented May 9, 2020

I look forward to receiving a link of the website where you're using LazyLoad 15.2 ;)
I'm going to close this now. Feel free to open another one (or as many as you want).
Cheers! ☕

@verlok verlok closed this as completed May 9, 2020
@verlok
Copy link
Owner

verlok commented May 9, 2020

And #437

@wzol
Copy link
Author

wzol commented May 10, 2020

I look forward to receiving a link of the website where you're using LazyLoad 15.2 ;)

My site is SteamPeek, a PC game recommendation site based on my own algorithm and formula. - It is still experimental, but already giving nice indie friendly results, also I have more ideas to make it better :)

@wzol
Copy link
Author

wzol commented Jun 2, 2021

Resurrecting this great thread :) In case of cancel_on_exit: true on cancel is the placeholder gets written back to the src? I'm using SVGs as placeholders on a long landing page - so there won't be reflow on image load. The menu scrolls up and down the page, and it seems that canceled loads adjusts the positions so the scroll stops at wrong positions. Not sure, about this, it is very hard to reproduce, but in theory can this be a cause?

@verlok
Copy link
Owner

verlok commented Jun 2, 2021

@wzol I have trouble understanding what you’re asking/stating here. Would you be able to write it in another way to make sure I understand? Thanks

@wzol
Copy link
Author

wzol commented Jun 2, 2021

You are probably familiar with the one page homepage/landing page style - one long page with a menu on top, and if you click on a menu item the page scrolls to the desired section.

To make this work correctly with lazyload I use empty SVG's as placeholders with the size of the original image for example for a 1920x1080 image

<img src="data:image/svg+xml,%3Csvg%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20viewBox%3D%220%200%201920%201080%22%3E%3C%2Fsvg%3E" ...>

This way when the image is loaded, it won't force a reflow, and the scroll to section should work correctly.

Sometimes now it scrolls to a wrong position (can't force to reproduce), but if I disable lazyload, or use cancel_on_exit: false it seems to work fine. Also rarely I find random images where it has the entered exited loaded classes and the src is set to the real image still the image doesn't show, looks like it still contains my SVG placeholder.

So I was wondering if there can be some problem with the cancel method. Btw I use currently Edge Canary 93.0.907.0.
I hope I could describe it better this time :)

@verlok
Copy link
Owner

verlok commented Jun 4, 2021

Hey @wzol,
thank you for clarifying.

So, to directly answer to your previous question:

In case of cancel_on_exit: true on cancel is the placeholder gets written back to the src?

The answer is: YES.

The following is part of the code that gets executed when:

  • an image has exited the viewport
  • the image was still loading
  • cancel_on_exit was set to true (default)
    If the previous conditions apply, LazyLoad does (I'm adding comments here to simplify the read):
// Sets the sources to `null` in order to cancel the HTTP request
resetSourcesImg(element); 

// Restores the original `src` and `srcset` attributes 
restoreOriginalAttributesImg(element);

// Removes the loading class
removeClass(element, settings.class_loading);

// Calls the cancel callback if defined
safeCallback(settings.callback_cancel, element, entry, instance);

Now, the rest of your question was full of sometimes, rarely, it seems and it's hard to reply to that if I don't have a test environment where to try it on.

Would you able to share part of your code or maybe put together a codepen / codesandbox to debug on it?

@wzol
Copy link
Author

wzol commented Jun 4, 2021

Thank you, I really appreciate your detailed answer. As currently I don't really have an idea how to isolate the issue (if there is any), take this as an "I thought you should know about this" feedback. I will post back if I can find/isolate anything concrete. ;)

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

No branches or pull requests

2 participants