Skip to content

Generate picture source elements when wp_calculate_image_srcset disable. #2201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Draft

Generate picture source elements when wp_calculate_image_srcset disable. #2201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions plugins/webp-uploads/picture-element.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@ function webp_uploads_wrap_image_in_picture( string $image, string $context, int
esc_attr( $image_srcset ),
is_string( $sizes ) ? sprintf( ' sizes="%s"', esc_attr( $sizes ) ) : ''
);
} else {
// Fallback when wp_calculate_image_srcset returns false - generate simple source without srcset.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SOURCE still has a srcset (as it can't have src).

Suggested change
// Fallback when wp_calculate_image_srcset returns false - generate simple source without srcset.
// Fallback when wp_calculate_image_srcset returns false - generate simple non-responsive srcset.

$image_url = webp_uploads_get_mime_type_image( $attachment_id, $src, $image_mime_type );
if ( is_string( $image_url ) ) {
$picture_sources .= sprintf(
'<source type="%s" srcset="%s">',
esc_attr( $image_mime_type ),
esc_attr( $image_url )
);
}
}
}
} else {
Expand Down
5 changes: 5 additions & 0 deletions plugins/webp-uploads/tests/test-picture-element.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,15 @@ public function test_disable_responsive_image_with_picture_element( Closure $set
$this->assertStringEndsWith( '.jpg', $picture_processor->get_attribute( 'src' ), 'Make sure the fallback IMG should return JPEG src.' );

$picture_processor = new WP_HTML_Tag_Processor( $picture_markup );
$source_count = 0;
while ( $picture_processor->next_tag( array( 'tag_name' => 'source' ) ) ) {
++$source_count;
$this->assertSame( self::$mime_type, $picture_processor->get_attribute( 'type' ), 'Make sure the Picture source should not return JPEG as source.' );
$this->assertStringContainsString( '.webp', $picture_processor->get_attribute( 'srcset' ), 'Make sure the Picture source srcset should return WEBP images.' );
}

// Ensure at least one source element was created (addresses GitHub issue with wp_calculate_image_srcset disabled).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice to add a separate test that sets the wp_calculate_image_srcset filter to return false and verifies the source generation.

$this->assertGreaterThan( 0, $source_count, 'At least one source element should be created even when responsive images are disabled.' );
}

/**
Expand Down
Loading