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

Add "webp" to the valid image types #557

Closed
leofeyer opened this issue Jul 5, 2019 · 7 comments

Comments

Projects
None yet
3 participants
@leofeyer
Copy link
Member

commented Jul 5, 2019

I think we should re-discuss our decision not to add "webp" to the valid image types by default. The feature will work for the majority of users, so we should not force people to overwrite the contao.image.valid_extensions settings – especially because they will have to re-adjust the setting if we change it in the future.

If we really want to hide the options when WEBP is not supported, we should use feature detection.

@leofeyer leofeyer added the defect label Jul 5, 2019

@leofeyer leofeyer added this to the 4.8 milestone Jul 5, 2019

@ausi

This comment has been minimized.

Copy link
Member

commented Jul 6, 2019

Should we use feature detection to add webp to the contao.image.valid_extensions configuration?

Or should we add webp by default and only hide the formats-setting based on feature detection?

@leofeyer

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

The latter IMHO. This way the admin can still disable WEBP images completely.

@Toflar

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

I agree with @leofeyer. Enabling and disabling explicitly is needed.
Also, the check in the image size should include both. So feature detection but also check if webp is present as a valid extension. Otherwise an admin disables webp because they explicitly do not want to have support but one can still choose it in the image size settings. That feels strange to me.

@leofeyer

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

So how do we detect this feature? 😄

@ausi

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

I will create a PR

@ausi

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

I will create a PR

#560

@Toflar

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

Closing in favor of #560

@Toflar Toflar closed this Jul 8, 2019

@leofeyer leofeyer removed this from the 4.8 milestone Jul 8, 2019

leofeyer added a commit that referenced this issue Jul 8, 2019

Detect if WEBP is supported (see #560)
Description
-----------

As discussed in #557

Commits
-------

edbb1ba Detect if WEBP is supported
6538233 Improve wording
7a21f88 Don’t show help message if webp was explicitly disabled
ef0775f Make the tl_image_sizes::isWebpSupported() method private
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.