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

The placeholder image in "gatsby-image" does not fade on Safari, again #20126

Closed
samselikoff opened this issue Dec 13, 2019 · 36 comments
Closed
Labels
stale? Issue that may be closed soon due to the original author not responding any more. status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby

Comments

@samselikoff
Copy link

This issue was fixed some time ago (#8323), but it seems to have crept up again.

You can see it by visiting https://using-gatsby-image.gatsbyjs.org/ on Safari. The placeholder does not fade in, it just disappears.

Let me know if you need more context!

@samselikoff
Copy link
Author

Interestingly, setting loading='eager' seems to fix the fade-in issue.

@LekoArts LekoArts added the status: needs reproduction This issue needs a simplified reproduction of the bug for further troubleshooting. label Dec 16, 2019
@LekoArts
Copy link
Contributor

Hi! I tested it on my Catalina MacBook with Safari 13.0.3 and don't see this behavior. Please provide a reproduction and video of what you're seeing. Thanks!

@samselikoff
Copy link
Author

Sure thing! Here's a video: https://www.youtube.com/watch?v=Rh0gXnIiC7k.

Safari's on the left, Chrome on the right, and as you can see in Safari the placeholder never fades away – it just disappears. I'm just visiting https://using-gatsby-image.gatsbyjs.org/ in both browsers.

I'm on Catalina and Safari 13.0.4.

In the other issue @ooloth also confirmed seeing the issue on the demo site.

@LekoArts LekoArts added status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby and removed status: needs reproduction This issue needs a simplified reproduction of the bug for further troubleshooting. labels Dec 17, 2019
@github-actions
Copy link

github-actions bot commented Jan 6, 2020

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Jan 6, 2020
@TheFirstMe
Copy link
Contributor

Not stale. I am also experiencing this on iOS safari with sometime the placeholder not fading even when the image is loaded.

@github-actions github-actions bot removed the stale? Issue that may be closed soon due to the original author not responding any more. label Jan 7, 2020
@nathobson
Copy link
Contributor

nathobson commented Jan 15, 2020

I'm also experiencing it on iOS (13) and desktop Safari (Catalina, Safari 13.0.3). It's especially harsh if you don't use any type holder, i.e. GatsbyImageSharpFluid_withWebp_noBase64. The image just pops in with no fade.

Here's a demo video in Safari: https://www.youtube.com/watch?v=PqLYXdaqSCI

Looks like in Chrome, the img element inside gatsby-image has a property transition: opacity 500ms ease 0s; whereas in Safari, this is transition: none;.

@github-actions
Copy link

github-actions bot commented Feb 4, 2020

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Feb 4, 2020
@nathobson
Copy link
Contributor

nathobson commented Feb 4, 2020

Not stale. I've added reproduction details and it was confirmed as a bug.

@TheFirstMe
Copy link
Contributor

Not stale

@github-actions github-actions bot removed the stale? Issue that may be closed soon due to the original author not responding any more. label Feb 5, 2020
@Digital-E
Copy link

Same issue here!

@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Feb 28, 2020
@ooloth
Copy link
Contributor

ooloth commented Feb 28, 2020

Hmm. In general, this auto-closing action is not cool.

I understand closing an issue after awhile if the Gatsby team has tried to address it but the reporter has stopped responding.

But when the reporter has responded with the requested reproduction and the delay is on Gatsby's end (totally fine), the issue should not be in danger of closing.

Perhaps the action can bypass issues labelled status:confirmed?

@Digital-E
Copy link

I had this issue too! A temporary workaround is to add this to your CSS:

.gatsby-image-wrapper img {
    -webkit-transition: opacity .5s ease 0s!important;
    transition: opacity .5s ease 0s!important;
}

@ooloth ooloth removed the stale? Issue that may be closed soon due to the original author not responding any more. label Feb 28, 2020
@steverandy
Copy link

It seems the issue is related to this line https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-image/src/index.js#L395

When I change it to imgCached: false, the fade animation works for both safari and chrome.

@github-actions
Copy link

github-actions bot commented Apr 8, 2020

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Apr 8, 2020
@github-actions
Copy link

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.
Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community! 💪💜

@polarathene
Copy link
Contributor

It seems the issue is related to this line https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-image/src/index.js#L395

When I change it to imgCached: false, the fade animation works for both safari and chrome.

Probably need to open a new issue now, since contributors can no longer open issues or add the not stale tag.

I added the imgCached bool originally (it's been modified a few times by others), I also have another PR open currently that reworks that, and also applies it to Chrome/Blink based browsers. I don't have access to Safari however, which will be using Intersection Observer for lazy loading instead of the native img support for lazyload that Firefox and Chrome have. I think Safari will be getting such in a new release sometime this year, so technically the problem might just disappear.

Can you clarify that the issue still happens on a slower connection (assuming Safari has network throttle in it's devtools like Chrome and Firefox do), and disable/empty the browser cache. imgCached should only be set if the image url returned, which meant it was cached in Chrome based browsers at least. Firefox doesn't seem to acknowledge it, and perhaps Safari is another variation in the behaviour, if so that needs to be properly verified, then the "optimization" can be removed.


One more thing. When you navigate between pages, the images shouldn't be fading in? But you want them to when visiting a page directly(via new tab or entering the URL) regardless of if the image needs to download or not? It should still do the fadein transition when first encountering the image (browser hasn't seen it before to cache it).

Here's my current PR(not a direct PR fix for this, but reworks cache logic), feel free to give that a try too.

@polarathene
Copy link
Contributor

Confirmed with @steverandy via Discord. For whatever reason Safari is apparently setting imgCached always as true, somehow currentSrc has been assigned a proper value(string path to an image) at that point, when it's not expected to unless it's in the browser cache. @steverandy tested with empty cache both current gatsby-image code as well as my linked PR.

I would appreciate if anyone else can test in Safari to confirm this.

@lucashfreitas
Copy link

lucashfreitas commented Sep 1, 2020

Same issue here. I am using fadeIn effect for the gatsby image therefore it's not working on safari/ IOS 13.

@polarathene
Copy link
Contributor

@lucashfreitas are you testing in a fresh browser instance (eg private/incognito session) with the browser dev tools open for the network tab and disabling+clearing the browsers cache? If so can you modify the gatsby-image modules source to check imgCached value? It shouldn't be true in this situation.

You can copy the JS source code from this repo package of gatsby-image index.js, and replace the equivalent file in your node_modules package. Modify this line to:

        this.setState({ isVisible: true }, () => {
          const imgCached = this.imageRef.current && this.imageRef.current.currentSrc
          console.log(`imageCached is: ${imageCached}, as boolean: ${!!(imageCached)}`)
          
          this.setState({
            imgLoaded: imageInCache,
            imgCached: !!(imageCached),
          })
        })

Then report back the value logged to console. If you test with another browser like Chrome in the same way, it should be false(empty string) with cache disabled, and then if you enable browser caching again, refreshing when it has been cached locally (and uses memory cache) should report it as true with the asset URL.

imageCached is a boolean value to decide if the fadeIn effect should be skipped, it's meant to mimic the behaviour of navigating the website where if the image has been seen before you'll notice it should not use the fadeIn again. Please confirm your expectation is for this skipping behaviour to match existing skipping of the fadeIn effect like page navigation does, or if you're expecting it to always fadeIn regardless including with page transitions.

I find it odd that Safari would be having a value that sets that to true when the asset has not been pulled from the cache, no other browser exhibits this behaviour that I've tested, including other webkit based browsers.

Also, please do test this with production, or at least gatsby build + gatsby serve, not just gatsby develop.

@nelsonvassalo
Copy link

This is still happening to me. Is there any known workaround while we wait for a possible PR?

@polarathene
Copy link
Contributor

This is still happening to me.

@nelsonvassalo it'd be helpful if you could follow the instructions in my previous comment above. And answer the questions about expectations.

Safari should have native image lazy loading support in it's next release which may make the issue go away, but it'd be nice to get proper confirmation from other users following my instructions above before that happens.

@intelligence
Copy link

Happening to me on Safari 13.1 on macOS and on iOS 13.5.1. I've applied the CSS fix for now but will try and test out your instructions in the coming days @polarathene

@nelsonvassalo
Copy link

This is still happening to me.

@nelsonvassalo it'd be helpful if you could follow the instructions in my previous comment above. And answer the questions about expectations.

Safari should have native image lazy loading support in it's next release which may make the issue go away, but it'd be nice to get proper confirmation from other users following my instructions above before that happens.

Hi @polarathene sorry. Yes I went ahead and applied the CSS fix but it has to be done with an !important flag, as gatsby somehow fills the inline style on the element to transition: noneon Safari.

@polarathene
Copy link
Contributor

polarathene commented Sep 6, 2020

Yes I went ahead and applied the CSS fix but it has to be done with an !important flag

@nelsonvassalo I didn't advise that. I asked you to test some modifications to gatsby-image for me, I don't have access to Safari and either there is a misunderstanding of how gatsby-image is working for intersection observer (no native lazyload support for img by browser) or Safari is not behaving the same as the webkit browser I have tested with.

as gatsby somehow fills the inline style on the element to transition: none on Safari.

gatsby-image uses inline styles, it has props that you can pass style overrides to so that you don't need to use !important to override it.

  • Is your expectation that the fade in transition always happen when you visit the page, even if you have already downloaded the image before(browser cached it)?
  • Have you tested with gatsby build + gatsby serve or only with gatsby develop?
  • Have you tested with network dev tools for Safari, disabled/cleared cache and set network throttle to 3G, then refreshed?

  1. Safari when tested with production build using gatsby serve and throttled connection with cache cleared should show the image placeholder while it downloads, and then you should see the fade in transition.
  2. Allowing browser cache now, refresh and the image should not take so long as it should not download and use the browser cache. If that is happening, the browser cache was used and you should be able to look at the image resource on the network tab, it should tell you if memory or disk cache was used, it should be memory-cache to skip the fade in transition, if disk-cache or no cache, you would see the fadein transition again.
  3. If that is not what you experience, only then is something wrong.

@blimpmason
Copy link

@polarathene as I understand from following this thread, this issue is caused by a known Safari bug and the only workaround is the CSS fix at the moment (AFAIK).

.gatsby-image-wrapper img,
.gatsby-image-wrapper > div {
  transition: opacity 500ms ease 0s !important;
}

Without that CSS override, the fade in transitions never happen in Safari, whether or not the image is already cached. With that CSS override it works every time. The consensus seems to be that Safari thinks it has the image cached when it doesn't. For me this applies to both gatsby build/serve and gatsby develop, as well as in production for every Gatsby site I've built.

Even after the Safari fix is rolled out by Apple, we'll still need to support the previous few versions at best. I think the hope is that the workaround could be implemented in the Gatsby Image component directly rather than having to apply this CSS bandaid (though I just include it in my boilerplate global CSS for every Gatsby project).

I think this issue should be re-opened.

@KyleAMathews
Copy link
Contributor

I can reproduce this with Safari 13.1.2

@KyleAMathews KyleAMathews reopened this Sep 7, 2020
@KyleAMathews
Copy link
Contributor

If the above CSS fixes things, I'd love to have it PRed into gatsby-image directly.

@polarathene
Copy link
Contributor

Without that CSS override, the fade in transitions never happen in Safari, whether or not the image is already cached.

Please follow the instructions I have detailed here. Provide me with actual console output as requested, and confirm what your expectations actually are from the described scenarios.

I'm well aware that Safari users are having an issue, but what has not been clarified is if this entirely the fault of code or also partially a mismatch of expectations. The native lazy load support for img arrived after imgCached was used for Intersection Observer, Firefox and Chrome now use native lazyload, that new branch has not been updated to use imgCached (I have a PR active that remedies that), so for all I know, users are comparing to that and expecting Safari to match that behaviour without understanding the intent of imgCached, if so you could equally say that Firefox/Chrome has a bug then.

If the behaviour on a throttled 3G connection in Safari shows no fade in transition when it should and the browser cache is cleared (thus the throttled connection delays the image loading with the placeholder enough and you can actually see a download is happening in the network tools), that is an issue.

When observing the network transfer, note that gatsby develop and gatsby serve serve content with Cache-Control headers that set a max-age of 0, this forces revalidation and may result in different behavior when max-age is set to a longer duration to not trigger a revalidation(ask server if content is still same, which if false downloads again, or if true uses local copy), while it can use the browser cache, it can result in different behaviour when a revalidation isn't performed. Chrome has an issue/bug that is affected by such for example.

If I had a macOS system with Safari I would validate all this myself properly.


Please let me know what the console output is when making the modifications to gatsby-image. imgCached should not be true unless the browser has a cache of the image already. No other browser triggers this, I have forced Firefox and Chrome to use Intersection Observer, Opera and Epiphany(Webkit) also do not misbehave in this way.

What value is Safari setting that causes imgCached to be true? It is most likely a string to one of the image assets, is it using an appropriate one for the picture element, or is it defaulting to the src directly on the img?


To clarify, there are several "caches" involved here.

  • gatsby-image uses a imageCache for several things, this is only available after the first time the JS sees the image and lasts until a browser refresh clears that state.
  • gatsby-image uses a imgCached presently only for Intersection Observer lazyload, it is set once the image is mounted and relies on an img attribute that the browser sets when it knows what picture url to request (waits on response from server if not cached by browser iirc)
  • Browser cache, which has memory-cache(true), disk-cache(false) and no cache(request), cached(but revalidate), cached(no need to revalidate).

I have a WIP PR that brings imgCached to Chrome, and reworks the handling for Intersection Observer as well, I would suggest trying that as well. But really, I need that console output I've requested.

@polarathene
Copy link
Contributor

@KyleAMathews you can remove imgCached or see if my WIP PR makes any difference.

Note, reverting this will trade one browser specific problem for another. You previously wanted fadein disabled if the image had been detected as cached in the browser. Related PR that fixed that by adding imageCached.

You can either chalk it up to a hacky detection solution or Safari behaving against expectations, all other browsers either return true when actually valid, and false if not in the cache or if the currentSrc technique doesn't work (Firefox).

If the above CSS fixes things, I'd love to have it PRed into gatsby-image directly.

It's in gatsby-image already. Issue is imgCached is apparently never false in Safari.

@github-actions github-actions bot removed the stale? Issue that may be closed soon due to the original author not responding any more. label Sep 7, 2020
@polarathene
Copy link
Contributor

polarathene commented Sep 20, 2020

I received another report of the issue that answered enough of what I had requested from a Safari user, sorry for how long you've all been affected by this.

PR available, I've not tested it, but it should make Safari users of gatsby-image happy once more.

For clarification, the placeholder transition will happen on first load of the image, there is still an internal gatsby-image cache that skips transition effect if the image has been seen before that session/visit, but that is consistent with all browsers.

@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Oct 11, 2020
@github-actions
Copy link

Hey again!

It’s been 60 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.
Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to comment on this issue or create a new one if you need anything else.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community! 💪💜

@doublejosh
Copy link

This is still happening... affects all browsers and is intermittent. Sounds like an on-going problem.

@polarathene
Copy link
Contributor

Pretty sure I had an optimal fix and contributed it, but the review process dragged on and despite the encouragement with feedback to satisfy merging some were rejected in favor of a replacement image component that was rewritten but still in an alpha/beta state at the time IIRC. Some of my fixes were ported over with no attribution.

I don't have time to look through my PR history on the repo, but you should be able to see what progress I made, I documented plenty of it (in rather verbose comments or commit messages in the PRs). If I did resolve it, then you might be able to point someone to that solution to port it over to whatever Gatsby uses now. I didn't appreciate how that experience panned out, so I no longer contribute or use Gatsby personally.

@shixish
Copy link

shixish commented Nov 13, 2021

Yeah I seem to be having this problem as well.
I'm just going to use placeholder="none" since the placeholder is showing up with opacity: 1; inline style on some page loads, and just getting stuck that way. I've been seeing this happen on Chrome.

Edit: I updated the related modules and it was still doing this when using gatsby build && gatsby serve (it had also been happening live). However, after turning off the placeholder then switching back to blurred it seems to be working ok now. Maybe there's some kind of cache that needed to be rebuilt..? Hard to know if it's going to work 100% of the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale? Issue that may be closed soon due to the original author not responding any more. status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.