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

Lazyload: N-fetches for each relevant mutation #5314

Closed
domfarolino opened this issue Feb 25, 2020 · 2 comments
Closed

Lazyload: N-fetches for each relevant mutation #5314

domfarolino opened this issue Feb 25, 2020 · 2 comments
Assignees

Comments

@domfarolino
Copy link
Member

After talking with @zcorpan and @rwlbuis, it seems there is an issue in the current specification for images with loading=lazy specified.

The updating-the-image-data algorithm at step (7), queues a microtask to continue the rest of the steps. Interestingly, any number of "relevant mutations" can happen by the time this algorithm is resumed, and therefore any number of "extra" invocations of updating-the-image-data can be started. This is why step (8) exists, which effectively kills any "outdated" invocations of this algorithm, so only the latest takes effect.

When we introduced lazyloading for images, we added another stopping point in this algorithm, that allows the algorithm to be resumed later (i.e., when the image intersects the viewport). During this async stopping point, again any number of "relevant mutations" can take place, starting an arbitrary number of invocations of this algorithm. We didn't add another step after the potential resumption similar to (8), to kill outdated invocations of the algorithm though. So per spec, I believe it is possible that N number of updating-the-image-data-algorithms can be running for any given <img> which is not good. I think we should add a step similar to (8), after the lazyload resumption, and verify that all of our tests assert this behavior. Does this sound good?

/cc @annevk @kinu

@domfarolino domfarolino self-assigned this Feb 25, 2020
@annevk
Copy link
Member

annevk commented Mar 3, 2020

cc @hiikezoe @emilio

@domfarolino
Copy link
Member Author

As per #5433 (comment), I'm going to close this.

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

Successfully merging a pull request may close this issue.

2 participants