Skip to content
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.

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

Harden logic to add auto keyword to sizes attribute to prevent duplicate keyword #1445

Merged
merged 9 commits into from
Aug 12, 2024
27 changes: 23 additions & 4 deletions plugins/auto-sizes/hooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function auto_sizes_update_image_attributes( $attr ): array {
}

// Don't add 'auto' to the sizes attribute if it already exists.
if ( str_contains( $attr['sizes'], 'auto,' ) ) {
if ( auto_sizes_attribute_includes_valid_auto( $attr['sizes'] ) ) {
return $attr;
}

Expand Down Expand Up @@ -63,12 +63,12 @@ function auto_sizes_update_content_img_tag( $html ): string {
}

// Bail early if the image is not responsive.
if ( false === strpos( $html, 'sizes="' ) ) {
if ( preg_match( '/sizes="([^"]+)"/', $html, $match ) === 0 || ! isset( $match[1] ) ) {
return $html;
}

// Don't double process content images.
if ( false !== strpos( $html, 'sizes="auto,' ) ) {
// Don't add 'auto' to the sizes attribute if it already exists.
if ( auto_sizes_attribute_includes_valid_auto( $match[1] ) ) {
return $html;
}

Expand All @@ -78,6 +78,25 @@ function auto_sizes_update_content_img_tag( $html ): string {
}
add_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' );

/**
* Checks whether the given 'sizes' attribute includes the 'auto' keyword as the first item in the list.
*
* Per the HTML spec, if present it must be the first entry.
*
* @since n.e.x.t
*
* @param string $sizes_attr The 'sizes' attribute value.
* @return bool True if the 'auto' keyword is present, false otherwise.
*/
function auto_sizes_attribute_includes_valid_auto( string $sizes_attr ): bool {
$parts = preg_split( '/\s*,\s*/', strtolower( trim( $sizes_attr ) ) );
if ( ! is_array( $parts ) || ! isset( $parts[0] ) ) {
return false;
}

return 'auto' === $parts[0];
}

/**
* Displays the HTML generator tag for the plugin.
*
Expand Down
113 changes: 113 additions & 0 deletions plugins/auto-sizes/tests/test-auto-sizes.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,119 @@ public function test_content_image_without_lazy_loading_does_not_have_auto_sizes
);
}

/**
* Test generated markup for an image with 'auto' keyword already present in sizes does not receive it again.
*
* @covers ::auto_sizes_update_image_attributes
* @covers ::auto_sizes_attribute_includes_valid_auto
* @dataProvider data_image_with_existing_auto_sizes
*/
public function test_image_with_existing_auto_sizes_is_not_processed_again( string $initial_sizes, bool $expected_processed ): void {
$image_tag = wp_get_attachment_image(
self::$image_id,
'large',
false,
array(
// Force pre-existing 'sizes' attribute and lazy-loading.
'sizes' => $initial_sizes,
'loading' => 'lazy',
)
);
if ( $expected_processed ) {
$this->assertStringContainsString( 'sizes="auto, ' . $initial_sizes . '"', $image_tag );
} else {
$this->assertStringContainsString( 'sizes="' . $initial_sizes . '"', $image_tag );
}
}

/**
* Test content filtered markup with 'auto' keyword already present in sizes does not receive it again.
*
* @covers ::auto_sizes_update_content_img_tag
* @covers ::auto_sizes_attribute_includes_valid_auto
* @dataProvider data_image_with_existing_auto_sizes
*/
public function test_content_image_with_existing_auto_sizes_is_not_processed_again( string $initial_sizes, bool $expected_processed ): void {
// Force lazy loading attribute.
add_filter( 'wp_img_tag_add_loading_attr', '__return_true' );

add_filter(
'get_image_tag',
static function ( $html ) use ( $initial_sizes ) {
return str_replace(
'" />',
'" sizes="' . $initial_sizes . '" />',
$html
);
}
);

$image_content = wp_filter_content_tags( $this->get_image_tag( self::$image_id ) );
if ( $expected_processed ) {
$this->assertStringContainsString( 'sizes="auto, ' . $initial_sizes . '"', $image_content );
} else {
$this->assertStringContainsString( 'sizes="' . $initial_sizes . '"', $image_content );
}
}

/**
* Returns data for the above test methods to assert correct behavior with a pre-existing sizes attribute.
*
* @return array<string, mixed[]> Arguments for the test scenarios.
*/
public function data_image_with_existing_auto_sizes(): array {
return array(
'not present' => array(
'(max-width: 1024px) 100vw, 1024px',
true,
),
'in beginning, without space' => array(
'auto,(max-width: 1024px) 100vw, 1024px',
false,
),
'in beginning, with space' => array(
'auto, (max-width: 1024px) 100vw, 1024px',
false,
),
'sole keyword' => array(
'auto',
false,
),
'with space before' => array(
' auto, (max-width: 1024px) 100vw, 1024px',
false,
),
'with uppercase' => array(
'AUTO, (max-width: 1024px) 100vw, 1024px',
false,
),

/*
* The following scenarios technically include the 'auto' keyword,
* but it is in the wrong place, as per the HTML spec it must be
* the first entry in the list.
* Therefore in these invalid cases the 'auto' keyword should still
* be added to the beginning of the list.
*/
'within, without space' => array(
'(max-width: 1024px) 100vw, auto,1024px',
true,
),
'within, with space' => array(
'(max-width: 1024px) 100vw, auto, 1024px',
true,
),
'at the end, without space' => array(
'(max-width: 1024px) 100vw,auto',
true,
),
'at the end, with space' => array(
'(max-width: 1024px) 100vw, auto',
true,
),
);
}

/**
* Test printing the meta generator tag.
*
Expand Down
Loading