Fix validation error due to native img tag when lightbox and carousel is enabled

includes/amp-helper-functions.php

@@ -1522,12 +1522,12 @@ function amp_get_content_sanitizers( $post = null ) {
 
 	$sanitizers = [
 		// Embed sanitization must come first because it strips out custom scripts associated with embeds.
-		AMP_Embed_Sanitizer::class             => [
+		AMP_Embed_Sanitizer::class                 => [
 			'amp_to_amp_linking_enabled' => $amp_to_amp_linking_enabled,
 		],
-		AMP_O2_Player_Sanitizer::class         => [],
-		AMP_Playbuzz_Sanitizer::class          => [],
-		AMP_Core_Theme_Sanitizer::class        => [
+		AMP_O2_Player_Sanitizer::class             => [],
+		AMP_Playbuzz_Sanitizer::class              => [],
+		AMP_Core_Theme_Sanitizer::class            => [
 			'template'        => get_template(),
 			'stylesheet'      => get_stylesheet(),
 			'theme_features'  => [
@@ -1536,50 +1536,53 @@ function amp_get_content_sanitizers( $post = null ) {
 			'native_img_used' => $native_img_used,
 		],
 
-		AMP_Comments_Sanitizer::class          => [
+		AMP_Comments_Sanitizer::class              => [
 			'comments_live_list' => ! empty( $theme_support_args['comments_live_list'] ),
 		],
 
-		AMP_Bento_Sanitizer::class             => [],
+		AMP_Bento_Sanitizer::class                 => [],
 
 		// The AMP_PWA_Script_Sanitizer run before AMP_Script_Sanitizer, to prevent the script tags
 		// from getting removed in PWA plugin offline/500 templates.
-		AMP_PWA_Script_Sanitizer::class        => [],
+		AMP_PWA_Script_Sanitizer::class            => [],
 
 		// The AMP_Script_Sanitizer runs here because based on whether it allows custom scripts
 		// to be kept, it may impact the behavior of other sanitizers. For example, if custom
 		// scripts are kept then this is a signal that tree shaking in AMP_Style_Sanitizer cannot be
 		// performed.
-		AMP_Script_Sanitizer::class            => [],
+		AMP_Script_Sanitizer::class                => [],
 
-		AMP_Srcset_Sanitizer::class            => [],
-		AMP_Img_Sanitizer::class               => [
+		AMP_Srcset_Sanitizer::class                => [],
+		AMP_Img_Sanitizer::class                   => [
 			'align_wide_support' => current_theme_supports( 'align-wide' ),
 			'native_img_used'    => $native_img_used,
 		],
-		AMP_Form_Sanitizer::class              => [],
-		AMP_Video_Sanitizer::class             => [],
-		AMP_Audio_Sanitizer::class             => [],
-		AMP_Object_Sanitizer::class            => [],
-		AMP_Iframe_Sanitizer::class            => [
+		AMP_Form_Sanitizer::class                  => [],
+		AMP_Video_Sanitizer::class                 => [],
+		AMP_Audio_Sanitizer::class                 => [],
+		AMP_Object_Sanitizer::class                => [],
+		AMP_Iframe_Sanitizer::class                => [
 			'add_placeholder'    => true,
 			'current_origin'     => $current_origin,
 			'align_wide_support' => current_theme_supports( 'align-wide' ),
 		],
-		AMP_Gallery_Block_Sanitizer::class     => [ // Note: Gallery block sanitizer must come after image sanitizers since itś logic is using the already sanitized images.
+		AMP_Gallery_Block_Sanitizer::class         => [ // Note: Gallery block sanitizer must come after image sanitizers since itś logic is using the already sanitized images.
 			'carousel_required' => ! is_array( $theme_support_args ), // For back-compat.
 			'native_img_used'   => $native_img_used,
 		],
-		AMP_Block_Sanitizer::class             => [], // Note: Block sanitizer must come after embed / media sanitizers since its logic is using the already sanitized content.
-		AMP_Style_Sanitizer::class             => [
+		AMP_Native_Img_Attributes_Sanitizer::class => [ // Note: Native img attributes sanitizer must come after image sanitizers since its logic is sanitizing the already sanitized images attributes.
+			'native_img_used' => $native_img_used,
+		],
+		AMP_Block_Sanitizer::class                 => [], // Note: Block sanitizer must come after embed / media sanitizers since its logic is using the already sanitized content.
+		AMP_Style_Sanitizer::class                 => [
 			'skip_tree_shaking'   => is_customize_preview(),
 			'allow_excessive_css' => is_customize_preview(),
 		],
-		AMP_Meta_Sanitizer::class              => [],
-		AMP_Layout_Sanitizer::class            => [],
-		AMP_Accessibility_Sanitizer::class     => [],
+		AMP_Meta_Sanitizer::class                  => [],
+		AMP_Layout_Sanitizer::class                => [],
+		AMP_Accessibility_Sanitizer::class         => [],
 		// Note: This validating sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch.
-		AMP_Tag_And_Attribute_Sanitizer::class => [
+		AMP_Tag_And_Attribute_Sanitizer::class     => [
 			'prefer_bento' => amp_is_bento_enabled(),
 		],
 	];
@@ -1723,6 +1726,7 @@ function amp_get_content_sanitizers( $post = null ) {
 		AMP_Object_Sanitizer::class,
 		AMP_Iframe_Sanitizer::class,
 		AMP_Gallery_Block_Sanitizer::class,
+		AMP_Native_Img_Attributes_Sanitizer::class, // Must come after gallery block sanitizer since it sanitizes img attributes.
 		AMP_Block_Sanitizer::class,
 		AMP_Accessibility_Sanitizer::class,
 		AMP_Layout_Sanitizer::class,

includes/sanitizers/class-amp-img-sanitizer.php

@@ -403,6 +403,20 @@ private function adjust_and_replace_node( Element $node ) {
 		if ( $this->args['native_img_used'] || ( $node->parentNode instanceof Element && Tag::PICTURE === $node->parentNode->tagName ) ) {
 			$attributes = $this->maybe_add_lightbox_attributes( [], $node ); // @todo AMP doesn't support lightbox on <img> yet.
 
+			/*
+			 * Mark lightbox as px-verified attribute until it's supported by AMP spec.
+			 * @see <https://github.com/ampproject/amp-wp/issues/7152#issuecomment-1157933188>
+			 * @todo Remove this once lightbox is added in `lightboxable-elements` for native img tag in AMP spec.
+			 */
+			if ( isset( $attributes['data-amp-lightbox'] ) || $node->hasAttribute( Attribute::LIGHTBOX ) ) {
+				$node_attr = $node->getAttributeNode( Attribute::LIGHTBOX );
+				if ( ! $node_attr instanceof DOMAttr ) {
+					$node_attr = $this->dom->createAttribute( Attribute::LIGHTBOX );
+					$node->setAttributeNode( $node_attr );
+				}
+				ValidationExemption::mark_node_as_px_verified( $node_attr );
+			}
+
 			// Set decoding=async by default. See <https://core.trac.wordpress.org/ticket/53232>.
 			if ( ! $node->hasAttribute( Attribute::DECODING ) ) {
 				$attributes[ Attribute::DECODING ] = 'async';

includes/sanitizers/class-amp-native-img-attributes-sanitizer.php

@@ -0,0 +1,73 @@
+<?php
+/**
+ * Native img attributes sanitizer.
+ *
+ * @since 2.3.1
+ * @package AMP
+ */
+
+use AmpProject\Html\Attribute;
+use AmpProject\Dom\Element;
+
+/**
+ * Class AMP_Native_Img_Attributes_Sanitizer
+ *
+ * @since 2.3.1
+ * @internal
+ */
+class AMP_Native_Img_Attributes_Sanitizer extends AMP_Base_Sanitizer {
+
+	/**
+	 * Array of flags used to control sanitization.
+	 *
+	 * @var array {
+	 *      @type bool $native_img_used   Whether native img is being used.
+	 * }
+	 */
+	protected $args;
+
+	/**
+	 * Default args.
+	 *
+	 * @var array
+	 */
+	protected $DEFAULT_ARGS = [
+		'native_img' => false,
+	];
+
+	/**
+	 * Sanitize the Native img attributes.
+	 *
+	 * @since 2.3
+	 */
+	public function sanitize() {
+		// Bail if native img is not being used.
+		if ( ! isset( $this->args['native_img_used'] ) || ! $this->args['native_img_used'] ) {
+			return;
+		}
+
+		$img_elements = $this->dom->xpath->query(
+			'.//img[ @layout = "fill" or @object-fit = "cover" ]',
+			$this->dom->body
+		);
+
+		if ( ! $img_elements instanceof DOMNodeList || 0 === $img_elements->length ) {
+			return;
+		}
+
+		$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 : '' );
+
+			if ( ' ' !== $style_attr_content ) {
+				$img_element->setAttribute( Attribute::STYLE, $style_attr_content );
+			}
+		}
+	}
+}

includes/sanitizers/class-amp-pwa-script-sanitizer.php

@@ -7,8 +7,6 @@
  */
 
 use AmpProject\AmpWP\ValidationExemption;
-use AmpProject\Dom\Element;
-use AmpProject\Html\Attribute;
 
 /**
  * Class AMP_PWA_Script_Sanitizer

tests/php/test-amp-img-sanitizer.php

@@ -776,6 +776,25 @@ public function test_image_block_link_to_media_file_with_lightbox() {
 		$this->assertEquals( $expected, $content );
 	}
 
+	/**
+	 * Test an native img tag has px-verified lightbox attributes.
+	 *
+	 * @covers ::sanitize()
+	 */
+	public function test_native_img_tag_has_px_verified_lightbox_attr() {
+		$source   = '<figure class="wp-block-image" data-amp-lightbox="true"><img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" /></a></figure>';
+		$expected = '<figure class="wp-block-image" data-amp-lightbox="true"><img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" lightbox="" data-px-verified-attrs="lightbox" data-amp-lightbox="" decoding="async" class="amp-wp-enforced-sizes"></figure>';
+
+		$dom       = AMP_DOM_Utils::get_dom_from_content( $source );
+		$sanitizer = new AMP_Img_Sanitizer( $dom, [ 'native_img_used' => true ] );
+		$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 );
+	}
+
 	/**
 	 * Test an Image block wrapped in an <a>, that has right-alignment, links to the media file, and has 'lightbox' selected.
 	 *

tests/php/test-class-amp-native-img-attributes-sanitizer.php

@@ -0,0 +1,44 @@
+<?php
+/**
+ * Test cases for AMP_Native_Img_Attributes_Sanitizer
+ *
+ * @package AmpProject\AmpWP
+ */
+
+use AmpProject\AmpWP\Tests\TestCase;
+
+/**
+ * Class AMP_Native_Img_Attributes_Sanitizer_Test
+ *
+ * @since 2.3.1
+ * @coversDefaultClass AMP_Native_Img_Attributes_Sanitizer
+ */
+class AMP_Native_Img_Attributes_Sanitizer_Test extends TestCase {
+
+	/**
+	 * 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()
+	 */
+	public function test_native_img_tag_has_not_layout_or_object_fit_attrs() {
+		$source   = '<amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></figure></amp-carousel>';
+		$expected = '<amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><img src="http://example.com/img.png" width="600" height="400" style="position:absolute; left:0; right:0; top:0; bottom: 0; width:100%; height:100%; object-fit:cover;"></figure></amp-carousel>';
+
+		$dom       = AMP_DOM_Utils::get_dom_from_content( $source );
+		$sanitizer = new AMP_Native_Img_Attributes_Sanitizer(
+			$dom,
+			[
+				'native_img_used'   => true,
+				'carousel_required' => true,
+			]
+		);
+		$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 );
+	}
+}

tests/php/test-class-amp-tiktok-embed-handler.php

@@ -177,6 +177,9 @@ public function test_conversion( $source, $expected = null ) {
 		// Remove new data attribute added recently.
 		$actual = str_replace( ' data-embed-from="oembed"', '', $actual );
 
+		// Remove refer param from URL.
+		$actual = str_replace( '?refer=embed', '', $actual );
+
 		$this->assertSimilarMarkup( $expected, $actual );
 	}
 }

tests/php/test-class-amp-youtube-embed-handler.php

@@ -348,6 +348,10 @@ public function test__conversion( $source, $expected, $fallback_for_expected = n
 			version_compare( strtok( get_bloginfo( 'version' ), '-' ), '5.1', '<' )
 			&& null !== $fallback_for_expected
 		) {
+			// Remove the title or alt attribute from $filtered_content if it exists.
+			$filtered_content = preg_replace( '/ title="[^"]*"/', '', $filtered_content );
+			$filtered_content = preg_replace( '/ alt="[^"]*"/', '', $filtered_content );
+
 			$this->assertEqualMarkup( $fallback_for_expected, $filtered_content );
 		} else {
 			$this->assertEqualMarkup( $expected, $filtered_content );