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

feat: support webp format with jpg fallback #76

Closed
wants to merge 4 commits into from
Closed

feat: support webp format with jpg fallback #76

wants to merge 4 commits into from

Conversation

midudev
Copy link

@midudev midudev commented Feb 26, 2021

In order to improve web performance we're using webp when available.

  • Added support to webp
  • Keep jpg fallback.
CleanShot.2021-02-26.at.22.00.38.mp4

This is addressing some issues from: #70

const img = new Image();
img.onload = () => resolveAndSaveValue(true);
img.onerror = () => resolveAndSaveValue(false);
img.src = 'data:image/webp;base64,UklGRh4AAABXRUJQVlA4TBEAAAAvAAAAAAfQ//73v/+BiOh/AAA=';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are better techniques on checking webp support like

document.createElement("canvas").toDataURL("image/webp").indexOf("data:image/webp") === 0

the example above will actually add new network request and will fail if not whitelisted in CSP img-src data:.
cc @paulirish

Copy link
Author

@midudev midudev Feb 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before adding the one from the PR, I tried that one you're mentioning.

Unfortunately, seems to be problematic with Safari and Firefox.

Anyway, I'm more than happy to change it if the solution based on canvas is more suitable according to @paulirish 👍

PS: You're mentioning "better techniques" in plural. Could you please share the other ones? I'm intrigued. :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could simply use html5 approach

<picture>
    <source srcset=poster.avif type=image/avif>
    <source srcset=poster.webp type=image/webp>
    <img src=poster.jpg>
</picture>

with it can cover more formats, also AVIF.

Copy link
Author

@midudev midudev Mar 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laukstein

document.createElement("canvas")
          .toDataURL("image/webp")
          .indexOf("data:image/webp") === 0

This is returning false on Safari while it supports webp. It seems that is not a good solution. :/

image

Copy link
Owner

@paulirish paulirish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could simply use html5 approach

<picture>
    <source srcset=poster.avif type=image/avif>
    <source srcset=poster.webp type=image/webp>
    <img src=poster.jpg>
</picture>

yeah i think this is my favored solution...

this, of course, means switching from a bg-image to using picture/source/srcset, but I suspect that may improve some other aspects. if anyone wants to give this a shot, i'd appreciate it.

@midudev
Copy link
Author

midudev commented Mar 18, 2021

@paulirish I've it already implemented and working with the HTML approach BUT...

We still might need a way to detect if we have webp support in order to preload the image.

As I see it, we have 3 options here:

a) We remove completely the preload functionality as we're already improving moving to webp.

👍 Less code and it relies on HTML and no JavaScript and weird checks.
👎 No preload might have some penaly perf?

b) We implement the "officialish" webp check support from webp docs: https://developers.google.com/speed/webp/faq#how_can_i_detect_browser_support_for_webp

👍 Preload
👎 More code, runtime aand all webp checks could have some pitfalls now or in the future.

c) We only preload image/webp in case it's supported by the browser.

👍 Preload and supports 90% of the users with almost no code.
👎 No preload for 10% of the users with old browsers.

I've implemented the third option as I think it could be the good one, without checking webp support but...
I think preload is doing nothing as the image is being loaded right away after appending it.
Maybe the a) is the good choice. :)

Waiting your opinion @paulirish.

PS: About the first check of webp support by @laukstein is only working for Chrome.
Safari and Firefox are returning false when they support webp, so it's not reliable.

@Garbee
Copy link
Contributor

Garbee commented Mar 20, 2021

Personally I'd prefer the option that wastes no bandwidth in any case. C, preloading regardless of support, is wasteful for those who don't have support. This may not be a big concern for most, but in places where data usage does cost it is a massive problem. Every byte matters.

I'd default to option A for now, let the browser make the best choice. Then we can analyze responsible preloading solutions on it's own merit separately.

@midudev
Copy link
Author

midudev commented Mar 29, 2021

Hey @paulirish. Just ping, waiting for your feedback. 🙇

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

Successfully merging this pull request may close these issues.

4 participants