Original HEIC image is not converted to JPEG

[azaozz]

When uploading a smaller HEIC image, for example 800px wide, the original is not converted to JPEG. That results in WordPress using the .heic image in posts, listing it in the srcset, etc. (The .heic image can only be displayed in Safari, other browsers show a broken image placeholder.)

[azaozz]

Seems to fix this the HEIC images should always be treated as oversized. Even if the original is smaller than the oversized threshold, it has to be converted to a JPEG with the same dimensions.

[ironprogrammer]

Thanks for the report, @azaozz!

Confirming that I see this as well, with the provided sample image and my own cropped and exported HEIC image (using macOS Photos):

<figure class="wp-block-image size-full">
    <img decoding="async" width="800" height="533"
        src="http://wp-src.test/wp-content/uploads/2024/10/sample-800px.heic" alt="" class="wp-image-1921"
        srcset="http://wp-src.test/wp-content/uploads/2024/10/sample-800px.heic 800w,
        http://wp-src.test/wp-content/uploads/2024/10/sample-800px-300x200.jpg 300w,
        http://wp-src.test/wp-content/uploads/2024/10/sample-800px-768x512.jpg 768w"
        sizes="(max-width: 800px) 100vw, 800px">
</figure>

For iOS/macOS users it doesn't seem this should happen often, unless they're cropping in Photos and exporting as HEIC (default is JPEG). Most iOS photos should be above the oversized threshold.

[adamsilverstein]

Seems to fix this the HEIC images should always be treated as oversized. When the original image is smaller than the oversized threshold (2560px), it has to be converted to a JPEG with the same dimensions.

That seems like a reasonable solution.

Trac ticket: https://core.trac.wordpress.org/ticket/62305

Fix converting all heic images to jpeg

Trac ticket: https://core.trac.wordpress.org/ticket/62305

PR may need to be adjusted a bit after https://core.trac.wordpress.org/ticket/62272.

[azaozz]

Seems ​https://github.com/WordPress/wordpress-develop/pull/7656 needs a bit more, couldn't get it to work in all cases here.

Made ​https://github.com/WordPress/wordpress-develop/pull/7667 instead. Uses a slightly different approach, and adds support for converting all HEIF mime types (although results may vary when converting animations/sequences).

[adamsilverstein]

​https://github.com/WordPress/wordpress-develop/pull/7667 looks good, thanks @azaozz! Left some tiny feedback on the PR, otherwise LGTM.

Would be good to get some unit tests for this although manual testing is probably enough confirmation at this point. I will give it a test.

Closing in favor of ​https://github.com/WordPress/wordpress-develop/pull/7667

[azaozz]

Replying to adamsilverstein:

Thanks!

Would be good to get some unit tests for this although manual testing is probably enough confirmation at this point.

Yep, planning to add few tests for this and for #62272, probably reuse the same sample image. But imho manual testing is more important at this point.

[azaozz]

In 59317:

Media: Fix converting of all HEIC/HEIF images to JPEGs after uploading regardless of dimensions.

Props ironprogrammer, adamsilverstein, azaozz.
Fixes #62305.

[azaozz]

Reopen for 6.7.

[ironprogrammer]

Test Report

For a pre-backport confidence check, please note that recent tests performed for related ticket #62272 included images that were smaller than the big image threshold:

• anim-heif-sequence.heic - 256 × 144
• burst-heif-sequence.heic - 640 × 360
• static-heif.heic - 1440 × 960
• static-ios-heic.HEIC - 4032 × 3024 (regression test)

Actual Results

• ✅ In each case, the images were successfully converted to JPEG.

Supplemental Artifacts

The following markup was generated for each "undersized" uploaded/converted image (HTML comment added for clarity):

<!-- anim-heif-sequence.heic -->
<img decoding="async" width="256" height="144"
  src="http://wp-src.test/wp-content/uploads/2024/10/anim-heif-sequence-256x144.jpg"
  alt="" class="wp-image-1929">

<!-- burst-heif-sequence.heic -->
<img fetchpriority="high" decoding="async" width="1024" height="576" data-id="1930"
  src="http://wp-src.test/wp-content/uploads/2024/10/burst-heif-sequence-1024x576.jpg"
  alt="" class="wp-image-1930"
  srcset="http://wp-src.test/wp-content/uploads/2024/10/burst-heif-sequence-1024x576.jpg 1024w,
    http://wp-src.test/wp-content/uploads/2024/10/burst-heif-sequence-300x169.jpg 300w,
    http://wp-src.test/wp-content/uploads/2024/10/burst-heif-sequence-768x432.jpg 768w,
    http://wp-src.test/wp-content/uploads/2024/10/burst-heif-sequence-1280x720.jpg 1280w"
  sizes="(max-width: 1024px) 100vw, 1024px">

<!-- static-heif.heic -->
<img decoding="async" width="1024" height="683" data-id="1931"
  src="http://wp-src.test/wp-content/uploads/2024/10/static-heif-1024x683.jpg"
  alt="" class="wp-image-1931"
  srcset="http://wp-src.test/wp-content/uploads/2024/10/static-heif-1024x683.jpg 1024w,
    http://wp-src.test/wp-content/uploads/2024/10/static-heif-300x200.jpg 300w,
    http://wp-src.test/wp-content/uploads/2024/10/static-heif-768x512.jpg 768w,
    http://wp-src.test/wp-content/uploads/2024/10/static-heif-1440x960.jpg 1440w"
  sizes="(max-width: 1024px) 100vw, 1024px">

[peterwilsoncc]

@azaozz The changes in r59317 will need a follow up. The docblock for the image_editor_output_format filter uses the variable name $output_format but the new code uses the variable name $default_output_format.

I think $output_format is the better name as by the time a plugin the filter runs in a plugin they may no longer be the defaults. Eg, plugin two in this code sample won't get the defaults.

add_filter( 'image_editor_output_format', 'plugin_one_changes', 10 );
add_filter( 'image_editor_output_format', 'plugin_two_changes', 20 );

[azaozz]

Replying to peterwilsoncc:

The changes in r59317 will need a follow up.
...
I think $output_format is the better

Ah, nice catch! Sure, $output_format sounds better. Fixing.

[azaozz]

In 59346:

Media: Better variable name and some docs fixes for the new wp_get_image_editor_output_format().

Props peterwilsoncc, apermo, azaozz.
See #62305.

[azaozz]

Just a note that both [59317] and [59346] need to be merged to the 6.7 branch (in that order).

[mukesh27]

The [59317] changes caused a unit test error for the Modern Format Image plugin. For more details, please see ​https://github.com/WordPress/performance/issues/1634.

[azaozz]

Replying to mukesh27:

The [59317] changes caused a unit test error for the Modern Format Image plugin.

@mukesh27 Thanks for the ping about this!

May be missing something but seems all failing tests there are "negative". I.e. they are to confirm that the originally uploaded image in not converted to the expected format (when smaller than the big image threshold), and only the sub-sizes are converted?

I.e. these tests confirm that when uploading an 1080x720px JPEG image, and the WP_Image_Editor is set to convert JPEG to WEBP, the original image remains JPEG but all sub-sizes are WEBP. This comment on one of the failing tests seems to explain it: ​https://github.com/WordPress/performance/blob/trunk/plugins/webp-uploads/tests/test-load.php#L56.

If that's the case, thinking these tests can be fixed to reflect that this bug in core is now fixed.

[peterwilsoncc]

Thanks for the follow up, Ozz.

I reviewed the failing test and I think you are correct that it's a false negative. The sample image used is 1080px wide (so under the -scaled limit) so it makes sense to me that the image is converted.

I've asked Adam to confirm that so I'll wait until tomorrow morning before doing a backport of [59317] and [59346].

Testing attachment metadata in trunk and 6.7 branch.

Track ticket: https://core.trac.wordpress.org/ticket/62305

The [59317] changes make some changes in attachment metadata.

# In 6.7 branch

<prE>Array
(
    [width] => 1920
    [height] => 1080
    [file] => /tmp//33772.jpg
    [filesize] => 176615
    [sizes] => Array
        (
            [medium] => Array
                (
                    [file] => 33772-300x169.webp
                    [width] => 300
                    [height] => 169
                    [mime-type] => image/webp
                    [filesize] => 55988
                )

            [large] => Array
                (
                    [file] => 33772-1024x576.webp
                    [width] => 1024
                    [height] => 576
                    [mime-type] => image/webp
                    [filesize] => 446208
                )

            [thumbnail] => Array
                (
                    [file] => 33772-150x150.webp
                    [width] => 150
                    [height] => 150
                    [mime-type] => image/webp
                    [filesize] => 31268
                )

            [medium_large] => Array
                (
                    [file] => 33772-768x432.webp
                    [width] => 768
                    [height] => 432
                    [mime-type] => image/webp
                    [filesize] => 279734
                )

        )

    [image_meta] => Array
        (
            [aperture] => 8
            [credit] => Photoshop Author
            [camera] => DMC-LX2
            [caption] => Photoshop Description
            [created_timestamp] => 1306315327
            [copyright] => Photoshop Copyrright Notice
            [focal_length] => 6.3
            [iso] => 100
            [shutter_speed] => 0.0025
            [title] => Photoshop Document Ttitle
            [orientation] => 1
            [keywords] => Array
                (
                    [0] => beach
                    [1] => baywatch
                    [2] => LA
                    [3] => sunset
                )
        )
)

# In current trunk

<prE>Array
(
    [width] => 1920
    [height] => 1080
    [file] => /tmp/33772-1920x1080.webp
    [filesize] => 865924
    [sizes] => Array
        (
            [medium] => Array
                (
                    [file] => 33772-300x169.webp
                    [width] => 300
                    [height] => 169
                    [mime-type] => image/webp
                    [filesize] => 55988
                )

            [large] => Array
                (
                    [file] => 33772-1024x576.webp
                    [width] => 1024
                    [height] => 576
                    [mime-type] => image/webp
                    [filesize] => 446208
                )

            [thumbnail] => Array
                (
                    [file] => 33772-150x150.webp
                    [width] => 150
                    [height] => 150
                    [mime-type] => image/webp
                    [filesize] => 31268
                )

            [medium_large] => Array
                (
                    [file] => 33772-768x432.webp
                    [width] => 768
                    [height] => 432
                    [mime-type] => image/webp
                    [filesize] => 279734
                )

        )

    [image_meta] => Array
        (
            [aperture] => 8
            [credit] => Photoshop Author
            [camera] => DMC-LX2
            [caption] => Photoshop Description
            [created_timestamp] => 1306315327
            [copyright] => Photoshop Copyrright Notice
            [focal_length] => 6.3
            [iso] => 100
            [shutter_speed] => 0.0025
            [title] => Photoshop Document Ttitle
            [orientation] => 1
            [keywords] => Array
                (
                    [0] => beach
                    [1] => baywatch
                    [2] => LA
                    [3] => sunset
                )
        )
    [original_image] => 33772.jpg
)

Check the file key in array. It use alternate mime type image and add the image sizes in name. Before it was /tmp//33772.jpg now it was /tmp/33772-1920x1080.webp. It cause the issues in plugins that use the file and update the metadata. See ​https://github.com/WordPress/performance/issues/1634

[flixos90]

@azaozz @peterwilsoncc I'm not sure the tests are "false negatives".

The assertImageNotHasSource( $attachment_id, 'image/jpeg' ) is to verify that the (full size) image is not available as a JPEG, as for that particular test it shouldn't be - it should only be converted to WebP or AVIF, and the original JPEG image should only be "backed up" under original_image but otherwise there should be no references to JPEG on the attachment.

I'm not sure yet why that behavior changed and whether it's something that needs to be fixed in Core or updated in the plugin to support an intentional Core change. But the assertions themselves still look correct to me.

[flixos90]

I looked into this more closely and found a few reasons for the problem, probably requiring changes in both places for different things:

• The bug reported by @mukesh27 above is an actual bug in the Modern Image Formats plugin itself, that as I now discovered has been there for quite a while, but so far was only "exposed" by big images, which we didn't have test coverage for. 😞 More context in ​https://github.com/WordPress/performance/pull/1635#issuecomment-2460323575. So this needs to be fixed in the plugin.
• As a separate more minor problem, [59317] does not check which MIME type to convert an image to. The image_editor_output_format can receive the same output as input MIME type. The Modern Image Formats plugin for example specifies it dynamically, depending on the configuration. It's definitely possible for the filter to include a map entry from image/jpeg to image/jpeg for example. If you think about it as a conversion map, this may seem weird, but the filter is about "output format", not "conversion map", so we cannot assume it's only used as the latter. Core may therefore currently try to convert a JPEG to a JPEG, which it shouldn't even attempt. We should only set the $convert flag to true if the output format only contains MIME types that differ from the input MIME type.

Minor follow up fix to https://core.trac.wordpress.org/changeset/59317, see https://core.trac.wordpress.org/ticket/62305#comment:34.

Trac ticket: https://core.trac.wordpress.org/ticket/62305

Minor follow up fix to https://core.trac.wordpress.org/changeset/59317, see https://core.trac.wordpress.org/ticket/62305#comment:34.

Trac ticket: https://core.trac.wordpress.org/ticket/62305

[flixos90]

In 59366:

Media: Only mark an image as requiring conversion if the output format differs from the input format.

Follow up to [59317] and [59346].

Props adamsilverstein, peterwilsoncc.
See #62305.

Committed in https://core.trac.wordpress.org/changeset/59366

[flixos90]

Requiring backport to 6.7 branch:

1. [59317]
2. [59346]
3. [59366]

Test group backport to 6.7 brnach

Trac ticket: Core-62305

[peterwilsoncc]

[59317], [59346], [59366] look good for backport.

[peterwilsoncc]

In 59367:

Media: Fix converting of all HEIC/HEIF images to JPEGs after uploading regardless of dimensions.

This backport includes follow up commits to improve a variable name and improve accuracy of when an image needs to be converted.

Reviewed by peterwilsoncc.
Merges [59317], [59346], [59366] to the 6.7 branch.

Props ironprogrammer, adamsilverstein, azaozz, peterwilsoncc, apermo, flixos90.
Fixes #62305.

Committed r59367 / ​https://github.com/WordPress/wordpress-develop/commit/50fc902eb86fe856d8e8ab426bd91d1f0e36738d

I've incorproated these tests in to ​https://github.com/WordPress/wordpress-develop/pull/7748 with a few modifications to use HEIC testing to allow it to rely on the default core features.

There was 1 failure:

1) Tests_Media::test_image_converted_to_other_format_has_correct_filename with data set "do not scale image" (false)
The file name is expected to be 33772.webp
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'33772.webp'
+'33772-1920x1080.webp'

@peterwilsoncc In ​https://github.com/WordPress/wordpress-develop/pull/7733/commits/76a5c39ab9208e11bb194b44dae3bc98a1566ed6 i incorporate changes from PR #7748 but now it return original image instead WebP mime type.

There was 1 failure:

1) Tests_Media::test_image_converted_to_other_format_has_correct_filename with data set "do not scale image" (false)
The file extension is expected to change.
Failed asserting that '/tmp/33772.jpg' ends not with ".jpg".

@mukeshpanchal27 That looks like it was due to the regex error @felixarntz picked up in ​https://github.com/WordPress/wordpress-develop/pull/7748#discussion_r1834688508

@peterwilsoncc Do we need these PR or can we close it?

[peterwilsoncc]

In 59379:

Media: Remove dimension suffix from full size converted HEIC images.

Removes the dimension suffix, eg -1000x1000 from the file name of full size images automatically converted from HEIC to JPEGs by WordPress. Introduces unit tests for the default conversion of images and customized conversion settings via the image_editor_output_format filter.

Follow up to [58849], [58942], [59317], [59346], [59366]

Props mukesh27, peterwilsoncc, azaozz, apermo, flixos90, ironprogrammer.
Fixes #62359.
See #53645, #62305.

@mukeshpanchal27 This was included in the follow up PR.

[davidbaumwald]

In 59381:

Media: Remove dimension suffix from full size converted HEIC images.

Removes the dimension suffix, eg -1000x1000 from the file name of full size images automatically converted from HEIC to JPEGs by WordPress. Introduces unit tests for the default conversion of images and customized conversion settings via the image_editor_output_format filter.

Follow up to [58849], [58942], [59317], [59346], [59366].

Reviewed by davidbaumwald.
Merges [59379] and [59380] to the 6.7 branch.

Props mukesh27, peterwilsoncc, azaozz, apermo, flixos90, ironprogrammer.
Fixes #62359.
See #53645, #62305.