next/image blur placeholder with conditional rendering is taken down too early in Firefox

yfhui commented 2 years ago

What version of Next.js are you using?

11.1.2

What version of Node.js are you using?

16.9.1

What browser are you using?

Firefox

What operating system are you using?

Windows

How are you deploying your application?

Vercel

Describe the Bug

Observation:

next/image placeholder disappears when image is loading in Firefox
The image placeholder is taken down too early at the image loading phase (even before the image starts loading)
In other words, the onload/onLoadingComplete function is fired prematurely and hence the placeholder get unset
This is very obvious when the image component is setup with conditional rendering
With conditional rendering onload function is executed twice, first time is executed before the image is loaded (it runs when rendering variable is set to true)
Without conditional rendering, onload/onLoadingComplete function is still fired early but it run once. However, the time to run onLoad image is still shorter than the time to load the image

conditional rendering example:

import React from 'react'
import Image from 'next/image';
export default function MyImage() {
    console.log('image is rendered');
    let loadTime = Date.now();

    return (
        <Image
            width={1395}
            height={947}
            blurDataURL={
                "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=="
            }
            placeholder="blur"
            src={"/mountains.jpg"}
            onLoadingComplete={() => {
                console.log("test", Date.now() - loadTime);
            }}
        />
    )
}

import MyImage from "../components/image";
import styles from "../styles/Home.module.css";
import { useState } from "react";

export default function Home() {
    const [mountImage, setMountImage] = useState(false);

    console.log('Home is rendered')
    return (
        <div className={styles.container}>
            <button
                onClick={() => {
                    setMountImage(true);
                }}
            >
                click to load image
            </button>
            {mountImage && (
                <MyImage/>
            )}
        </div>
    );
}

log in Firefox, we could see the first time to execute onLoad function is shorter than the image loading time: log in Chrome, we could see the time to execute onLoad function is longer than the image loading time(and it only run once):

Expected Behavior

onLoadingComplete/onload should be run once only. And placeholder exists until the image is fully loaded. (Chrome will execute onLoad once only and gives the expected behavior.)

To Reproduce

A demo is deployed to vercel. Click the button to update the conditional rendering variable. The base code is from npx create-next-app@latest. (complete repo)

https://test-nextjs-peach.vercel.app/

styfle commented 2 years ago

This might be a difference with the way Firefox resolved img.decode()

https://github.com/vercel/next.js/blob/c4e26ab248a9c505bf9aac425f4ff0821cbb641b/packages/next/client/image.tsx#L260-L268

Would you like to submit a PR to fix it?

yfhui commented 2 years ago

Thanks for suggesting the direction. after some investigation I think the reason for the early firing is because of the line if(!img.src.startsWith('data:'))

my discovery:

Chrome will execute onLoad 2 times and Firefox will execute onLoad 3 times (I could not explain why)
For chrome it only executes the things in the decode promise at the 2nd time because the checking of !img.src.startsWith('data:') of the first execution of the onLoad is false, so the decode promise is never registered neither it is resolved.
however in firefox, the checking of !img.src.startsWith('data:') is true in both first and second time of the execution of onLoad function, so the decode promise is registered and resolved at the beginning
I tried to remove !img.src.startsWith('data:') checking and Chrome and Firefox has the same behavior which at the very first time the of the onLoad execution, the handleLoad function has everything executed, placeholder is unset early before image is loaded. I am not sure what exactly I need to do for fixing this bug, if I know what to do, I would love to submit a PR to fix it!

Edit: Noticed that !img.src.startsWith('data:') is changed to if (img.src !== emptyDataURL) in 11.1.3 but tested with 11.1.3-canary.100, the problem still exists

styfle commented 2 years ago

Theres a similar bug reported in #34697. It seems that Firefox implements img.decode() differently so there is a Firefox bug with some discussion here https://bugzilla.mozilla.org/show_bug.cgi?id=1758035

yfhui commented 2 years ago

That's what I suspected too, thx for you update!

bennettdams commented 2 years ago

I hope this is related, but there's also a problem of a blank image while the blur placeholder transitions to the real image.

I couldn't see the bug for the blurred placeholder in your repro, @yfhui, so here is another one that uses a remote image (from Imgur.com). You can see the bug in action:

blurred image -> blank -> final image

Animation

Deployed at: https://nextjs-image-bug.vercel.app/

Here is a reproduction repo, it just uses an image from Imgur.com and a static blur URL.

Firefox 98.0.2 (64-bit)
Next.js 12.1.4

npx create-next-app@latest --ts --use-npm .
Add a remote image
Open the browser and disable cache an throttle the network speed
npm run build && npm run start

This is only an issue in Firefox - Chrome doesn't show this blank space between the placeholder and the final image.

yfhui commented 2 years ago

@bennettdams I noticed this issue too! but I am not sure if it's related to the Firefox bug mentioned above

bennettdams commented 2 years ago

@bennettdams I noticed this issue too! but I am not sure if it's related to the Firefox bug mentioned above

It doesn't happen in Chrome, that's why I posted it in this issue - but maybe it's worth to keep this as a separate issue, maybe someone from the team could let me know?

styfle commented 2 years ago

This should be fixed in PR #35889

styfle commented 2 years ago

One issue https://github.com/vercel/next.js/issues/30128#issuecomment-1086754374 is fixed can you can try it on canary with npm install next@canary.

The original issue https://github.com/vercel/next.js/issues/30128#issue-1032077189 still fails and seems to be related to be either decode() resolving immediately mentioned in https://github.com/vercel/next.js/issues/30128#issuecomment-1063249762 or it could be img.complete is true. It requires a deeper dive.

That being said, the blur placeholder is only acting up on Firefox when its conditionally rendered. Images that are rendered during page load seem to work fine with the blur placeholder.

Etsi0 commented 8 months ago

This is still happening on version 14.0.4

What i have installed on my project:

├── autoprefixer@10.4.16
├── eslint-config-next@14.0.4
├── eslint-config-prettier@9.1.0
├── eslint@8.56.0
├── next@14.0.4
├── postcss@8.4.32
├── react-dom@18.2.0
├── react@18.2.0
└── tailwindcss@3.4.0

Browser:

Firefox: 121.0 (64-bit)

Bug

When the higher quality image begins to load, the placeholder image disappears, but it remains visible in the inspector.

<Image
className='
    w-full
    max-w-prose
    aspect-square
    rounded-2xl
'
src={Selfie}
alt='Test image'
width={562}
height={562}
blurDataURL=' data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAAAKCAMAAAC67D+PAAAAAXNSR0IB2cksfwAAAAlwSFlzAAALEwAACxMBAJqcGAAAAPNQTFRFW0keFREFJR4JSUMtDgwEEQ8EEw4ECgcCCQcCEQ4EWUkfOzIYIR4TCAcDCwgDCwcDBwYCCgcDDwsEUUIcGBQFVUwrCgkDBwUBCggDDQkERDgYGxUFW1IyCAYCCwkDDwwDDgkENy4SIhsITEUsDAkDEg8FNCwQGBQJDAkEDgkDLiUNKCEMPjcgDQsDCAcCGBUGST8cGRQKDQkFKSEKKSIOVkcgDgsDJCAKT0MfGRUKKSAJIxwLY1EfGhUIKCMKT0IeHhkLKiEIFhEFZ1YlNzAYDQoDKCIKTD8bLycQDQgDKiEGbWE4PzkjJSAISTwXRjkXDQgEswbEGAAAAHZJREFUeJxjYGBkYmZhZWPn4GTgYuTm4eXjFxAUYhAWERUT4BMXkJBkkJKWkZCVk5fgU2BQVFJW4VVVU9fQZNDS1tHV0zcwNNJkMDYxNZMwt7CU1GSwsraxlbOzd9BQYHB0cnZxdXP30PBk8HL19pHz9fPnCwAALcMOpQsiKTMAAAAASUVORK5CYII='
placeholder='blur'
/>

Video

The first one is Firefox and the second one is edge

https://github.com/vercel/next.js/assets/43541844/ca344549-94b8-4070-824f-80263ac4852a

vercel / next.js