Multi-mime support, use WebP for content when smaller #2393

adamsilverstein · 2022-03-08T15:40:39Z

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

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

…_wp_get_sources_from_meta

…pes. * Revert to 0 (first element) as default mime type ofr sub sizes

…ss-develop-fork into add/webp-uploads

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>

adamsilverstein · 2022-07-20T01:50:08Z

You might want to try to add some tests for wp_delete_attachment() to cover the new parts so that it does delete the files for the additional sources as expected

I see we do have it_should_remove_the_generated_webp_images_when_the_attachment_is_deleted (and a few other related tests) in tests/phpunit/tests/image/editor.php (and I see some since tags need updating in there). Easy to miss this file as GitHub has it collapsed by default! I will check if this can be expanded at all.

adamsilverstein · 2022-07-20T14:56:18Z

regarding reading EXIF data, I found another instance we we have a filterable list of types which support reading EXIF data; I wonder if we should use the same list (plus WebP) here, looks like tiff image which we support also include EXIF data:

wordpress-develop/src/wp-admin/includes/image.php

Lines 833 to 843 in 57f4d6c

    
           	/** 
        
           	 * Filters the image types to check for exif data. 
        
           	 * 
        
           	 * @since 2.5.0 
        
           	 * 
        
           	 * @param int[] $image_types Array of image types to check for exif data. Each value 
        
           	 *                           is usually one of the `IMAGETYPE_*` constants. 
        
           	 */ 
        
           	$exif_image_types = apply_filters( 'wp_read_image_metadata_types', array( IMAGETYPE_JPEG, IMAGETYPE_TIFF_II, IMAGETYPE_TIFF_MM ) ); 
        
           	if ( is_callable( 'exif_read_data' ) && in_array( $image_type, $exif_image_types, true ) ) {

adamsilverstein · 2022-07-20T15:00:27Z

@felixarntz in 253e908 I tried switching from set_mime_type to set_output_mime_type. Because we are only setting the output type now, the main image mime data remains the original type, so we no longer need to remove the exif check only for jpeg part. I'll do more testing to make sure this works as expected (so far, so good).

felixarntz · 2022-07-20T15:58:36Z

@adamsilverstein Pushed one tiny code fix in ab79356.

felixarntz · 2022-07-20T16:02:07Z

src/wp-includes/class-wp-image-editor.php

 	 * @param string $dest_path
 	 * @param string $extension
 	 * @return string filename


@adamsilverstein Nitpick, but since we've now added documentation for $suffix, it may be good to document the other parameters and the return value as well.

Documented in feaea78 (#2393)

adamsilverstein · 2022-07-20T20:14:03Z

Addressing final feedback and looking into failing test, the output_mime_type change is breaking the test_set_quality_with_image_conversion test and I'm digging into what the best fix is.

…ses correctly

…ss-develop-fork into add/webp-uploads

adamsilverstein · 2022-07-20T20:41:34Z

Fix for the final test issue in ebe3ad4 (#2393)

src/wp-includes/class-wp-image-editor.php

felixarntz

LGTM! From my perspective this is basically good to commit now 🎉

getsource · 2022-07-21T15:44:01Z

src/wp-admin/includes/image.php

 	// Check if any of the new sizes already exist.
-	if ( isset( $image_meta['sizes'] ) && is_array( $image_meta['sizes'] ) ) {
-		foreach ( $image_meta['sizes'] as $size_name => $size_meta ) {
+	if ( isset( $sizes ) && is_array( $sizes ) ) {


I may be missing something here, but thought I'd check in.
It looks like this is checking $sizes, which is set to array() just above and can only be empty.

Why is $image_meta['sizes'] no longer checked?

I haven't made it through the rest of the PR yet, so it's possible I'm missing context.
If this is no longer needed due to other flow changes, would it make sense to change things, since the variable is set to array() just above?

Good catch! this was a bug probably from a late refactor that would only affect retries (when _wp_make_subsizes is used). I restored the previous behavior now - the only difference from trunk now is adding the mime type checks so each mime type can be checked.

Fixed in f1db696 (#2393)

I verified retries work by setting my PHP timeout very low and uploading a large image.

adamsilverstein · 2022-07-21T18:07:32Z

closed in https://core.trac.wordpress.org/changeset/53751

adamsilverstein added 15 commits March 7, 2022 13:38

Core patch, first pass

a96ea8a

Store the sources meta

fa8b9ef

Delete additional mime types in wp_delete_attachment_files

32807da

Cleanup, remove unintended changes

c930645

Add full size and root level meta

3db70bd

Add alternate mime full sizes to wp_delete_attachment_files

62855be

Update image tag output to use specified mime type.

54d4346

Ensure full size image generated, sources populated

daaacba

Handle the full size image in image tags

958e752

Cleanup, documentation

80a78d1

Unroll full sized image generation to single loop, dry out code with …

f2d0893

…_wp_get_sources_from_meta

remove duplicate editor construction

0822850

Merge branch 'trunk' into add/webp-uploads

1caa589

Add mime type support check in loops, skip trying unsupported mime ty…

bd9a90c

…pes. * Revert to 0 (first element) as default mime type ofr sub sizes

Docblock improvements & cleanup

f863431

adamsilverstein force-pushed the add/webp-uploads branch from 2fc4fe7 to f863431 Compare March 8, 2022 22:23

adamsilverstein added 4 commits March 8, 2022 16:01

Cleanup and match filter name to issue

cbd4ce7

Add a new ‘wp_content_image_mimes’ filter for output

810818a

Clean up and document image replacement approach.

892b34c

Add default value to make_subsize functions

0d38d2e

adamsilverstein force-pushed the add/webp-uploads branch from a0bed59 to 0d38d2e Compare March 9, 2022 15:56

Add and improve tests

775f4ff

adamsilverstein force-pushed the add/webp-uploads branch from 2b8acf3 to 775f4ff Compare March 9, 2022 18:50

Add mapping for PDF images to export as jpeg

e61ec6b

adamsilverstein force-pushed the add/webp-uploads branch 5 times, most recently from 2e6bd30 to 503a299 Compare March 10, 2022 00:36

Add ‘sources’ to test data, clean up tests

913c157

adamsilverstein and others added 2 commits July 19, 2022 19:22

Merge branch 'add/webp-uploads' of github.com:adamsilverstein/wordpre…

1a4b3ad

…ss-develop-fork into add/webp-uploads

Adjust tests removing size

237b6e9

Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>

Try: set output mime type instead of setting mime type

253e908

adamsilverstein and others added 3 commits July 20, 2022 09:14

only use output mime type when mime type not passed

9ecddf9

phpcs

2325da1

Clean up one remaining use of mime_type_set which is no longer needed.

ab79356

Clarify doc comment and update other doc comment to include JPEG again.

a224f58

felixarntz reviewed Jul 20, 2022

View reviewed changes

adamsilverstein added 3 commits July 20, 2022 14:36

Reset mime types to ensure test_set_quality_with_image_conversion pas…

ebe3ad4

…ses correctly

Merge branch 'add/webp-uploads' of github.com:adamsilverstein/wordpre…

8729f5b

…ss-develop-fork into add/webp-uploads

Merge branch 'trunk' into add/webp-uploads

5f06019

adamsilverstein added 4 commits July 20, 2022 14:51

Clarify doc block for adding extension to generated name

06d61e6

Document all parameters for generate_filename

feaea78

Clean up test doc blocks

5a4cb4d

phpcs fixes

6f6f6b4

felixarntz reviewed Jul 20, 2022

View reviewed changes

src/wp-includes/class-wp-image-editor.php Outdated Show resolved Hide resolved

felixarntz approved these changes Jul 20, 2022

View reviewed changes

adamsilverstein added 2 commits July 20, 2022 15:38

Fix doc block typo

5dbcd64

Restore empty line removed inadvertently

425468a

getsource reviewed Jul 21, 2022

View reviewed changes

Correct usage of sizes in _wp_make_subsizes

f1db696

adamsilverstein closed this Jul 21, 2022

adamsilverstein mentioned this pull request Aug 23, 2022

Optimize compression levels for WebP, test file sizes vs. JPEGs WordPress/performance#7

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Multi-mime support, use WebP for content when smaller #2393

Multi-mime support, use WebP for content when smaller #2393

Uh oh!

adamsilverstein commented Mar 8, 2022 •

edited

Loading

Uh oh!

adamsilverstein commented Jul 20, 2022 •

edited

Loading

Uh oh!

adamsilverstein commented Jul 20, 2022

Uh oh!

adamsilverstein commented Jul 20, 2022

Uh oh!

felixarntz commented Jul 20, 2022

Uh oh!

felixarntz Jul 20, 2022

Uh oh!

adamsilverstein Jul 20, 2022

Uh oh!

adamsilverstein commented Jul 20, 2022

Uh oh!

adamsilverstein commented Jul 20, 2022

Uh oh!

Uh oh!

felixarntz left a comment

Uh oh!

getsource Jul 21, 2022

Uh oh!

adamsilverstein Jul 21, 2022 •

edited

Loading

Uh oh!

adamsilverstein commented Jul 21, 2022

Uh oh!

Uh oh!

Multi-mime support, use WebP for content when smaller #2393

Multi-mime support, use WebP for content when smaller #2393

Uh oh!

Conversation

adamsilverstein commented Mar 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamsilverstein commented Jul 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamsilverstein commented Jul 20, 2022

Uh oh!

adamsilverstein commented Jul 20, 2022

Uh oh!

felixarntz commented Jul 20, 2022

Uh oh!

felixarntz Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

adamsilverstein Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

adamsilverstein commented Jul 20, 2022

Uh oh!

adamsilverstein commented Jul 20, 2022

Uh oh!

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

getsource Jul 21, 2022

Choose a reason for hiding this comment

Uh oh!

adamsilverstein Jul 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsilverstein commented Jul 21, 2022

Uh oh!

Uh oh!

adamsilverstein commented Mar 8, 2022 •

edited

Loading

adamsilverstein commented Jul 20, 2022 •

edited

Loading

adamsilverstein Jul 21, 2022 •

edited

Loading