-
Notifications
You must be signed in to change notification settings - Fork 146
Prevent transparency loss in AVIF by falling back to WebP on older ImageMagick #2245
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
base: trunk
Are you sure you want to change the base?
Changes from 16 commits
ae7f22d
a1aff66
be0817b
3fe6954
2247b61
1a58d1c
fdf65ec
50f2be1
f8af1e9
991d7c9
fece741
9020b24
435f5ff
4da1758
717e768
c033c44
b28885e
6004d18
d218d0f
41f94e1
28f2583
b421017
433ea1c
b00d2d0
28d6020
4cf8302
a28415f
99a2704
92f6528
23f528b
06f67f7
4e1ac64
fec8109
50e74ad
d8acc81
0e0ee09
23e12d5
1045006
789906c
8140089
2c50ed7
d01f2d6
315d6c3
3190c5b
f0beb09
346a61e
3f55c70
59d39dc
be178ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,138 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| <?php | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * WordPress Image Editor Class for Image Manipulation through Imagick | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * for transparency detection | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @package webp-uploads | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // @codeCoverageIgnoreStart | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( ! defined( 'ABSPATH' ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| exit; // Exit if accessed directly. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // @codeCoverageIgnoreEnd | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( class_exists( 'WebP_Uploads_Image_Editor_Imagick_Base' ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * WordPress Image Editor Class for Image Manipulation through Imagick | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * for transparency detection. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @see WP_Image_Editor | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| class WebP_Uploads_Image_Editor_Imagick extends WebP_Uploads_Image_Editor_Imagick_Base { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * The current instance of the image editor. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @var WebP_Uploads_Image_Editor_Imagick|null $current_instance The current instance. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| public static $current_instance = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Stores already checked images for transparency. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @var array<string, bool> Associative array with file paths as keys and transparency detection results as values. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| private static $checked_images = array(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Load the image and set the current instance. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return WP_Error|true True on success, WP_Error on failure. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| public function load() { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // @phpstan-ignore-next-line -- Parent class is created via class_alias at runtime. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| $result = parent::load(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( ! is_wp_error( $result ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self::$current_instance = $this; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return $result; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Get the file path of the image. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return string The file path of the image. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| public function get_file(): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( property_exists( $this, 'file' ) && is_string( $this->file ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return $this->file; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Looks for transparent pixels in the image. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * If there are none, it returns false. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @since n.e.x.t | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return bool|WP_Error True or false based on whether there are transparent pixels, or an error on failure. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| public function has_transparency() { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( ! property_exists( $this, 'image' ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( ! (bool) $this->image ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return new WP_Error( 'image_editor_has_transparency_error_no_image', __( 'Transparency detection no image found.', 'webp-uploads' ) ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| $file_path = $this->get_file(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( isset( self::$checked_images[ $file_path ] ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| return self::$checked_images[ $file_path ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( ! $this->image instanceof Imagick ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Check if the image has an alpha channel if false, then it can't have transparency so return early. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Note that Imagick::getImageAlphaChannel() is only available if Imagick | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * has been compiled against ImageMagick version 6.4.0 or newer. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( is_callable( array( $this->image, 'getImageAlphaChannel' ) ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( Imagick::ALPHACHANNEL_UNDEFINED === $this->image->getImageAlphaChannel() ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| self::$checked_images[ $file_path ] = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Walk through the pixels and look transparent pixels. | ||||||||||||||||||||||||||||||||||||||||||||||||||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| $w = $this->image->getImageWidth(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| $h = $this->image->getImageHeight(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| for ( $x = 0; $x < $w; $x++ ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| for ( $y = 0; $y < $h; $y++ ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| $pixel = $this->image->getImagePixelColor( $x, $y ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| $color = $pixel->getColor( 2 ); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( $color['a'] > 0 && $color['a'] < 255 ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| self::$checked_images[ $file_path ] = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Walk through the pixels and look transparent pixels. | |
| $w = $this->image->getImageWidth(); | |
| $h = $this->image->getImageHeight(); | |
| for ( $x = 0; $x < $w; $x++ ) { | |
| for ( $y = 0; $y < $h; $y++ ) { | |
| $pixel = $this->image->getImagePixelColor( $x, $y ); | |
| $color = $pixel->getColor( 2 ); | |
| if ( $color['a'] > 0 && $color['a'] < 255 ) { | |
| self::$checked_images[ $file_path ] = true; | |
| return true; | |
| } | |
| } | |
| } | |
| // Use the image histogram to look for partially transparent pixels more efficiently. | |
| $histogram = $this->image->getImageHistogram(); | |
| foreach ( $histogram as $pixel ) { | |
| // Get normalized color values (0-1), including alpha channel. | |
| $color = $pixel->getColor( true ); | |
| // Match previous behavior: consider pixels with partial transparency only. | |
| if ( isset( $color['a'] ) && $color['a'] > 0 && $color['a'] < 1 ) { | |
| self::$checked_images[ $file_path ] = true; | |
| return true; | |
| } | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure about this. Histogram calculation can also be expensive, and a large image like 4000×3000 could have millions of unique colors. It might be faster for smaller images, though. I’ll run some tests with different types of images first to get actual numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about using getImageChannelMean method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have found a way to make this more efficient using getImageChannelMean and getImageChannelRange done in 28d6020.
example:
$rgb_mean = $this->image->getImageChannelMean( Imagick::CHANNEL_ALL );
$alpha_range = $this->image->getImageChannelRange( Imagick::CHANNEL_ALPHA );
if ( isset( $rgb_mean['mean'], $alpha_range['maxima'] ) ) {
$maxima = (int) $alpha_range['maxima'];
$mean = (int) $rgb_mean['mean'];
if ( 0 > $maxima || 0 > $mean ) {
// For invalid values assume no transparency.
$transparency = false;
} elseif ( 0 === $maxima && 0 === $mean ) {
// Alpha channel is all zeros AND no RGB content indicates fully transparent image.
$transparency = true;
} elseif ( 0 === $maxima && $mean > 0 ) {
// Alpha maxima of 0 with RGB content present indicates no real alpha channel exists (hence fully opaque).
$transparency = false;
} elseif ( 0 < $maxima && 0 < $mean ) {
// Non-zero alpha values with RGB content present indicates some transparency.
$transparency = true;
}
}| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,12 +21,18 @@ | |
| * @since 2.0.0 Added support for AVIF. | ||
| * @since 2.2.0 Added support for PNG. | ||
| * | ||
| * @param string|null $filename Optional. The filename. Default null. | ||
| * @return array<string, array<string>> An array of valid mime types, where the key is the mime type and the value is the extension type. | ||
| */ | ||
| function webp_uploads_get_upload_image_mime_transforms(): array { | ||
| function webp_uploads_get_upload_image_mime_transforms( ?string $filename = null ): array { | ||
| $avif_support = webp_uploads_mime_type_supported( 'image/avif' ); | ||
|
|
||
| if ( $avif_support && webp_uploads_check_image_transparency( $filename ) ) { | ||
| $avif_support = false; | ||
|
||
| } | ||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // Check the selected output format. | ||
| $output_format = webp_uploads_mime_type_supported( 'image/avif' ) ? webp_uploads_get_image_output_format() : 'webp'; | ||
| $output_format = $avif_support ? webp_uploads_get_image_output_format() : 'webp'; | ||
|
|
||
| $default_transforms = array( | ||
| 'image/jpeg' => array( 'image/' . $output_format ), | ||
|
|
@@ -512,3 +518,71 @@ function webp_uploads_get_attachment_file_mime_type( int $attachment_id, string | |
| $mime_type = $filetype['type'] ?? get_post_mime_type( $attachment_id ); | ||
| return is_string( $mime_type ) ? $mime_type : ''; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if Imagick has AVIF transparency support. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @return bool True if Imagick has AVIF transparency support, false otherwise. | ||
| */ | ||
| function webp_uploads_imagick_avif_transparency_supported(): bool { | ||
| if ( extension_loaded( 'imagick' ) && class_exists( 'Imagick' ) ) { | ||
| $imagick_version = Imagick::getVersion(); | ||
| if ( (bool) preg_match( '/\d+(?:\.\d+)+(?:-\d+)?/', $imagick_version['versionString'], $matches ) ) { | ||
| $imagick_version = $matches[0]; | ||
| } else { | ||
| $imagick_version = $imagick_version['versionString']; | ||
| } | ||
| return version_compare( $imagick_version, '7.0.25', '>=' ); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an AVIF image has transparency | ||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param string|null $filename The uploaded file name. | ||
| * @return bool Whether the image has transparency. | ||
| */ | ||
| function webp_uploads_check_image_transparency( ?string $filename ): bool { | ||
| static $processed_images = array(); | ||
|
|
||
| if ( 'avif' !== webp_uploads_get_image_output_format() || webp_uploads_imagick_avif_transparency_supported() || ! class_exists( 'WebP_Uploads_Image_Editor_Imagick' ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| if ( null === $filename ) { | ||
| if ( null === WebP_Uploads_Image_Editor_Imagick::$current_instance ) { | ||
| return false; | ||
| } | ||
| $file = WebP_Uploads_Image_Editor_Imagick::$current_instance->get_file(); | ||
| if ( '' === $file ) { | ||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return false; | ||
| } | ||
| $filename = $file; | ||
| } | ||
|
|
||
| if ( ! is_string( $filename ) || ! file_exists( $filename ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| if ( isset( $processed_images[ $filename ] ) ) { | ||
| return $processed_images[ $filename ]; | ||
| } | ||
| $processed_images[ $filename ] = false; | ||
|
|
||
| $editor = wp_get_image_editor( $filename ); | ||
|
||
|
|
||
| if ( is_wp_error( $editor ) || ! $editor instanceof WebP_Uploads_Image_Editor_Imagick ) { | ||
| return false; | ||
| } | ||
|
|
||
| $has_transparency = $editor->has_transparency(); | ||
| $processed_images[ $filename ] = is_wp_error( $has_transparency ) ? false : $has_transparency; | ||
|
|
||
| return $processed_images[ $filename ]; | ||
| } | ||
|
Comment on lines
+520
to
+625
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -65,8 +65,7 @@ function webp_uploads_create_sources_property( array $metadata, int $attachment_ | |||||
| return $metadata; | ||||||
| } | ||||||
|
|
||||||
| $valid_mime_transforms = webp_uploads_get_upload_image_mime_transforms(); | ||||||
|
|
||||||
| $valid_mime_transforms = webp_uploads_get_upload_image_mime_transforms( $file ); | ||||||
| // Not a supported mime type to create the sources property. | ||||||
| if ( ! isset( $valid_mime_transforms[ $mime_type ] ) ) { | ||||||
| return $metadata; | ||||||
|
|
@@ -334,7 +333,7 @@ function webp_uploads_filter_image_editor_output_format( $output_format, ?string | |||||
| } | ||||||
|
|
||||||
| // Use the original mime type if this type is allowed. | ||||||
| $valid_mime_transforms = webp_uploads_get_upload_image_mime_transforms(); | ||||||
| $valid_mime_transforms = webp_uploads_get_upload_image_mime_transforms( $filename ); | ||||||
| if ( | ||||||
| ! isset( $valid_mime_transforms[ $mime_type ] ) || | ||||||
| in_array( $mime_type, $valid_mime_transforms[ $mime_type ], true ) | ||||||
|
|
@@ -970,3 +969,50 @@ function webp_uploads_convert_palette_png_to_truecolor( $file ): array { | |||||
| } | ||||||
| add_filter( 'wp_handle_upload_prefilter', 'webp_uploads_convert_palette_png_to_truecolor' ); | ||||||
| add_filter( 'wp_handle_sideload_prefilter', 'webp_uploads_convert_palette_png_to_truecolor' ); | ||||||
|
|
||||||
| /** | ||||||
| * Overloads wp_image_editors() to load the extended class when AVIF transparency is not supported. | ||||||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| * | ||||||
| * @since n.e.x.t | ||||||
| * | ||||||
| * @param string[] $editors Array of available image editor class names. Defaults are 'WP_Image_Editor_Imagick', 'WP_Image_Editor_GD'. | ||||||
| * @return string[] Registered image editors class names. | ||||||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| */ | ||||||
| function webp_uploads_set_image_editors( array $editors ): array { | ||||||
b1ink0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| if ( 'avif' !== webp_uploads_get_image_output_format() || webp_uploads_imagick_avif_transparency_supported() ) { | ||||||
| return $editors; | ||||||
| } | ||||||
|
|
||||||
| if ( 0 === count( $editors ) || false === array_search( WP_Image_Editor_Imagick::class, $editors, true ) ) { | ||||||
| return $editors; | ||||||
| } | ||||||
|
|
||||||
| if ( ! class_exists( $editors[0] ) ) { | ||||||
| return $editors; | ||||||
| } | ||||||
|
|
||||||
| if ( ! class_exists( 'WebP_Uploads_Image_Editor_Imagick_Base' ) ) { | ||||||
| if ( WP_Image_Editor_Imagick::class !== $editors[0] ) { | ||||||
| if ( ! is_subclass_of( WP_Image_Editor_Imagick::class, $editors[0] ) ) { | ||||||
|
||||||
| if ( ! is_subclass_of( WP_Image_Editor_Imagick::class, $editors[0] ) ) { | |
| if ( ! is_subclass_of( $editors[0], WP_Image_Editor_Imagick::class ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, instanceof is easier to read and generally better, AFAIK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that we don’t have an instance here we only have the class name as a string. That’s why I used is_subclass_of().
Outdated
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The webp_uploads_set_image_editors() function lacks test coverage. Given the repository's comprehensive test coverage for other functions and the complex conditional logic with class aliasing in this function (lines 981-1018), tests should be added to verify correct editor registration behavior under different scenarios: when AVIF support exists, when it doesn't, with different editor configurations, etc.
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,51 @@ | ||||||||||||
| <?php | ||||||||||||
| /** | ||||||||||||
| * Helper functions used for checking Imagick AVIF transparency support. | ||||||||||||
| * | ||||||||||||
| * @package webp-uploads | ||||||||||||
| * @since n.e.x.t | ||||||||||||
| */ | ||||||||||||
|
|
||||||||||||
| // @codeCoverageIgnoreStart | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious why this needs to be ignored for code coverage?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I know, this section gets flagged by Codecov as the constant is always defined in the test environment, and as there is no way to undefine or modify a const value in PHP, we have to ignore it.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's right. However, this could be made slightly more concise: if ( ! defined( 'ABSPATH' ) ) {
exit; // @codeCoverageIgnore
}This could be done at once for all files in the repo, since they all have it now, for example: performance/plugins/performance-lab/load.php Lines 18 to 22 in b4f9898
|
||||||||||||
| if ( ! defined( 'ABSPATH' ) ) { | ||||||||||||
| exit; // Exit if accessed directly. | ||||||||||||
| } | ||||||||||||
| // @codeCoverageIgnoreEnd | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Callback for Imagick AVIF transparency support test. | ||||||||||||
| * | ||||||||||||
| * @since n.e.x.t | ||||||||||||
| * | ||||||||||||
| * @return array{label: string, status: string, badge: array{label: string, color: string}, description: string, actions: string, test: string} Result. | ||||||||||||
| */ | ||||||||||||
| function webp_uploads_imagick_avif_transparency_supported_test(): array { | ||||||||||||
| $result = array( | ||||||||||||
| 'label' => __( 'Your site supports AVIF image format transparency with ImageMagick', 'webp-uploads' ), | ||||||||||||
| 'status' => 'good', | ||||||||||||
| 'badge' => array( | ||||||||||||
| 'label' => __( 'Performance', 'webp-uploads' ), | ||||||||||||
| 'color' => 'blue', | ||||||||||||
| ), | ||||||||||||
| 'description' => sprintf( | ||||||||||||
| '<p>%s</p>', | ||||||||||||
| __( 'Older versions of ImageMagick do not support transparency in AVIF images, which can result in loss of transparency when uploading AVIF files.', 'webp-uploads' ) | ||||||||||||
| ), | ||||||||||||
| 'actions' => sprintf( | ||||||||||||
| '<p><strong>%s</strong></p>', | ||||||||||||
| __( 'Your ImageMagick installation supports AVIF transparency.', 'webp-uploads' ) | ||||||||||||
| ), | ||||||||||||
| 'test' => 'is_imagick_avif_transparency_supported_enabled', | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| if ( ! webp_uploads_imagick_avif_transparency_supported() ) { | ||||||||||||
| $result['status'] = 'recommended'; | ||||||||||||
| $result['label'] = __( 'Your site does not support AVIF transparency', 'webp-uploads' ); | ||||||||||||
| $result['actions'] = sprintf( | ||||||||||||
| '<p>%s</p>', | ||||||||||||
| __( 'Update ImageMagick to the latest version by contacting your hosting provider.', 'webp-uploads' ) | ||||||||||||
| ); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| return $result; | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+22
to
+51
|
||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make more sense to move this Site Health test to the Modern Image Formats plugin?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just seeing this comment. I think it makes sense to move all Site Health tests for images to the plugin. See #1781 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?php | ||
| /** | ||
| * Hook callbacks used for checking Imagick AVIF transparency support. | ||
| * | ||
| * @package webp-uploads | ||
| * @since n.e.x.t | ||
| */ | ||
|
|
||
| // @codeCoverageIgnoreStart | ||
| if ( ! defined( 'ABSPATH' ) ) { | ||
| exit; // Exit if accessed directly. | ||
| } | ||
| // @codeCoverageIgnoreEnd | ||
|
|
||
| /** | ||
| * Adds tests to site health. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param array{direct: array<string, array{label: string, test: string}>} $tests Site Health Tests. | ||
| * @return array{direct: array<string, array{label: string, test: string}>} Amended tests. | ||
| */ | ||
| function webp_uploads_add_imagick_avif_transparency_supported_test( array $tests ): array { | ||
| $tests['direct']['imagick_avif_transparency_supported'] = array( | ||
| 'label' => __( 'Imagick AVIF Transparency Support', 'webp-uploads' ), | ||
| 'test' => 'webp_uploads_imagick_avif_transparency_supported_test', | ||
| ); | ||
| return $tests; | ||
| } | ||
| add_filter( 'site_status_tests', 'webp_uploads_add_imagick_avif_transparency_supported_test' ); |
Uh oh!
There was an error while loading. Please reload this page.