From ee80e10a34d175f4559825c7881efda439f4b983 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 7 Jul 2022 12:19:35 -0700 Subject: [PATCH] Preserve original styles; support object-position; support non-cover object-fit --- ...ss-amp-native-img-attributes-sanitizer.php | 48 +++++++++---- ...ss-amp-native-img-attributes-sanitizer.php | 70 ++++++++++++++++--- 2 files changed, 92 insertions(+), 26 deletions(-) diff --git a/includes/sanitizers/class-amp-native-img-attributes-sanitizer.php b/includes/sanitizers/class-amp-native-img-attributes-sanitizer.php index 2f35479a3b2..4a822dd1d3b 100644 --- a/includes/sanitizers/class-amp-native-img-attributes-sanitizer.php +++ b/includes/sanitizers/class-amp-native-img-attributes-sanitizer.php @@ -8,6 +8,7 @@ use AmpProject\Html\Attribute; use AmpProject\Dom\Element; +use AmpProject\Layout; /** * Class AMP_Native_Img_Attributes_Sanitizer @@ -46,27 +47,44 @@ public function sanitize() { return; } + // Images with layout=fill. $img_elements = $this->dom->xpath->query( - './/img[ @layout = "fill" or @object-fit = "cover" ]', + './/img[ @layout = "fill" ]', $this->dom->body ); - - if ( ! $img_elements instanceof DOMNodeList || 0 === $img_elements->length ) { - return; + if ( $img_elements instanceof DOMNodeList ) { + foreach ( $img_elements as $img_element ) { + /** @var Element $img_element */ + $img_element->removeAttribute( Attribute::LAYOUT ); + $img_element->addInlineStyle( 'position:absolute;left:0;right:0;top:0;bottom:0;width:100%;height:100%' ); + } } - $style_layout_fill = 'position:absolute; left:0; right:0; top:0; bottom: 0; width:100%; height:100%;'; - $style_object_fit = 'object-fit:cover;'; - - foreach ( $img_elements as $img_element ) { - /** @var Element $img_element */ - - $remove_layout_attr = $img_element->removeAttribute( Attribute::LAYOUT ); - $remove_object_fit_attr = $img_element->removeAttribute( Attribute::OBJECT_FIT ); - $style_attr_content = sprintf( '%s %s', $remove_layout_attr ? $style_layout_fill : '', $remove_object_fit_attr ? $style_object_fit : '' ); + // Images with object-fit attributes. + $img_elements = $this->dom->xpath->query( + './/img[ @object-fit ]', + $this->dom->body + ); + if ( $img_elements instanceof DOMNodeList ) { + foreach ( $img_elements as $img_element ) { + /** @var Element $img_element */ + $value = $img_element->getAttribute( Attribute::OBJECT_FIT ); + $img_element->removeAttribute( Attribute::OBJECT_FIT ); + $img_element->addInlineStyle( sprintf( 'object-fit:%s', $value ) ); + } + } - if ( ' ' !== $style_attr_content ) { - $img_element->setAttribute( Attribute::STYLE, $style_attr_content ); + // Images with object-position attributes. + $img_elements = $this->dom->xpath->query( + './/img[ @object-position ]', + $this->dom->body + ); + if ( $img_elements instanceof DOMNodeList ) { + foreach ( $img_elements as $img_element ) { + /** @var Element $img_element */ + $value = $img_element->getAttribute( Attribute::OBJECT_POSITION ); + $img_element->removeAttribute( Attribute::OBJECT_POSITION ); + $img_element->addInlineStyle( sprintf( 'object-position:%s', $value ) ); } } } diff --git a/tests/php/test-class-amp-native-img-attributes-sanitizer.php b/tests/php/test-class-amp-native-img-attributes-sanitizer.php index ed54a07516f..66849151077 100644 --- a/tests/php/test-class-amp-native-img-attributes-sanitizer.php +++ b/tests/php/test-class-amp-native-img-attributes-sanitizer.php @@ -6,6 +6,7 @@ */ use AmpProject\AmpWP\Tests\TestCase; +use AmpProject\AmpWP\Tests\Helpers\MarkupComparison; /** * Class AMP_Native_Img_Attributes_Sanitizer_Test @@ -15,30 +16,77 @@ */ class AMP_Native_Img_Attributes_Sanitizer_Test extends TestCase { + use MarkupComparison; + + public function get_data_to_test_sanitize() { + $amp_carousel_source = '
'; + $amp_carousel_sanitized = '
'; + + return [ + 'disabled' => [ + false, + $amp_carousel_source, + null, // Same as above. + ], + 'carousel_with_img' => [ + true, + $amp_carousel_source, + $amp_carousel_sanitized, + ], + 'amp_img' => [ + true, + '', + null, // Same as above. + ], + 'img_with_layout_fill' => [ + true, + '', + '', + ], + 'img_with_layout_nodisplay' => [ + true, + '', + null, // Same as above. + ], + 'img_with_object_fit_cover' => [ + true, + '', + '', + ], + 'img_with_object_fit_contain' => [ + true, + '', + '', + ], + 'img_with_object_position' => [ + true, + '', + '', + ], + ]; + } + /** * Test an native img tag has not layout or object-fit attributes. * * `layout` and `object-fit` will be replaced with a style attribute. * - * @covers \AMP_Native_Img_Attributes_Sanitizer::sanitize() + * @dataProvider get_data_to_test_sanitize + * @covers ::sanitize() */ - public function test_native_img_tag_has_not_layout_or_object_fit_attrs() { - $source = '
'; - $expected = '
'; + public function test_sanitize( $native_img_used, $source, $expected ) { + if ( null === $expected ) { + $expected = $source; + } $dom = AMP_DOM_Utils::get_dom_from_content( $source ); $sanitizer = new AMP_Native_Img_Attributes_Sanitizer( $dom, - [ - 'native_img_used' => true, - 'carousel_required' => true, - ] + compact( 'native_img_used' ) ); $sanitizer->sanitize(); - $sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom ); - $sanitizer->sanitize(); $content = AMP_DOM_Utils::get_content_from_dom( $dom ); - $this->assertEquals( $expected, $content ); + $this->assertEqualMarkup( $expected, $content ); } }