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.

Sign up for GitHub

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

Jump to bottom

Show error 500 for unsupported image types #1815

Merged
merged 3 commits into from
Jun 19, 2020
Merged

Show error 500 for unsupported image types #1815

merged 3 commits into from
Jun 19, 2020

Conversation

ausi
Copy link
Member

@ausi ausi commented Jun 11, 2020

Q A
Fixed issues Fixes #1090 (comment)

Based on #1814

@ausi
Copy link
Member Author

ausi commented Jun 11, 2020

I’m not sure if we should actually trigger this warning as we include tif, tiff and webp by default in the valid extensions list. This also breaks the tests now because the warning gets always triggered.

@ausi ausi marked this pull request as ready for review June 11, 2020 19:40
@ausi ausi self-assigned this Jun 11, 2020
@ausi ausi requested a review from a team June 11, 2020 19:40
@ausi ausi added the bug label Jun 11, 2020
@ausi ausi added this to the 4.9 milestone Jun 11, 2020
@ausi ausi mentioned this pull request Jun 11, 2020
Closed
@aschempp
Copy link
Member

Am I understanding correctly that eg. GD supports TIFF but ImageMagick does not?

There‘s no way to „retrieve“ the valid image types right?

@ausi
Copy link
Member Author

ausi commented Jun 12, 2020

Am I understanding correctly that eg. GD supports TIFF but ImageMagick does not?

No, I think it’s the other way round. But which image types are supported depends on the setup. Both Imagick and GD can be set up with different supported image formats.

There‘s no way to „retrieve“ the valid image types right?

I don’t think there is.

@ausi ausi mentioned this pull request Jun 12, 2020
Closed
@leofeyer
Copy link
Member

I'm for dropping the verifyValidImageFileExtensions() method again.

@ausi
Copy link
Member Author

ausi commented Jun 19, 2020

I'm for dropping the verifyValidImageFileExtensions() method again.

Done in 127a162

@leofeyer leofeyer merged commit 42fa71f into contao:4.9 Jun 19, 2020
@leofeyer
Copy link
Member

Thank you @ausi.

@ausi ausi mentioned this pull request Jul 12, 2020
Merged
leofeyer pushed a commit that referenced this pull request Jul 15, 2020
@ausi
Improve error message for unsupported image formats (see #1932)
f56d2f0 
Description
-----------

#1913 was another case where the `contao.image.valid_extensions` caused confusion. I think especially because the IMagick error message is not very descriptive. We tried triggering a warning at container build time in #1815 (comment) but concluded that it might not be a good idea.

How about improving the error message of the exception?

Before:
<img width="856" alt="Bildschirmfoto 2020-07-12 um 10 51 56" src="https://user-images.githubusercontent.com/367169/87242551-fcad2700-c42d-11ea-9269-979670e8762b.png">

After:
<img width="857" alt="Bildschirmfoto 2020-07-12 um 10 51 27" src="https://user-images.githubusercontent.com/367169/87242555-033b9e80-c42e-11ea-8128-b622364ee43a.png">

Commits
-------

e44ff13 Improve error message for unsupported image formats
leofeyer pushed a commit to contao/core-bundle that referenced this pull request Jul 15, 2020
@ausi
Improve error message for unsupported image formats (see #1932)
3296102 
Description
-----------

#1913 was another case where the `contao.image.valid_extensions` caused confusion. I think especially because the IMagick error message is not very descriptive. We tried triggering a warning at container build time in contao/contao#1815 (comment) but concluded that it might not be a good idea.

How about improving the error message of the exception?

Before:
<img width="856" alt="Bildschirmfoto 2020-07-12 um 10 51 56" src="https://user-images.githubusercontent.com/367169/87242551-fcad2700-c42d-11ea-9269-979670e8762b.png">

After:
<img width="857" alt="Bildschirmfoto 2020-07-12 um 10 51 27" src="https://user-images.githubusercontent.com/367169/87242555-033b9e80-c42e-11ea-8128-b622364ee43a.png">

Commits
-------

e44ff13c Improve error message for unsupported image formats
@ausi ausi deleted the fix/unsupported-image-types-exception branch December 3, 2021 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leofeyer leofeyer leofeyer approved these changes

Assignees

@ausi ausi

Labels
bug
Projects
None yet
Milestone
4.9
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ausi @aschempp @leofeyer