fix(gatsby-image): Remove native browser cache detection feature #26965
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Presently only implemented for IntersectionObserver fallback, this method will always be falsey in Firefox and always truthy in Safari, other browsers when using the IO render path work correctly. Safari is the primary browser that defaults to IO (native lazy load is available as an experimental feature toggle), and while `imgCached` can be implemented to support native lazy loading too, Safari is likely to remain buggy, so this feature is being removed. There has been no complaints regarding it's omission with Firefox and Chrome native lazy load support that has been available for a reasonable amount of time now. --- The `handleRef()` method has also been slightly refactored. Better for any future handling without IO, and for Art Directed images using IO.
gatsbot
bot
added
the
status: triage needed
This was referenced Sep 20, 2020
polarathene
changed the title
imgCached featureIt should just be `isVisible`, that will make `<picture>` available on the DOM to start downloading the image, and `imgLoaded` will be set once that image has finished loading.
Don't override the `fadeIn` state, `imgCached` handles this purpose. `imgLoaded` would also be true in this event. `seenBefore` should only be valid in the browser after hydration phase, initial render should therefore still match SSR. Re-worded comments for maintainers.
polarathene
force-pushed
the
fix/gatsby-image-imgcached-safari
branch
from
2b9da32
to
f20d6f9
Compare
LekoArts
added
status: needs core review
|
Closing, this is fixed in our new gatsby-plugin-image component. Thank you for your contribution! <3 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
imgCachedis presently only implemented for IntersectionObserver fallback, this method will always be falsey in Firefox and always truthy in Safari, other browsers when using the IO render path work correctly.Safari is the primary browser that defaults to IO (native lazy load is available as an experimental feature toggle), and while
imgCachedcan be implemented to support native lazy loading too, Safari is likely to remain buggy, so this feature is being removed.There has been no complaints regarding it's omission with Firefox and Chrome native lazy load support that has been available for a reasonable amount of time now.
The
handleRef()method has also been slightly refactored. Better for any future handling without IO, and for Art Directed images using IO.This PR uses a portion from my Image cache improvements PR, slightly modified to remove the
imgCachedhandling that brought the feature to native image lazy load (at least for blink browsers). There has been various reports / confirmations of Safari mishandling this feature.Note: This PR has not been thoroughly tested. It should only affect browsers that use IntersectionObserver, and since the majority for that is Safari, it should resolve the reported bug. The latest Safari 14 release from this month keeps img native lazy loading as an experimental toggle feature, this will continue to be an issue for a while longer without this PR.
Warning:
imgCachedwas originally introduced by this PR to address this issue, avoiding a placeholder flicker for an already cached image. I do not have access to Safari to reproduce, nor have I verified if that is an issue for Chrome/Firefox with currentgatsby-image.TL;DR
Safari mishandles a feature that Chrome used to benefit from. It's been reverted to remove the rendering bug.
imgCachedis broken for Safari users. That feature was added before native lazy load, only Intersection Observer render path uses it.gatsby-imagefallsback to Intersection Observer for lazy load.Related Issues
Fixes: #20126
Potential regression: #12254