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

Visual tool to scan a page and report missing width/height elements #10

Open
adamsilverstein opened this issue Nov 29, 2021 · 13 comments
Open
Assignees
Labels
[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Feature A new feature within an existing module

Comments

@adamsilverstein
Copy link
Member

This task was suggested by Aymen Loukil for the image focus, however it may fit better in the measurement focus.

@eclarke1
Copy link

@adamsilverstein I'll add this to the measurement focus for now and into the backlog, but let me know if that should change.

@eclarke1 eclarke1 added [Focus] Measurement [Type] Feature A new feature within an existing module labels Jan 17, 2022
@dainemawer
Copy link
Contributor

@eclarke1 @adamsilverstein I think I could take a stab at this - sounds like a fun feature to build out.

@eclarke1
Copy link

Thanks @dainemawer we can assign this issue to you then and it would be great to move over to 'In Progress' once this is started please

@dainemawer
Copy link
Contributor

@adamsilverstein before I move onto this ticket - do you have any thoughts with regards to implementation? Is this something that we would invision living in WordPress, or separate, like a chrome extension?

@westonruter
Copy link
Member

Just to note that sometimes authors intentionally omit width/height attributes such as when adding an remote image that changes randomly. See #49 (comment). Surely not the normal case, but I just wanted to mention the use case.

@AymenLoukil AymenLoukil self-assigned this Feb 15, 2022
@dainemawer
Copy link
Contributor

@felixarntz @ThierryA @adamsilverstein @AymenLoukil - I have a minor proof of concept ready here, but I just want to make sure that this is something we actively want to pursue, considering where we currently are with images overall? This feature is blurring the lines between what Lighthouse does pretty well already in terms of flagging images, so I guess my question is - should we rely on Lighthouse and will users know to use it? Or should we looking into incorporating this tool into the plugin?

@swissspidy
Copy link
Member

@westonruter Just stumbled upon this — maybe a candidate for Optimization Detective?

@westonruter
Copy link
Member

@swissspidy Hummm. Yes! Beyond just reporting issues with missing width and height on elements, it can measure the dimensions of those elements and supply the missing width and height attributes during optimization. 🎉

@westonruter westonruter added the [Plugin] Optimization Detective Issues for the Optimization Detective plugin label May 10, 2024
@sstopfer sstopfer moved this to Not Started/Backlog 📆 in WP Performance 2024 May 26, 2024
@sstopfer sstopfer added [Focus] Measurement [Plugin] Optimization Detective Issues for the Optimization Detective plugin and removed [Focus] Measurement [Plugin] Optimization Detective Issues for the Optimization Detective plugin labels May 26, 2024
@westonruter
Copy link
Member

In the case where an image lacks width/height due to the loaded image having variable dimensions, it will be important that we collect multiple URL metrics for a URL before attempting to supply missing width and height attributes. Only once we have multiple URL metrics and the element ends up with the same width/height across each page load can we safely supply the dimensions.

@westonruter westonruter removed their assignment Oct 11, 2024
@westonruter
Copy link
Member

As noted in #1420:

I think detect.js may need to be extend to obtain the intrinsic dimensions of elements that have them: img, video, canvas, svg (maybe), and input[type=image] (ewwww).

So for an image, I believe this would be by getting the naturalWidth & naturalHeight, and for video it would be the videoWidth and videoHeight properties.

With these stored in URL metrics, the optimization pass would then set the missing width and height attributes to match. Another custom attribute should also be added like data-od-using-intrinsic-dimensions so that the detection logic knows it should continue to obtain the intrinsic dimensions for storage, if it isn't doing so already.

@westonruter
Copy link
Member

Related to this, I just learned that VIDEO tags in the Video block are not inserted with width and height attributes, which means they can cause layout shift. See WordPress/gutenberg#52185.

@westonruter westonruter self-assigned this Oct 15, 2024
@westonruter
Copy link
Member

westonruter commented Oct 15, 2024

Here's an example portrait video on a 3G connection without preload specified and no poster:

Screen.recording.2024-10-15.13.52.38.webm
<video controls="" muted="" src="http://localhost:8888/wp-content/uploads/2024/10/goat-18139442-hd_1080_1920_30fps.mp4"></video>

And with a poster supplied, in which case the poster image dimensions become the video's initial intrinsic dimensions:

Screen.recording.2024-10-15.13.56.46.webm
<video controls="" muted="" poster="http://localhost:8888/wp-content/uploads/2024/10/goat-poster-jpg.webp" src="http://localhost:8888/wp-content/uploads/2024/10/goat-18139442-hd_1080_1920_30fps.mp4"></video>

And without poster and with preload=none:

Screen.recording.2024-10-15.13.59.54.webm
<video controls="" muted="" preload="none" src="http://localhost:8888/wp-content/uploads/2024/10/goat-18139442-hd_1080_1920_30fps.mp4"></video>

Note what happens when a landscape image is set as the poster:

Screen.recording.2024-10-15.14.06.56.webm

All of these layout shifts would be eliminated if we supplied the width and height on the VIDEO to correspond to the videoWidth and videoHeight as discovered client-side.

I've also filed #1592 for setting appropriate preload values based on whether a video is in the initial viewport. (Note: I initially commented here in error that the default preload value is auto when in reality it is up to the browser, but the spec encourages metadata as the default.)

@westonruter westonruter moved this from Not Started/Backlog 📆 to To Do 🔧 in WP Performance 2024 Oct 23, 2024
@westonruter westonruter moved this from To Do 🔧 to In Progress 🚧 in WP Performance 2024 Dec 24, 2024
@westonruter
Copy link
Member

I'm exploring this in the context of a separate repo at the moment, although I intend to open a PR to implement it as part of Image Prioritizer once it is stable: https://github.com/westonruter/od-intrinsic-dimensions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Feature A new feature within an existing module
Projects
Status: In Progress 🚧
Development

Successfully merging a pull request may close this issue.

7 participants