Fix/1144 webp support
Description of the Change
This PR:
- [x] adds WebP support to Distributor, for parity with WordPress 5.8
- [x] adds HEIC support to Distributor, for parity with WordPress 6.5
- [x] adds AVIF support to Distributor, for parity with WordPress 6.7
- [ ] adds tests:
- [ ] WebP
- [ ] HEIC
- [ ] AVIF
Closes #810
How to test the Change
Upload test AVIF, HEIC, and WebP files to a Distributor source site. Ensure taht they are synchronized to a downstream Distributor site.
Changelog Entry
Added - Support for WebP, paralleling WordPress 5.8 Added - Support for HEIC, paralleling WordPress 6.5 Added - Support for AVIF, paralleling WordPress 6.7
Credits
~~Props @username, @username2, ...~~
Checklist:
- [x] I agree to follow this project's Code of Conduct.
- [x] I have updated the documentation accordingly.
- [ ] I have added Critical Flows, Test Cases, and/or End-to-End Tests to cover my change.
- [ ] All tests pass:
- [ ] existing
- [ ] new
Before this PR is merged, we should:
- [ ] add tests covering support for WebP, HEIC, and AVIF βΒ There are specific image tests in https://github.com/10up/distributor/blob/develop/tests/php/DistributorPostTest.php but it's not clear to me where the example images come from. The domain names given are fictional or reserved, and the images referenced are not included in this plugin's filesystem. Can support be added simply with mock data in the test case, or do the files need to also exist somewhere?
- [ ] add a test covering a fictional image type, to ensure that we reject disallowed media types
- [ ] discuss how to handle differences in support between Distributor instances running on different versions of WordPress β Should the receiver communicate to the sender which MIME types and other supported things are allowed? Or is it entirely upon the receiving Distributor instance to filter out that which it does not support?
Can support be added simply with mock data in the test case, or do the files need to also exist somewhere?
Yes, tests can be added by mocking the image names. As the tests use https://github.com/10up/wp_mock/ the files don't need to exist in the repo.
This should work for the fictional file type too.
Should the receiver communicate to the sender which MIME types and other supported things are allowed
As some of the supported media types depend on the version of underlying libraries installed (GD, ImageMagick) then I think it would be good for the pushing instance to know what the receiving instance supports.
There's a few REST API endpoints that Distributor registers that the system could make use of. I think we'd only want to make the data available if the user is logged in to the endpoint but we'd need to check for the data during a push, which I don't think is something that's done at the moment.
Maybe add the data to check_post_types_permissions for logged in users and if the data isn't recorded in the meta data prior to pushing, then we can recheck the remote end point.
Flagging that I'm not sure when I'm going to have time to work on this PR again.
@peterwilsoncc I added a few test cases for WebP image types in https://github.com/10up/distributor/pull/1269/commits/b83212b805f56e54d33651ccfaa5873035c78ffe, Can you let me know if that's the right direction to proceed with? If so, I can proceed with HEIC/AVIF image types test cases.
Also, I tested HEIC & AVIF image types, with different versions, and every time it passed to the receiver site whether the receiver site supports that type or not. I'm not sure what I'm doing wrong or WP is acting weird for me. π€·π»ββοΈ
Can you try as well? Just in case I'm missing anything!
@sanketio
I added a few test cases for WebP image types in https://github.com/10up/distributor/commit/b83212b805f56e54d33651ccfaa5873035c78ffe, Can you let me know if that's the right direction to proceed with? If so, I can proceed with HEIC/AVIF image types test cases.
Yes, these tests look good to me. Thank you!
Also, I tested HEIC & AVIF image types, with different versions, and every time it passed to the receiver site whether the receiver site supports that type or not. I'm not sure what I'm doing wrong or WP is acting weird for me. π€·π»ββοΈ
I'm unsure what you mean here:
- were the image types in the array
$allowed_extensions? - by unsupported, do you mean image types that the receiving site can't convert - for example,
heicon a site that doesn't support uploading the mime type?
We can limit the files in $allowed_extensions to what WP reports as supported with the following code but it might be overkill... especially if I am misunderstanding you.
$wp_allowed_image_mimes = array_filter( get_allowed_mime_types(), function( $mime ) {
return str_starts_with( $mime, 'image/' );
} );
$wp_allowed_image_extensions = array();
foreach ( $wp_allowed_image_mimes as $ext => $mime ) {
$wp_allowed_image_extensions = array_merge( $wp_allowed_image_extensions, explode( '|', $ext ) );
}
$dt_allowed_image_extensions = array_intersect( $wp_allowed_image_extensions, array(
'jpg',
'jpeg',
'jpe',
'gif',
'png',
'webp',
'heic',
'avif',
) );
@peterwilsoncc
Also, I tested HEIC & AVIF image types, with different versions, and every time it passed to the receiver site whether the receiver site supports that type or not. I'm not sure what I'm doing wrong or WP is acting weird for me. π€·π»ββοΈ
What II mean by this is, I tried the latest version of WordPress with uploading HEIC & AVIF image types, and then distribute those media files to the receiver site where neither of these image types are supported, but still that receiver site was showing these images just fine.
@sanketio Got you. WordPress converts HEIC images to jpegs on uploads, so that might be why it was working. AVIF images aren't converted so I'm not sure what was happening in that instance. It might have been a reference to the image on the original site, depending on when you last merged in the develop branch.
AVIF images aren't converted so I'm not sure what was happening in that instance. It might have been a reference to the image on the original site, depending on when you last merged in the develop branch.
@peterwilsoncc I tested this again with fresh install, merging latest code from develop branch to the feature branch. But I think I found the reason. I was testing both the sites with the feature branch, as we are already supporting AVIF in this branch, it will work fine, but if we change the receiver site branch to develop, then it is referencing to the source site image. π€¦π»ββοΈ
Now, as we will be allowing AVIF image type when this branch is merged, that shouldn't be a problem, so I'm thinking this is fine π€