Skip to content

Commit 5600db9

Browse files
westonruterwestonruter
authored
Merge pull request #1885 from b1ink0/fix/wrong-image-size-picture-element
Fix incorrect image size selection in `PICTURE` element
2 parents 8efc280 + 2c92462 commit 5600db9

File tree

2 files changed

+175
-20
lines changed

2 files changed

+175
-20
lines changed

‎plugins/webp-uploads/picture-element.php‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,33 @@ function webp_uploads_wrap_image_in_picture( string $image, string $context, int
4141
array_unshift( $image_sizes, $image_meta );
4242
}
4343

44+
// Extract sizes using regex to parse image tag, then use to retrieve tag.
45+
$width = 0;
46+
$height = 0;
47+
$processor = new WP_HTML_Tag_Processor( $image );
48+
if ( $processor->next_tag( array( 'tag_name' => 'IMG' ) ) ) {
49+
$width = (int) $processor->get_attribute( 'width' );
50+
$height = (int) $processor->get_attribute( 'height' );
51+
}
52+
$size_to_use = ( $width > 0 && $height > 0 ) ? array( $width, $height ) : 'full';
53+
54+
$image_src = wp_get_attachment_image_src( $attachment_id, $size_to_use );
55+
if ( false === $image_src ) {
56+
return $image;
57+
}
58+
list( $src, $width, $height ) = $image_src;
59+
$size_array = array( absint( $width ), absint( $height ) );
60+
4461
// Collect all the sub size image mime types.
4562
$mime_type_data = array();
4663
foreach ( $image_sizes as $size ) {
4764
if ( isset( $size['sources'] ) && isset( $size['width'] ) && isset( $size['height'] ) ) {
4865
foreach ( $size['sources'] as $mime_type => $data ) {
49-
$mime_type_data[ $mime_type ] = $mime_type_data[ $mime_type ] ?? array();
50-
$mime_type_data[ $mime_type ]['w'][ $size['width'] ] = $data;
51-
$mime_type_data[ $mime_type ]['h'][ $size['height'] ] = $data;
66+
if ( wp_image_matches_ratio( $size_array[0], $size_array[1], $size['width'], $size['height'] ) ) {
67+
$mime_type_data[ $mime_type ] = $mime_type_data[ $mime_type ] ?? array();
68+
$mime_type_data[ $mime_type ]['w'][ $size['width'] ] = $data;
69+
$mime_type_data[ $mime_type ]['h'][ $size['height'] ] = $data;
70+
}
5271
}
5372
}
5473
}
@@ -97,23 +116,6 @@ function webp_uploads_wrap_image_in_picture( string $image, string $context, int
97116
// Add each mime type to the picture's sources.
98117
$picture_sources = '';
99118

100-
// Extract sizes using regex to parse image tag, then use to retrieve tag.
101-
$width = 0;
102-
$height = 0;
103-
$processor = new WP_HTML_Tag_Processor( $image );
104-
if ( $processor->next_tag( array( 'tag_name' => 'IMG' ) ) ) {
105-
$width = (int) $processor->get_attribute( 'width' );
106-
$height = (int) $processor->get_attribute( 'height' );
107-
}
108-
$size_to_use = ( $width > 0 && $height > 0 ) ? array( $width, $height ) : 'full';
109-
110-
$image_src = wp_get_attachment_image_src( $attachment_id, $size_to_use );
111-
if ( false === $image_src ) {
112-
return $image;
113-
}
114-
list( $src, $width, $height ) = $image_src;
115-
$size_array = array( absint( $width ), absint( $height ) );
116-
117119
// Gets the srcset and sizes from the IMG tag.
118120
$sizes = $processor->get_attribute( 'sizes' );
119121
$srcset = $processor->get_attribute( 'srcset' );

‎plugins/webp-uploads/tests/test-picture-element.php‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@ class Test_WebP_Uploads_Picture_Element extends TestCase {
2525
*/
2626
public static $image_id;
2727

28+
/**
29+
* Temporary attachment IDs created during tests.
30+
*
31+
* @var array<int>
32+
*/
33+
private $temp_attachment_ids = array();
34+
35+
/**
36+
* Temporary custom image sizes created during tests.
37+
*
38+
* @var array<string>
39+
*/
40+
private $temp_custom_image_sizes = array();
41+
2842
public function set_up(): void {
2943
parent::set_up();
3044

@@ -36,6 +50,24 @@ public function set_up(): void {
3650
$this->mock_frontend_body_hooks();
3751
}
3852

53+
public function tear_down(): void {
54+
// Clean up any attachments created during tests.
55+
foreach ( $this->temp_attachment_ids as $id ) {
56+
wp_delete_attachment( $id, true );
57+
}
58+
$this->temp_attachment_ids = array();
59+
60+
// Clean up custom image sizes.
61+
foreach ( $this->temp_custom_image_sizes as $size ) {
62+
if ( has_image_size( $size ) ) {
63+
remove_image_size( $size );
64+
}
65+
}
66+
$this->temp_custom_image_sizes = array();
67+
68+
parent::tear_down();
69+
}
70+
3971
/**
4072
* Setup shared fixtures.
4173
*/
@@ -264,6 +296,102 @@ function ( string $content ): string {
264296
);
265297
}
266298

299+
/**
300+
* Test that the picture element source tag srcset has the same image sizes as the img tag srcset.
301+
*
302+
* @dataProvider data_provider_test_picture_element_source_tag_srcset_has_same_image_sizes_as_img_tag_srcset
303+
* @covers ::webp_uploads_wrap_image_in_picture
304+
*
305+
* @param Closure|null $set_up Set up the test.
306+
*/
307+
public function test_picture_element_source_tag_srcset_has_same_image_sizes_as_img_tag_srcset( ?Closure $set_up ): void {
308+
if ( $set_up instanceof Closure ) {
309+
$set_up();
310+
}
311+
312+
update_option( 'perflab_generate_webp_and_jpeg', '1' );
313+
update_option( 'perflab_generate_all_fallback_sizes', '1' );
314+
315+
// Create image with different sizes.
316+
$attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/data/images/leaves.jpg' );
317+
$this->assertNotWPError( $attachment_id );
318+
$this->temp_attachment_ids[] = $attachment_id;
319+
320+
$image = wp_get_attachment_image(
321+
$attachment_id,
322+
'large',
323+
false,
324+
array(
325+
'class' => 'wp-image-' . $attachment_id,
326+
'alt' => 'Green Leaves',
327+
)
328+
);
329+
330+
// Apply picture element support.
331+
$this->opt_in_to_picture_element();
332+
333+
$picture_markup = apply_filters( 'the_content', $image );
334+
$picture_processor = new WP_HTML_Tag_Processor( $picture_markup );
335+
$picture_processor->next_tag( array( 'tag_name' => 'SOURCE' ) );
336+
$source_srcset = $picture_processor->get_attribute( 'srcset' );
337+
$picture_processor->next_tag( array( 'tag_name' => 'IMG' ) );
338+
$img_srcset = $picture_processor->get_attribute( 'srcset' );
339+
340+
// Extract file names from srcset.
341+
$extract_file_names_from_srcset = static function ( string $srcset ): array {
342+
$srcset_parts = explode( ', ', $srcset );
343+
$files = array();
344+
foreach ( $srcset_parts as $srcset_part ) {
345+
$parts = explode( ' ', trim( $srcset_part ) );
346+
$url = $parts[0];
347+
$files[] = pathinfo( basename( $url ), PATHINFO_FILENAME );
348+
}
349+
return $files;
350+
};
351+
352+
$source_files = $extract_file_names_from_srcset( $source_srcset );
353+
$img_files = $extract_file_names_from_srcset( $img_srcset );
354+
355+
$this->assertCount( count( $source_files ), $img_files );
356+
357+
foreach ( $img_files as $img_index => $img_file ) {
358+
// Check the file name starts with the same prefix.
359+
// e.g. $img_file = 'leaves-350x200' and $source_files[ $img_index ] = 'leaves-350x350-jpg'.
360+
$this->assertStringStartsWith( $img_file, $source_files[ $img_index ] );
361+
}
362+
}
363+
364+
/**
365+
* Data provider for test_picture_element_source_tag_srcset_has_same_image_sizes_as_img_tag_srcset.
366+
*
367+
* @return array<string, array{ set_up: Closure|null }>
368+
*/
369+
public function data_provider_test_picture_element_source_tag_srcset_has_same_image_sizes_as_img_tag_srcset(): array {
370+
return array(
371+
'default_sizes' => array(
372+
'set_up' => null,
373+
),
374+
'when_two_different_image_sizes_have_same_width' => array(
375+
'set_up' => function (): void {
376+
add_image_size( 'square', 768, 768, true );
377+
$this->temp_custom_image_sizes[] = 'square';
378+
add_filter(
379+
'pre_option_medium_large_size_w',
380+
static function () {
381+
return '768';
382+
}
383+
);
384+
add_filter(
385+
'pre_option_medium_large_size_h',
386+
static function () {
387+
return '512';
388+
}
389+
);
390+
},
391+
),
392+
);
393+
}
394+
267395
/**
268396
* @dataProvider data_provider_test_disable_responsive_image_with_picture_element
269397
*
@@ -373,4 +501,29 @@ public function test_picture_source_should_have_full_size_image_in_its_srcset():
373501
$this->assertStringContainsString( $image_meta['sources'][ self::$mime_type ]['file'], $picture_processor->get_attribute( 'srcset' ), 'Make sure the IMG srcset should have full size image.' );
374502
}
375503
}
504+
505+
/**
506+
* Test handling of case when wp_get_attachment_image_src returns false.
507+
*
508+
* @covers ::webp_uploads_wrap_image_in_picture
509+
*/
510+
public function test_wrap_image_in_picture_with_false_image_src(): void {
511+
$this->opt_in_to_picture_element();
512+
513+
$image = wp_get_attachment_image(
514+
self::$image_id,
515+
'large',
516+
false,
517+
array(
518+
'class' => 'wp-image-' . self::$image_id,
519+
'alt' => 'Green Leaves',
520+
)
521+
);
522+
523+
add_filter( 'wp_get_attachment_image_src', '__return_false' );
524+
$filtered_image = apply_filters( 'wp_content_img_tag', $image, 'the_content', self::$image_id );
525+
remove_filter( 'wp_get_attachment_image_src', '__return_false' );
526+
527+
$this->assertSame( $image, $filtered_image );
528+
}
376529
}

0 commit comments

Comments
 (0)