v8.dev icon indicating copy to clipboard operation
v8.dev copied to clipboard

Explicit image dimensions are ignored and result in significant layout jumps

Open RReverser opened this issue 4 years ago • 9 comments

Currently, we set (or, now, generate) explicit width and height on all images, presumably to avoid layout jumps.

However, these dimensions are getting completely ignored and overridden by https://github.com/v8/v8.dev/blob/cf7598164fdc7a881f93f4a6f058ac95bb5a110e/src/_css/main.css#L412-L416 and result in different sizes once image is loaded.

This results in a significant amount of layout jumps while scrolling through the document, especially due to combination with our extensive usage of loading="lazy".

In order to verify, go to an image-heavy post, e.g. https://v8.dev/blog/pointer-compression/, open DevTools and paste:

[document.body.clientHeight, ...$$('img').map(img => `${img.offsetWidth}x${img.offsetHeight}`)]

On my computer, it results in:

[22875, "600x371", "764x240", "764x336", "764x764", "764x764", "764x764", "764x764", "764x764", "764x764", "764x764", "764x764", "600x600", "600x600", "96x96", "96x96"]

You can see how any images outside the immediate viewport resulted in 1:1 dimensions (squares) with explicit height completely ignored.

Then, scroll through the document to the end and try same expression again:

[18141, "600x371", "764x240", "764x336", "764x336", "764x183", "764x158", "764x336", "764x267", "764x213", "764x103", "764x241", "600x371", "600x371", "96x96", "96x96"]

Now that images are loaded, all the heights got updated to actual dimensions from the image itself and the total document height shrunk by 4,734 pixels.

cc @mathiasbynens and @tomayac as it looks like you gave this some thought as part of removal of intrinsicsize in #231 / #232 and might have ideas on how to fix this.

RReverser avatar Apr 03 '20 16:04 RReverser

Looks like simply removing height: auto fixes the problem, but not 100% sure if there are any unwanted side-effects from doing that?

RReverser avatar Apr 03 '20 17:04 RReverser

Please don’t remove this. See https://youtu.be/4-d_SoCHeWE for why it’s there. All browsers have agreed to put the aspect ratio in the UA stylesheet. It’s gonna be good.

tomayac avatar Apr 03 '20 17:04 tomayac

You seem to be referring to the entire block, but I was suggesting only removing height:auto part. Removing it and keeping max-width seems to result in correct aspect-ratio and dimensions right away, before images are loaded.

See https://youtu.be/4-d_SoCHeWE for why it’s there.

Is there TLDR for this use case?

All browsers have agreed to put the aspect ratio in the UA stylesheet. It’s gonna be good.

Do you mean in future? Can't we do something like above (or anything, really) to make image sizes work correctly today?

RReverser avatar Apr 03 '20 18:04 RReverser

It seems to be working perfectly fine today. There's no content jump at all:

Screen Shot 2020-04-06 at 11 00 36 Screen Shot 2020-04-06 at 11 00 42

It's calculating the aspect ratio based on width and height, leaves exactly the space it needs given the current actual width, and that's it. This is also what we recommend officially.

tomayac avatar Apr 06 '20 09:04 tomayac

@RReverser Not sure if this context was clear, so I'll say it just in case: the height: auto is needed for the case where an image gets downsized on a narrow viewport.

mathiasbynens avatar Apr 06 '20 09:04 mathiasbynens

@mathiasbynens That seems to be doing the opposite, as far as I can tell - if you look at e.g. the 4th image, its specified dimensions are <img width="827" height="363" ... />.

With height: auto it's currently downsized to 764x764 before image is loaded, and to 764x336 when it's loaded; however, when I comment out height: auto, then it's getting downsized with correct aspect-ratio to 764x336 even before the image is loaded, which seems like the desired behaviour?

RReverser avatar Apr 06 '20 11:04 RReverser

@tomayac You're looking at the first image, which, as you can see from the calculated image in issue description, is correct right away anyway. This is because that image is getting preloaded right away, because it's in the viewport, so it's much harder to catch the point where it has incorrect dimensions.

It's images outside the initial viewport (which, correspondingly, aren't visible on screenshot) that are the issue and cause document height to jump - this is visual noticeable only via the length of scrollbar, but still causes layout jumps as you scroll, as can be seen from calculated dimensions.

RReverser avatar Apr 06 '20 11:04 RReverser

It's calculating the aspect ratio based on width and height, leaves exactly the space it needs given the current actual width, and that's it.

It's really not, and that's the point of the issue report :( If we recommend if officially, then it's either a bug in Chrome that needs to be fixed, or we need to change our recommendation.

RReverser avatar Apr 06 '20 11:04 RReverser

however, when I comment out height: auto, then it's getting downsized with correct aspect-ratio to 764x336 even before the image is loaded, which seems like the desired behaviour?

Nvm, removing height: auto causes issue after image is loaded instead.

Would still appreciate suggestions for a proper fix from someone who knows more.

RReverser avatar Apr 06 '20 13:04 RReverser

Declaring bug bankruptcy after 4 years of inactivity.

camillobruni avatar Mar 23 '24 21:03 camillobruni