Make WordPress Core

Opened 7 weeks ago

Last modified 6 weeks ago

#60932 assigned defect (bug)

Remove PHP capability-block from uploading webp and avif

Reported by: cybr's profile Cybr Owned by: adamsilverstein's profile adamsilverstein
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.5
Component: Media Keywords: avif close
Focuses: administration Cc:

Description

Currently, WordPress wants the server to support editing of the image file types .webp and .avif before allowing them to be uploaded.

This happens in media_upload_form().

// Check if WebP images can be edited.
if ( ! wp_image_editor_supports( array( 'mime_type' => 'image/webp' ) ) ) {
        $plupload_init['webp_upload_error'] = true;
}

// Check if AVIF images can be edited.
if ( ! wp_image_editor_supports( array( 'mime_type' => 'image/avif' ) ) ) {
        $plupload_init['avif_upload_error'] = true;
}

Then, at handler.js, under a deprecated jQuery.ready() call, the errors are considered, and the entire upload is blocked.

} else if ( file.type === 'image/webp' && up.settings.webp_upload_error ) {
        // Disallow uploading of WebP images if the server cannot edit them.
        wpQueueError( pluploadL10n.noneditable_image );
        up.removeFile( file );
        return;
} else if ( file.type === 'image/avif' && up.settings.avif_upload_error ) {
        // Disallow uploading of AVIF images if the server cannot edit them.
        wpQueueError( pluploadL10n.noneditable_image );
        up.removeFile( file );
        return;
}

Since images can still be served without editing them, this block should only be considered when actually editing the image.

Change History (10)

#1 @dd32
7 weeks ago

What about thumbnail generation? I assume that's why it's currently blocking on upload.

#2 @Cybr
7 weeks ago

For the thumbnails, we could either show the original file or show a placeholder.

We could even set a flag when the editor isn't present and clear that flag when support is found again during an upgrade cycle -- whereafter we could generate the thumbnails.

#3 @swissspidy
7 weeks ago

Just using full size images without any thumbnails is a bad idea performance wise.

#4 @adamsilverstein
7 weeks ago

  • Keywords reporter-feedback added

Since images can still be served without editing them, this block should only be considered when actually editing the image.

While this is technically true, users won't benefit from the subsizes and srcset WordPress automatically generates and - as @swissspidy points out - will suffer from the performance implications of always serving the original image. For most users uploading and using the original image is probably not best practice.

That said, I can see a use case for it for some users, so it should be possible to bypass these checks as a developer or with a plugin. Looking at the code you referenced in media_upload_form, you should be able to use the 'plupload_init' filter.

Something like this (untested) code:

add_filter( 'plupload_init', function( $plupload_init ) {
    unset( $plupload_init[ 'avif_upload_error' ] );
    return $plupload_init;
} );

@Cybr can you test this code snippet and see if that resolves the issue for you?

#5 @adamsilverstein
7 weeks ago

  • Keywords avif added

#6 @joemcgill
7 weeks ago

For environments that intentionally offload thumbnail generation, we could potentially only included the edit check if there are intermediate sizes defined.

#7 @Cybr
6 weeks ago

  • Keywords close added; reporter-feedback removed

It slipped my mind that WordPress creates subsizes of images for srcsets. Sorry about that!
With that, yes, it now makes sense to me that uploading is blocked when editing isn't possible.

Should we add a more descriptive error message to alleviate tickets/support questions like mine?

The filter @adamsilverstein provided in comment:4 could be used by plugins that offload thumbnail generation.

#8 @adamsilverstein
6 weeks ago

Should we add a more descriptive error message to alleviate tickets/support questions like mine?

Probably, although it would have to be simple enough that most users could make sense of it.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


6 weeks ago

#10 @adamsilverstein
6 weeks ago

  • Milestone changed from Awaiting Review to 6.6
  • Owner set to adamsilverstein
  • Status changed from new to assigned
  • Version set to 6.5
Note: See TracTickets for help on using tickets.