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

Properly Size Images 54 KB to 53 KB #8927

Closed
halilozanyilmaz opened this issue May 11, 2019 · 7 comments · Fixed by #8940
Closed

Properly Size Images 54 KB to 53 KB #8927

halilozanyilmaz opened this issue May 11, 2019 · 7 comments · Fixed by #8940

Comments

@halilozanyilmaz
Copy link

Lighthouse version, I have no idea how to check it. I use chrome developer tools audit section and there is no information online how to check the version.

Site this issue occurs is ---> tisort.ist (You can recreate the issue by auditing performance from developer tools audit section)

What is the current behavior?

Lighthouse says "Properly Size Images" and it says there is a potention saving about 53 KB for the following image on my website:

images.tisort.ist/spritesheet2.png

well this image is 54 kb. So I have no idea what to do. I minified this image from 2 different image optimization websites (like tinypng) and I don't get any results.

Here screenshot:

image

@patrickhulce
Copy link
Collaborator

You're using an entire spritesheet but it's only used a few times on the page which is why it's showing up here. In order to trigger our spritesheet logic, we have to see it being used at least 3 different times.

If you're loading an entire spritesheet for just a single image, consider splitting out that single image, so your users don't download all of them unnecessarily. That being said, it's just a suggestion and opportunities don't affect the score, so you can feel free to ignore this advice if you feel strongly about the sprite sheet decision.

@halilozanyilmaz
Copy link
Author

halilozanyilmaz commented May 11, 2019

Hi patrick, I counted and the spritesheet is being used for 18 times for my main page.

A quick example is, you can see that it is used around 10 times when you open the menu named "Alışveriş". Below is the picture. Am I missing something here ?

image

@patrickhulce
Copy link
Collaborator

That's on interaction (after hovering/clicking the dropdown), correct? If the images won't be in the DOM when we just load the page without hovering, then Lighthouse will never know they're being used multiple times to know it's a spritesheet.

@halilozanyilmaz
Copy link
Author

No it is not on interaction, the dom is already loaded when entered to my main page.

My site is a live one, you can enter and check yourself. -> tisort.ist

Even if it would be on interaction, there are still 7 images I can count to you which uses spritesheet2

top logo
search magnifier
cover page text logo
footer credit cards image
social media icons at footer x 3

@patrickhulce
Copy link
Collaborator

Thanks for the additional info! Something weird is definitely going on in this gatherer for that page, and we'll have take a look. Apologies for the rote initial replies :)

@connorjclark
Copy link
Collaborator

connorjclark commented May 11, 2019

Gatherer looks right, it does indeed limit the artifact to 2 images per url. It's just that there's no marker for "we think this is a sprite sheet" and thus no way in the audits to ignore them.

  1. improve the heuristic

Currently we just say 3 usages = a sprite sheet.

We can also check that the unique usages are non-overlapping.

  1. add sprite: boolean to each result in the artifact.

  2. ignore the sprites in audits (haven't thought about how, if at all, they could be process differently)

@patrickhulce
Copy link
Collaborator

Hm @hoten I say something is going wrong because we're supposed to exclude the image entirely if it looks like it was a sprite sheet. We even have a smoketest for it.

Looking at the logic again though, it's very confusing and seems like there's a bug where it only accidentally works if there is a certain multiple of usages 😆PR pending :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants