Disable AVIF when ImageMagick does not support AVIF transparency#2540
Disable AVIF when ImageMagick does not support AVIF transparency#2540b1ink0 wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #2540 +/- ##
==========================================
+ Coverage 69.64% 69.92% +0.27%
==========================================
Files 90 90
Lines 7736 7753 +17
==========================================
+ Hits 5388 5421 +33
+ Misses 2348 2332 -16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @xtecneo, @wes-davis. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| return false; | ||
| } | ||
|
|
||
| $supported = version_compare( $imagick_version, '6.9.12-68', '>=' ); |
There was a problem hiding this comment.
The -68 here. Is that compared right? In looking at https://www.php.net/manual/en/function.version-compare.php it seems this would cause the version to be considered less than 6.9.12-dev.
There was a problem hiding this comment.
Added tests for different version strings in d76f49b.
| $avif_supported = webp_uploads_mime_type_supported( 'image/avif' ); | ||
| $webp_supported = webp_uploads_mime_type_supported( 'image/webp' ); | ||
| $selected = webp_uploads_get_image_output_format(); | ||
| $avif_supported = webp_uploads_mime_type_supported( 'image/avif' ); |
There was a problem hiding this comment.
I think this should actually be returning false if webp_uploads_imagick_avif_transparency_supported() is returning false.
This is because the plugin will still default to AVIF otherwise at present, even when it is not fully supported:
performance/plugins/webp-uploads/helper.php
Lines 30 to 38 in 4fdf1e9
There was a problem hiding this comment.
If webp_uploads_mime_type_supported() returns false for AVIF when webp_uploads_imagick_avif_transparency_supported() fails, showing a separate settings warning specifically for AVIF transparency support would make the notice logic more complex.
If we are okay with showing the existing generic message that AVIF support is not available, then I think this is fine.
@westonruter you can take over the PR if we want to release today.
There was a problem hiding this comment.
I was thinking of something like "AVIF is supported, but not fully: transparency support is lacking."
Summary
Fixes #2237
Relevant technical choices
This PR adds a helper function to detect whether the installed ImageMagick version is older than
6.9.12-68.If the ImageMagick version does not fully support AVIF transparency, the AVIF option is disabled and a warning notice is shown on the settings screen.
See #2245 (comment) for why
6.9.12-68was chosen, and see the discussion in PR #2245 for additional context.Screenshots
Use of AI Tools
Used Codex to review the code and tests.