WP_Image_Editor_Imagick creates unexpected 'thumbnail' image size when original image is 150x150

[danielbachhuber]

Discovered with ​https://github.com/wp-cli/media-command/pull/173

If you have brew install imagemagick and pecl install imagick, the issue reproduces with pretty easily with WP-CLI:

$ wp site empty --uploads --yes && wp db reset --yes && wp core install && wp option update uploads_use_yearmonth_folders 0
Success: The site at 'https://vanilla.test' was emptied.
Success: Database reset.
Success: WordPress installed successfully.
Success: Updated 'uploads_use_yearmonth_folders' option.
$ wget http://wp-cli.org/behat-data/white-150-square.jpg
$ wp media import white-150-square.jpg --title="My imported small attachment"
Imported file 'white-150-square.jpg' as attachment ID 4.
Success: Imported 1 of 1 items.
$ ls wp-content/uploads
white-150-square-150x150.jpg   white-150-square.jpg

Only white-150-square.jpg should be present, not white-150-square-150x150.jpg.

The underlying problem is that WP_Image_Editor_Imagick->resize() returns true (​https://github.com/WordPress/wordpress-develop/blob/e993bc5f739af0ac589049c1f102788f57a1983d/src/wp-includes/class-wp-image-editor-imagick.php#L270-L273), and then make_subsize() proceeds to save a new image anyway (​https://github.com/WordPress/wordpress-develop/blob/e993bc5f739af0ac589049c1f102788f57a1983d/src/wp-includes/class-wp-image-editor-imagick.php#L506-L516)

The behavior doesn't happen with WP_Image_Editor_GD because it has some is_gd_image() check (​https://github.com/WordPress/wordpress-develop/blob/e993bc5f739af0ac589049c1f102788f57a1983d/src/wp-includes/class-wp-image-editor-gd.php#L178-L185)

See https://core.trac.wordpress.org/ticket/57370

[danielbachhuber]

​https://github.com/WordPress/wordpress-develop/pull/3796 is one approach at a fix, although I'm not 100% confident it's the most appropriate approach.

Is that new error translate-able?

Is that new error translate-able?

@NekoJonez It's wrapped in __() so it should be? Is there something specific you're uncertain about?

Is that new error translate-able?

@NekoJonez It's wrapped in __() so it should be? Is there something specific you're uncertain about?

Maybe I am confusing with textdomains and such.

[wojtekn]

@danielbachhuber good catch!

I tried to validate it on the wp-env environment, but I can't reproduce the current behavior that we consider incorrect:

1. I open SSH for the container:

wp-env run cli bash

2. I ensure everything is clean:

wp site empty --uploads --yes && wp option update uploads_use_yearmonth_folders 0
Success: The site at 'http://localhost:8888' was emptied.
Success: Value passed for 'uploads_use_yearmonth_folders' option is unchanged.

3. I download the test image:

wget http://wp-cli.org/behat-data/white-150-square.jpg
'white-150-square.jpg' saved

4. I import the file:

wp media import white-150-square.jpg --title="My imported small attachment"
Imported file 'white-150-square.jpg' as attachment ID 1.
Success: Imported 1 of 1 items.

5. I see no additional thumbnail created:

ls wp-content/uploads
white-150-square.jpg

wp-env uses Imagick:

$ php -i | grep imag
/usr/local/etc/php/conf.d/docker-php-ext-imagick.ini,
imagick
imagick module => enabled
imagick module version => 3.6.0
imagick classes => Imagick, ImagickDraw, ImagickPixel, ImagickPixelIterator, ImagickKernel
Imagick compiled with ImageMagick version => ImageMagick 7.1.0-50 beta Q16-HDRI aarch64 20489 https://imagemagick.org

Maybe it happens only for ImageMagick 6.x?

[danielbachhuber]

@wojtekn I'm using 7.1.0-55:

$ brew info imagemagick
==> imagemagick: stable 7.1.0-55 (bottled), HEAD
Tools and libraries to manipulate images in many formats
https://imagemagick.org/index.php
/opt/homebrew/Cellar/imagemagick/7.1.0-55 (807 files, 31.0MB) *
  Poured from bottle on 2022-12-21 at 10:47:11
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/imagemagick.rb
License: ImageMagick
==> Dependencies
Build: pkg-config ✔
Required: freetype ✔, ghostscript ✔, jpeg-turbo ✔, libheif ✔, liblqr ✔, libpng ✔, libraw ✔, libtiff ✔, libtool ✔, little-cms2 ✔, openexr ✔, openjpeg ✔, webp ✔, xz ✔, libomp ✔
==> Options
--HEAD
	Install HEAD version
==> Analytics
install: 184,160 (30 days), 470,977 (90 days), 1,972,966 (365 days)
install-on-request: 162,236 (30 days), 418,097 (90 days), 1,745,374 (365 days)
build-error: 19 (30 days)

[wojtekn]

@danielbachhuber I reproduced the issue with PHP and MariaDB installed by homebrew:

• PHP 8.0.26
• mariadb 10.10.2
• imagick 3.7.0 with ImageMagick 7.1.0-57 (Beta) Q16-HDRI aarch64 20701

Now when I run wp media import white-150-square.jpeg --title="My imported small attachment" I get two files in the wp-content/uploads directory:

• white-150-square-150x150.jpeg
• white-150-square.jpeg

When I apply your patch, I get only one file. It looks like it fixes the issue.

I'm still unsure for which environment setup this issue pops out, but it seems that with your fix, we may have consistent behavior across environments.

[antpb]

Moving this into 6.2 to investigate. Worth noting that I think this issue already in 6.2 is related: #48485

[danielbachhuber]

In 55278:

Media: Bail early if image is already the requested size.

In WP_Image_Editor_Imagick, bail early in make_subsize() if the image is already the requested size. Previously, make_subsize() would create another copy of the file. WP_Image_Editor_GD doesn't have the same problem.

Props wojtekn, danielbachhuber.
Fixes #57370.

Shipped in https://core.trac.wordpress.org/changeset/55278

[danielbachhuber]

In 55280:

Media: Add test file missed in [55278].

See #57370.

See https://core.trac.wordpress.org/ticket/57370

[danielbachhuber]

In 55300:

Media: Use strict comparison in make_subsize().

Follow up from [55278].

Props bueltge, desrosj, mukesh27.
See #57370.

Shipped in https://core.trac.wordpress.org/changeset/55300

[hellofromTonya]

In 55467:

Tests: Improve Tests_Media::test_wp_generate_attachment_metadata_doesnt_generate_sizes_for_150_square_image().

Changes:

• from assertEquals() to assertSame(). Why? To ensure both the return value and data type match the expected results.

• the expected height and width from string to integer data types. Why integer? getimagesize() (within wp_getimagesize()) will return an integer for both height and weight.

• adds the ticket annotation.

• adds assertion failure messages. Why? To denote which assertion failed, which aids in debugging efforts.

Follow-up to [55278].

Props costdev, peterwilsoncc, mukesh27, ankitmaru, hellofromTonya.
See #56800, #57370.