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

Blur placeholder stay visible with JavaScript disabled #28251

Closed
hlubek opened this issue Aug 18, 2021 · 5 comments · Fixed by #28269
Closed

Blur placeholder stay visible with JavaScript disabled #28251

hlubek opened this issue Aug 18, 2021 · 5 comments · Fixed by #28269
Assignees
Labels
kind: bug Confirmed bug that is on the backlog
Milestone

Comments

@hlubek
Copy link

hlubek commented Aug 18, 2021

What version of Next.js are you using?

11.1.0

What version of Node.js are you using?

12.22.1

What browser are you using?

Chrom, Firefox, Safari

What operating system are you using?

macOS

How are you deploying your application?

next dev, next start

Describe the Bug

When rendering an image with next/image that has a blur placeholder set:
If JavaScript is disabled in the browser, the img tag with blur placeholder as a background is shown on top of the noscript image. See this screenshot:

nextjs-img-placeholder-noscript

And this is the DOM showing that the later img overlaps the noscript img:

nextjs-img-placeholder-dom

We could solve this with some CSS trickery like:

noscript + img[data-nimg] {
    visibility: hidden;
}

But I think it would be best to change the order of both elements (but it's not clear to me if the order is on purpose here).

Expected Behavior

No blur placeholder should be visible when images are loaded with JavaScript disabled.

To Reproduce

@hlubek hlubek added the bug Issue was opened via the bug report template. label Aug 18, 2021
@hlubek
Copy link
Author

hlubek commented Aug 18, 2021

... I noticed an additional case where the Image has priorityset: no noscript will be rendered in this case and only an img tag with blur placeholder in background is visible with JS disabled.

@styfle styfle added kind: bug Confirmed bug that is on the backlog and removed bug Issue was opened via the bug report template. labels Aug 18, 2021
@styfle styfle self-assigned this Aug 18, 2021
@styfle
Copy link
Member

styfle commented Aug 18, 2021

But I think it would be best to change the order of both elements (but it's not clear to me if the order is on purpose here).

Why would the order matter?

I think the issue is that the blurStyle is applied on the initial page load and then we use JS to remove the blur styles when the image loads. When scripts are disabled, then we can never remove the blurStyle.

However, we're not adding the blurStyle to the <noscript> image so I don't think changing the order would affect anything.

style={{ ...imgStyle, ...blurStyle }}

@styfle
Copy link
Member

styfle commented Aug 18, 2021

I see what you mean about changing the order since the second image will overlay on top of the first.

This should fix it: #28269

Thanks!

@styfle styfle added this to the Iteration 23 milestone Aug 18, 2021
@kodiakhq kodiakhq bot closed this as completed in #28269 Aug 19, 2021
kodiakhq bot pushed a commit that referenced this issue Aug 19, 2021
This PR does a few things:

- Moves `<noscript>` usage below the blur image since so that the `<noscript>` image renders on top of the blur image
- Remove the `isVisible` check for `<noscript>` since we can't rely on client-side JS
- Add `loading=lazy` to the `<noscript>` image to take advantage of native lazy loading (can't rely on JS lazy loading)

Fixes #28251
@styfle
Copy link
Member

styfle commented Aug 19, 2021

This is fixed in v11.1.1-canary.12.

You can try it out today with yarn add next@canary, thanks!

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind: bug Confirmed bug that is on the backlog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants