Helpful error on upload of image type not supported by server

[adamsilverstein]

What?

Reject upload of image types not supported by server - match the behavior when trying to upload the same file types in the media library.

This will fix this issue reported in Trac: https://core.trac.wordpress.org/ticket/61167

Why?

Currently users can upload AVIF, WebP or HEIC images even if their web server doesn't support - meaning WordPress will be unable to process the uploaded image into the sub-sized images used for display on the front end. This matches what we do in the media library - we help users by suggesting they convert their images before uploading them

While the upload might look like it is working currently, since the sub-sized images are not created, the front experience is sub-par and the user might never realize this.

How?

• detect webp, avif and heic support server side - similar to the media library
• pass those values from settings thru to the media upload component
• reject uploads the server cannot handle with the same error message used in the media library

Testing Instructions

1. use a server that doesn't support AVIF images
2. try uploading an AVIF image
3. before the patch, check the web page on the front end - note no srcset is created, bedcause sub-sized images were not created
4. apply the patch and try uploading an avif again
5. note the error message

Screenshots or screencast

Media library error:

Post editor error:

Missing srcset

[github-actions]

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[Mamaduka]

Do you think we should return this error on the REST API level? The uploadMedia is just one of its consumers; I'm guessing others would also benefit from his helpful error.

Plus, it removes the need to introduce another config argument for media upload utils.

[Mamaduka]

Related:

• Improve display of HEIC placeholders in post editor #66293
• Improve display of HEIC placeholders in post editor #66293 (comment)

[adamsilverstein]

Do you think we should return this error on the REST API level? The uploadMedia is just one of its consumers; I'm guessing others would also benefit from his helpful error.

Plus, it removes the need to introduce another config argument for media upload utils.

Good idea, I can try that.

[adamsilverstein]

Do you think we should return this error on the REST API level? The uploadMedia is just one of its consumers; I'm guessing others would also benefit from his helpful error.

Plus, it removes the need to introduce another config argument for media upload utils.

@Mamaduka I tried this approach and notice an issue I did not anticipate. When the image is dropped on the canvas, we create a faded version of it to display during the upload. Because the error isn't known until the REST API response, that "ghost" image shows until the error is returned:

upload.demo.mp4

Implementation in a separate branch since I'm not sure we should use it: #68788

[swissspidy]

Instead of adding this new field, can't we just omit the non-supported types from wpAllowedMimeTypes in the first place?

[adamsilverstein]

Instead of adding this new field, can't we just omit the non-supported types from wpAllowedMimeTypes in the first place?

We could, but the experience would be sub-par, see my comment here: https://core.trac.wordpress.org/ticket/61167#comment:20 after trying that exact approach in this PR: WordPress/wordpress-develop#7629.

Basically the error message is cryptic ("Sorry, you are not allowed to upload this file type"), and doesn't help the user figure out what went wrong.

The approach in this PR matches the experience in core->Media - the user is informed that the media type they are trying to upload isn't supported by the server and to try converting to a compatible format like JPEG before uploading.

[swissspidy]

Thanks for pointing to that, very helpful!

Showing a different messages for files that could technically be uploaded but just not resized makes sense to me.

I'm just wondering whether there is a better way to do that.

Right now this touches a lot of files and I'm wary of such a change to the uploadMedia util... Although I can't think of a better way right now. Can we find a better name for that variable though? :)

[adamsilverstein]

@swissspidy - this is ready for an additional review whenever you have a chance!

[Mamaduka]

This is now working as expected. One thing I noticed when testing this is that the placeholder stays even after the upload error. This isn't a new issue (it already happens if there is any other failure) so we can address it separately.

Can you share reproduction steps from trunk? I tried testing with the wp_handle_upload_prefilter filter but couldn't reproduce the issue.

add_filter( 'wp_handle_upload_prefilter', function( $file ) {
	$file['error'] = 'File upload error.';
	return $file;
} );

[swissspidy]

^ I'd also like testing instructions specifically for that. I tried uploading a JXL image in Playground and I see the placeholder for a split second before the error appears and the image is removed.

Can we find a better name for that variable though? :)

How about serverUnsupportedTypes?

renamed in 0964ef6

How about wpUnsupportedMimeTypes? This way it's closer to wpAllowedMimeTypes.

Apart from that, functionality-wise the PR looks good to me.

[adamsilverstein]

Can you share reproduction steps from trunk? I tried testing with the wp_handle_upload_prefilter filter but couldn't reproduce the issue.

@Mamaduka try uploading an AVIF file on a system that doesn't support AVIF.

How about wpUnsupportedMimeTypes? This way it's closer to wpAllowedMimeTypes.

Sure, great suggestion!

[adamsilverstein]

How about wpUnsupportedMimeTypes? This way it's closer to wpAllowedMimeTypes.

Updated in 1fd268b

[adamsilverstein]

@Mamaduka do you want to give this one more test before I merge?

[adamsilverstein]

Also, do I need to address this error?

[Mamaduka]

I'll do another round of tests tomorrow, but it generally looks good.

My only concern is yet another block editor setting. This is an old practice that the project tries to avoid nowadays.

Avoid passing editor settings as inline JS from the backend to the frontend. It is a pattern we used heavily early in the project. We should ideally move away from it and prefer a better separation between client and server (use REST API calls).

See: https://make.wordpress.org/core/2024/09/12/gutenberg-development-practices-and-common-pitfalls/#:~:text=Other%20random%20common%20pitfalls

I know you also started working on #68788. Why did you choose this solution instead of the REST API alternative? Is it only due to the uploading state issue mentioned above? The current solution seems to have a similar problem.

Also, do I need to address this error?

It would make backport tracking easier. We could use it if you're already working on core PR. See: https://github.com/WordPress/gutenberg/blob/trunk/backport-changelog/readme.md

[swissspidy]

My only concern is yet another block editor setting. This is an old practice that the project tries to avoid nowadays.

I had the same thought but since wpAllowedMimeTypes is also passed like that, it doesn't make sense to pass this one differently. If anything, that would need to be done for both (e.g. via select( coreStore ).getEntityRecord( 'root', '__unstableBase' )).

[Mamaduka]

Since the upload is "rejected" by the server as unsupported, would it not make more sense to let the REST API handle the error?

[swissspidy]

Since the upload is "rejected" by the server as unsupported, would it not make more sense to let the REST API handle the error?

That was previously answered here:

Do you think we should return this error on the REST API level? The uploadMedia is just one of its consumers; I'm guessing others would also benefit from his helpful error.
Plus, it removes the need to introduce another config argument for media upload utils.

@Mamaduka I tried this approach and notice an issue I did not anticipate. When the image is dropped on the canvas, we create a faded version of it to display during the upload. Because the error isn't known until the REST API response, that "ghost" image shows until the error is returned:

upload.demo.mp4
Implementation in a separate branch since I'm not sure we should use it: #68788

But now that I re-read it again... This behavior was also mentioned with the current patch, though I can't reproduce it there.

So can we reopen #68788 again to try this? If the error handling is purely done in WP_REST_Attachments_Controller then the feedback might arrive a tick later, but otherwise the user experience would be identical. Theoretically, a change to uploadMedia shouldn't be needed, so it could also be done as a PR against core.

[Mamaduka]

If the error handling is purely done in WP_REST_Attachments_Controller then the feedback might arrive a tick later, but otherwise the user experience would be identical. Theoretically, a change to uploadMedia shouldn't be needed, so it could also be done as a PR against core.

Exactly. The feedback would be identical to any other server upload error. Below is the emulated error on Slow 4G via the wp_handle_upload_prefilter filter.

Screencast

CleanShot.2025-02-05.at.18.16.31.mp4

[adamsilverstein]

If the error handling is purely done in WP_REST_Attachments_Controller then the feedback might arrive a tick later, but otherwise the user experience would be identical. Theoretically, a change to uploadMedia shouldn't be needed, so it could also be done as a PR against core.

Exactly. The feedback would be identical to any other server upload error. Below is the emulated error on Slow 4G via the wp_handle_upload_prefilter filter.

Screencast

CleanShot.2025-02-05.at.18.16.31.mp4

I can retest #68788, The issue there was the flash of the image showing while the upload started, then failed. In the new approach, the supported types are already known, so the UI can respond immediately, without waiting for the REST request to return an error. I will capture screen captures of both approaches so we can compare the results.

I also see some value in knowing which types aren't supported by the server before trying to upload, this could be especially useful to know when leveraging client side image processings.

[Mamaduka]

@adamsilverstein, the screencast I shared on the trunk with an emulated server error. The flash is due to optimistic UI, so I don't think it's breaking anything.

The editor starts uploading > displays the blob as a preview while uploading> the server returns an error > the editor displays a notice and returns to an optimistic state.

[swissspidy]

I also see some value in knowing which types aren't supported by the server before trying to upload, this could be especially useful to know when leveraging client side image processings.

I thought about that too at first, but I am actually not sure about that. If we process and convert all the images on the client, including any sub-sizes, then it doesn't really matter if the server can't do it. So we don't have to be constrained by server limitations anymore.

[adamsilverstein]

I thought about that too at first, but I am actually not sure about that. If we process and convert all the images on the client,

I was thinking about the case where the client is underpowered, say a mobile device. In that case, you might want to defer to the server if you know it supports the format(s) required.

[swissspidy]

Fair point. I think in that case that's best explored separately as part of that effort, as right now such information is provided via the REST API and not block editor settings.

[adamsilverstein]

I used playground to compare #68644 and #68788. Even without network throttling, the "flash" of the image is clearly visible with 68788, however 68644 has the issue of showing the placeholder with a spinner after a failed upload (when dragging the image on to the canvas).

I'm currently favoring #68788 - it is

• A much smaller change, only adding the data to the rest API
• avoids touching editor settings
• doesn't change current behavior other than proving a more helpful error message

Here are the screencasts I recorded. I also tested uploading multiple images at once of various types.

68644

68644.drag.no.flash.placeholder.mp4

pr.68644.-.upload.no.flash.mp4

68788

68788.-.drag.image.-.flash.mp4

pr-68788-upload-flashes.mp4

WDYT?

[swissspidy]

Amazing, super helpful comparison!

#68788 looks good to me as well.

However, can't we just merge that PHP change in WordPress core? I don't see a point in adding this via the plugin. Of course the JS change needs to be done here, but that is a separate enhancement that makes a lot of sense regardless.

[Mamaduka]

However, can't we just merge that PHP change in WordPress core? I don't see a point in adding this via the plugin. Of course the JS change needs to be done here, but that is a separate enhancement that makes a lot of sense regardless.

I think that makes sense.

[adamsilverstein]

However, can't we just merge that PHP change in WordPress core? I don't see a point in adding this via the plugin. Of course the JS change needs to be done here, but that is a separate enhancement that makes a lot of sense regardless.

Good point, I'll move this part back to https://core.trac.wordpress.org/ticket/61167

[adamsilverstein]

However, can't we just merge that PHP change in WordPress core? I don't see a point in adding this via the plugin. Of course the JS change needs to be done here, but that is a separate enhancement that makes a lot of sense regardless.

Good point, I'll move this part back to https://core.trac.wordpress.org/ticket/61167

Core PR adding the error message: WordPress/wordpress-develop#8322

GB PR updated to only update error handling: #68788 (comment)

[adamsilverstein]

Closing in favor of #68788