From 7a2ca29b98af41bf0d38f483c3da1ae0996fc025 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 26 May 2020 12:37:30 -0700
Subject: [PATCH 01/47] Rework how and whether galleries are rendered as
 carousels

---
 assets/src/block-editor/helpers/index.js      | 216 ++----------------
 assets/src/block-editor/index.js              |   3 +-
 .../embeds/class-amp-core-block-handler.php   |  20 ++
 .../class-amp-gallery-embed-handler.php       |  21 +-
 .../class-amp-gallery-block-sanitizer.php     |  16 +-
 5 files changed, 59 insertions(+), 217 deletions(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index a0f7be968c4..afd1996d133 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -8,7 +8,7 @@ import { ReactElement } from 'react';
  * WordPress dependencies
  */
 import { __, _x } from '@wordpress/i18n';
-import { cloneElement, RawHTML, render } from '@wordpress/element';
+import { cloneElement, render } from '@wordpress/element';
 import { TextControl, SelectControl, ToggleControl, Notice, PanelBody, FontSizePicker } from '@wordpress/components';
 import { InspectorControls } from '@wordpress/block-editor';
 import { select } from '@wordpress/data';
@@ -94,15 +94,17 @@ const ampLayoutOptions = [
  */
 export const addAMPAttributes = ( settings, name ) => {
 	// AMP Carousel settings.
-	if ( 'core/shortcode' === name || 'core/gallery' === name ) {
+	if ( 'core/gallery' === name ) {
 		if ( ! settings.attributes ) {
 			settings.attributes = {};
 		}
 		settings.attributes.ampCarousel = {
 			type: 'boolean',
+			default: ! select( 'amp/block-editor' ).hasThemeSupport(), // @todo We could just default this to false now even in Reader mode since block styles are loaded.
 		};
 		settings.attributes.ampLightbox = {
 			type: 'boolean',
+			default: false,
 		};
 	}
 
@@ -113,6 +115,7 @@ export const addAMPAttributes = ( settings, name ) => {
 		}
 		settings.attributes.ampLightbox = {
 			type: 'boolean',
+			default: false,
 		};
 	}
 
@@ -124,6 +127,7 @@ export const addAMPAttributes = ( settings, name ) => {
 			settings.attributes = {};
 		}
 		settings.attributes.ampFitText = {
+			type: 'boolean',
 			default: false,
 		};
 		settings.attributes.minFont = {
@@ -173,58 +177,12 @@ export const addAMPAttributes = ( settings, name ) => {
  * @return {Object} Output element.
  */
 export const filterBlocksSave = ( element, blockType, attributes ) => { // eslint-disable-line complexity
-	let text = attributes.text || '',
-		content = '';
-
 	const fitTextProps = {
 		layout: 'fixed-height',
 	};
 
-	if ( 'core/shortcode' === blockType.name && isGalleryShortcode( attributes ) ) {
-		if ( ! attributes.ampLightbox ) {
-			if ( hasGalleryShortcodeLightboxAttribute( attributes.text || '' ) ) {
-				text = removeAmpLightboxFromShortcodeAtts( attributes.text );
-			}
-		}
-		if ( attributes.ampCarousel ) {
-			// If the text contains amp-carousel or amp-lightbox, lets remove it.
-			if ( hasGalleryShortcodeCarouselAttribute( text ) ) {
-				text = removeAmpCarouselFromShortcodeAtts( text );
-			}
-
-			// If lightbox is not set, we can return here.
-			if ( ! attributes.ampLightbox ) {
-				if ( attributes.text !== text ) {
-					return (
-						<RawHTML>
-							{ text }
-						</RawHTML>
-					);
-				}
-
-				// Else lets return original.
-				return element;
-			}
-		} else if ( ! hasGalleryShortcodeCarouselAttribute( attributes.text || '' ) ) {
-			// Add amp-carousel=false attribute to the shortcode.
-			text = attributes.text.replace( '[gallery', '[gallery amp-carousel=false' );
-		} else {
-			text = attributes.text;
-		}
-
-		if ( attributes.ampLightbox && ! hasGalleryShortcodeLightboxAttribute( text ) ) {
-			text = text.replace( '[gallery', '[gallery amp-lightbox=true' );
-		}
-
-		if ( attributes.text !== text ) {
-			return (
-				<RawHTML>
-					{ text }
-				</RawHTML>
-			);
-		}
-	} else if ( 'core/paragraph' === blockType.name && ! attributes.ampFitText ) {
-		content = getAmpFitTextContent( attributes.content );
+	if ( 'core/paragraph' === blockType.name && ! attributes.ampFitText ) {
+		const content = getAmpFitTextContent( attributes.content );
 		if ( content !== attributes.content ) {
 			return cloneElement(
 				element,
@@ -302,48 +260,6 @@ export const getLayoutOptions = ( block ) => {
 	return layoutOptions;
 };
 
-/**
- * Add extra data-amp-layout attribute to save to DB.
- *
- * @param {Object} props          Properties.
- * @param {Object} blockType      Block type.
- * @param {Object} blockType.name Block type name.
- * @param {Object} attributes     Attributes.
- *
- * @return {Object} Props.
- */
-export const addAMPExtraProps = ( props, blockType, attributes ) => {
-	const ampAttributes = {};
-
-	// Shortcode props are handled differently.
-	if ( 'core/shortcode' === blockType.name ) {
-		return props;
-	}
-
-	// AMP blocks handle layout and other props on their own.
-	if ( 'amp/' === blockType.name.substr( 0, 4 ) ) {
-		return props;
-	}
-
-	if ( attributes.ampLayout ) {
-		ampAttributes[ 'data-amp-layout' ] = attributes.ampLayout;
-	}
-	if ( attributes.ampNoLoading ) {
-		ampAttributes[ 'data-amp-noloading' ] = attributes.ampNoLoading;
-	}
-	if ( attributes.ampLightbox ) {
-		ampAttributes[ 'data-amp-lightbox' ] = attributes.ampLightbox;
-	}
-	if ( attributes.ampCarousel ) {
-		ampAttributes[ 'data-amp-carousel' ] = attributes.ampCarousel;
-	}
-
-	return {
-		...ampAttributes,
-		...props,
-	};
-};
-
 /**
  * Filters blocks edit function of all blocks.
  *
@@ -353,26 +269,11 @@ export const addAMPExtraProps = ( props, blockType, attributes ) => {
  */
 export const filterBlocksEdit = ( BlockEdit ) => {
 	const EnhancedBlockEdit = function( props ) {
-		const { attributes: { text, ampLayout }, setAttributes, name } = props;
+		const { attributes: { ampLayout }, name } = props;
 
 		let inspectorControls;
 
-		if ( 'core/shortcode' === name ) {
-			// Lets remove amp-carousel from edit view.
-			if ( hasGalleryShortcodeCarouselAttribute( text || '' ) ) {
-				setAttributes( { text: removeAmpCarouselFromShortcodeAtts( text ) } );
-			}
-			// Lets remove amp-lightbox from edit view.
-			if ( hasGalleryShortcodeLightboxAttribute( text || '' ) ) {
-				setAttributes( { text: removeAmpLightboxFromShortcodeAtts( text ) } );
-			}
-
-			inspectorControls = setUpShortcodeInspectorControls( props );
-			if ( '' === inspectorControls ) {
-				// Return original.
-				return <BlockEdit { ...props } />;
-			}
-		} else if ( 'core/gallery' === name ) {
+		if ( 'core/gallery' === name ) {
 			inspectorControls = setUpGalleryInspectorControls( props );
 		} else if ( 'core/image' === name ) {
 			inspectorControls = setUpImageInspectorControls( props );
@@ -666,7 +567,7 @@ const setUpTextBlocksInspectorControls = ( props ) => {
 setUpTextBlocksInspectorControls.propTypes = {
 	isSelected: PropTypes.bool,
 	attributes: PropTypes.shape( {
-		ampFitText: PropTypes.string,
+		ampFitText: PropTypes.bool,
 		minFont: PropTypes.number,
 		maxFont: PropTypes.number,
 		height: PropTypes.number,
@@ -674,38 +575,6 @@ setUpTextBlocksInspectorControls.propTypes = {
 	setAttributes: PropTypes.func.isRequired,
 };
 
-/**
- * Set up inspector controls for shortcode block.
- * Adds ampCarousel attribute in case of gallery shortcode.
- *
- * @param {Object} props Props.
- *
- * @return {ReactElement} Inspector controls.
- */
-const setUpShortcodeInspectorControls = ( props ) => {
-	const { isSelected } = props;
-
-	if ( ! isGalleryShortcode( props.attributes ) || ! isSelected ) {
-		return null;
-	}
-
-	const hasThemeSupport = select( 'amp/block-editor' ).hasThemeSupport();
-
-	return (
-		<InspectorControls>
-			<PanelBody title={ __( 'AMP Settings', 'amp' ) }>
-				{ hasThemeSupport && <AmpCarouselToggle { ...props } /> }
-				<AmpLightboxToggle { ...props } />
-			</PanelBody>
-		</InspectorControls>
-	);
-};
-
-setUpShortcodeInspectorControls.propTypes = {
-	isSelected: PropTypes.bool,
-	attributes: PropTypes.object,
-};
-
 /**
  * Get AMP Lightbox toggle control.
  *
@@ -739,7 +608,7 @@ const AmpLightboxToggle = ( props ) => {
 
 AmpLightboxToggle.propTypes = {
 	attributes: PropTypes.shape( {
-		ampLightbox: PropTypes.string,
+		ampLightbox: PropTypes.bool,
 		ampLayout: PropTypes.string,
 		linkTo: PropTypes.string,
 	} ),
@@ -770,7 +639,7 @@ const AmpCarouselToggle = ( props ) => {
 
 AmpCarouselToggle.propTypes = {
 	attributes: PropTypes.shape( {
-		ampCarousel: PropTypes.string,
+		ampCarousel: PropTypes.bool,
 	} ),
 	setAttributes: PropTypes.func.isRequired,
 };
@@ -821,12 +690,10 @@ const setUpGalleryInspectorControls = ( props ) => {
 		return null;
 	}
 
-	const hasThemeSupport = select( 'amp/block-editor' ).hasThemeSupport();
-
 	return (
 		<InspectorControls>
 			<PanelBody title={ __( 'AMP Settings', 'amp' ) }>
-				{ hasThemeSupport && <AmpCarouselToggle { ...props } /> }
+				<AmpCarouselToggle { ...props } />
 				<AmpLightboxToggle { ...props } />
 			</PanelBody>
 		</InspectorControls>
@@ -837,61 +704,6 @@ setUpGalleryInspectorControls.propTypes = {
 	isSelected: PropTypes.bool,
 };
 
-/**
- * Removes amp-carousel=false from shortcode attributes.
- *
- * @param {string} shortcode Shortcode text.
- *
- * @return {string} Modified shortcode.
- */
-export const removeAmpCarouselFromShortcodeAtts = ( shortcode ) => {
-	return shortcode.replace( ' amp-carousel=false', '' );
-};
-
-/**
- * Removes amp-lightbox=true from shortcode attributes.
- *
- * @param {string} shortcode Shortcode text.
- *
- * @return {string} Modified shortcode.
- */
-export const removeAmpLightboxFromShortcodeAtts = ( shortcode ) => {
-	return shortcode.replace( ' amp-lightbox=true', '' );
-};
-
-/**
- * Determines whether a shortcode includes the amp-carousel attribute.
- *
- * @param {string} text Shortcode.
- *
- * @return {boolean} Whether the shortcode includes the attribute.
- */
-export const hasGalleryShortcodeCarouselAttribute = ( text ) => {
-	return -1 !== text.indexOf( 'amp-carousel=false' );
-};
-
-/**
- * Determines whether a shortcode includes the amp-lightbox attribute.
- *
- * @param {string} text Shortcode.
- *
- * @return {boolean} Whether the shortcode includes the attribute.
- */
-export const hasGalleryShortcodeLightboxAttribute = ( text ) => {
-	return -1 !== text.indexOf( 'amp-lightbox=true' );
-};
-
-/**
- * Determines whether the current shortcode is a gallery shortcode.
- *
- * @param {Object} attributes Shortcode attributes.
- *
- * @return {boolean} Whether it is a gallery shortcode.
- */
-export const isGalleryShortcode = ( attributes ) => {
-	return attributes.text && -1 !== attributes.text.indexOf( 'gallery' );
-};
-
 /**
  * Determines whether AMP is enabled for the current post or not.
  *
diff --git a/assets/src/block-editor/index.js b/assets/src/block-editor/index.js
index 88d4a197e7b..f625e6f2fdd 100644
--- a/assets/src/block-editor/index.js
+++ b/assets/src/block-editor/index.js
@@ -13,7 +13,7 @@ import domReady from '@wordpress/dom-ready';
 import { withFeaturedImageNotice } from '../common/components';
 import { getMinimumFeaturedImageDimensions } from '../common/helpers';
 import { withMediaLibraryNotice, AMPPreview } from './components';
-import { addAMPAttributes, addAMPExtraProps, filterBlocksEdit, filterBlocksSave, renderPreviewButton } from './helpers';
+import { addAMPAttributes, filterBlocksEdit, filterBlocksSave, renderPreviewButton } from './helpers';
 import './store';
 
 const {
@@ -31,7 +31,6 @@ plugins.keys().forEach( ( modulePath ) => {
 addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', addAMPAttributes );
 addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', filterBlocksSave );
 addFilter( 'editor.BlockEdit', 'ampEditorBlocks/filterEdit', filterBlocksEdit, 20 );
-addFilter( 'blocks.getSaveContent.extraProps', 'ampEditorBlocks/addExtraAttributes', addAMPExtraProps );
 addFilter( 'editor.PostFeaturedImage', 'ampEditorBlocks/withFeaturedImageNotice', withFeaturedImageNotice );
 addFilter( 'editor.MediaUpload', 'ampEditorBlocks/withMediaLibraryNotice', ( InitialMediaUpload ) => withMediaLibraryNotice( InitialMediaUpload, getMinimumFeaturedImageDimensions() ) );
 
diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php
index 94de522b13a..a656d03545a 100644
--- a/includes/embeds/class-amp-core-block-handler.php
+++ b/includes/embeds/class-amp-core-block-handler.php
@@ -76,6 +76,26 @@ public function unregister_embed() {
 	 * @return string Filtered block content.
 	 */
 	public function filter_rendered_block( $block_content, $block ) {
+
+		if ( isset( $block['attrs'] ) ) {
+			$injected_attributes    = '';
+			$prop_attribute_mapping = [
+				'ampLayout'    => 'data-amp-layout',
+				'ampNoLoading' => 'data-amp-noloading',
+				'ampLightbox'  => 'data-amp-lightbox',
+				'ampCarousel'  => 'data-amp-carousel',
+			];
+			foreach ( $prop_attribute_mapping as $prop => $attr ) {
+				if ( isset( $block['attrs'][ $prop ] ) ) {
+					$property_value       = rest_sanitize_boolean( $block['attrs'][ $prop ] );
+					$injected_attributes .= sprintf( ' %s="%s"', $attr, esc_attr( wp_json_encode( $property_value ) ) );
+				}
+			}
+			if ( $injected_attributes ) {
+				$block_content = preg_replace( '/(<\w+)/', '$1' . $injected_attributes, $block_content, 1 );
+			}
+		}
+
 		if ( ! isset( $block['blockName'] ) ) {
 			return $block_content;
 		}
diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index 875ad3e793a..d49a30b06e6 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -27,7 +27,10 @@ public function register_embed() {
 	/**
 	 * Unregister embed.
 	 */
-	public function unregister_embed() {}
+	public function unregister_embed() {
+		remove_filter( 'post_gallery', [ $this, 'maybe_override_gallery' ], 10 );
+		remove_action( 'wp_print_styles', [ $this, 'print_styles' ] );
+	}
 
 	/**
 	 * Shortcode handler.
@@ -61,7 +64,7 @@ public function shortcode( $attr ) {
 		);
 
 		if ( ! empty( $attr['amp-lightbox'] ) ) {
-			$atts['lightbox'] = filter_var( $attr['amp-lightbox'], FILTER_VALIDATE_BOOLEAN );
+			$atts['lightbox'] = rest_sanitize_boolean( $attr['amp-lightbox'] );
 		}
 
 		$id = (int) $atts['id'];
@@ -156,9 +159,17 @@ public function shortcode( $attr ) {
 	 * @return string $html Markup for the gallery.
 	 */
 	public function maybe_override_gallery( $html, $attributes ) {
-		$is_lightbox = isset( $attributes['amp-lightbox'] ) && true === filter_var( $attributes['amp-lightbox'], FILTER_VALIDATE_BOOLEAN );
-		if ( isset( $attributes['amp-carousel'] ) && false === filter_var( $attributes['amp-carousel'], FILTER_VALIDATE_BOOLEAN ) ) {
-			if ( true === $is_lightbox ) {
+		$is_lightbox = isset( $attributes['amp-lightbox'] ) && rest_sanitize_boolean( $attributes['amp-lightbox'] );
+
+		// Use <amp-carousel> for the gallery if requested via amp-carousel shortcode attribute, or use by default if in Reader mode.
+		$is_carousel = (
+			isset( $attributes['amp-carousel'] ) ?
+				rest_sanitize_boolean( $attributes['amp-carousel'] ) :
+				! current_theme_supports( 'amp' ) // In AMP_Gallery_Block_Sanitizer, this is referred to as carousel_required.
+		);
+
+		if ( ! $is_carousel ) {
+			if ( $is_lightbox ) {
 				$add_lightbox_attribute = static function ( $attr ) {
 					$attr['lightbox'] = '';
 					return $attr;
diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index cc66bf4745d..bce5d4d454f 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -81,14 +81,14 @@ public function sanitize() {
 			$gallery_node = isset( $node->parentNode ) && AMP_DOM_Utils::has_class( $node->parentNode, self::$class ) ? $node->parentNode : $node;
 			$attributes   = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $gallery_node );
 
-			$is_amp_lightbox = isset( $attributes['data-amp-lightbox'] ) && true === filter_var( $attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN );
-			$is_amp_carousel = (
-				! empty( $this->args['carousel_required'] )
-				||
-				filter_var( $node->getAttribute( 'data-amp-carousel' ), FILTER_VALIDATE_BOOLEAN )
-				||
-				filter_var( $node->parentNode->getAttribute( 'data-amp-carousel' ), FILTER_VALIDATE_BOOLEAN )
-			);
+			$is_amp_lightbox = isset( $attributes['data-amp-lightbox'] ) && rest_sanitize_boolean( $attributes['data-amp-lightbox'] );
+			if ( $node->hasAttribute( 'data-amp-carousel' ) || $node->parentNode->hasAttribute( 'data-amp-carousel' ) ) {
+				$is_amp_carousel = rest_sanitize_boolean( $node->getAttribute( 'data-amp-carousel' ) ) || rest_sanitize_boolean( $node->parentNode->getAttribute( 'data-amp-carousel' ) );
+			} else {
+				// The carousel_required argument is set to true when the theme does not support AMP. However, it is no
+				// no longer strictly required. Rather, carousels are just enabled by default.
+				$is_amp_carousel = ! empty( $this->args['carousel_required'] );
+			}
 
 			// We are only looking for <ul> elements which have amp-carousel / amp-lightbox true.
 			if ( ! $is_amp_carousel && ! $is_amp_lightbox ) {

From d186af5eed1294cd915a19f7a3e006c317e31f55 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Tue, 30 Jun 2020 18:35:38 -0400
Subject: [PATCH 02/47] Remove `loading` attribute from img tag since
 dimensions are not provided

---
 .../php/test-class-amp-youtube-embed-handler.php | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/tests/php/test-class-amp-youtube-embed-handler.php b/tests/php/test-class-amp-youtube-embed-handler.php
index 71ef3e825ec..056e92c1215 100644
--- a/tests/php/test-class-amp-youtube-embed-handler.php
+++ b/tests/php/test-class-amp-youtube-embed-handler.php
@@ -140,8 +140,6 @@ public function test_video_override() {
 	}
 
 	public function get_conversion_data() {
-		$loading_attribute = version_compare( get_bloginfo( 'version' ), '5.5-alpha', '>' ) ? 'loading="lazy" ' : '';
-
 		return [
 			'no_embed'                         => [
 				'<p>Hello world.</p>',
@@ -150,20 +148,20 @@ public function get_conversion_data() {
 
 			'url_simple'                       => [
 				'https://www.youtube.com/watch?v=kfVsfOSbJY0' . PHP_EOL,
-				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281" title="Rebecca Black - Friday"><a placeholder href="https://www.youtube.com/watch?v=kfVsfOSbJY0"><img ' . $loading_attribute . 'src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover" alt="Rebecca Black - Friday"></img></a></amp-youtube></p>' . PHP_EOL,
-				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281"><a placeholder href="https://www.youtube.com/watch?v=kfVsfOSbJY0"><img ' . $loading_attribute . 'src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover"></img></a></amp-youtube></p>' . PHP_EOL,
+				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281" title="Rebecca Black - Friday"><a placeholder href="https://www.youtube.com/watch?v=kfVsfOSbJY0"><img src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover" alt="Rebecca Black - Friday"></img></a></amp-youtube></p>' . PHP_EOL,
+				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281"><a placeholder href="https://www.youtube.com/watch?v=kfVsfOSbJY0"><img src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover"></img></a></amp-youtube></p>' . PHP_EOL,
 			],
 
 			'url_short'                        => [
 				'https://youtu.be/kfVsfOSbJY0' . PHP_EOL,
-				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281" title="Rebecca Black - Friday"><a placeholder href="https://youtu.be/kfVsfOSbJY0"><img ' . $loading_attribute . 'src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover" alt="Rebecca Black - Friday"></img></a></amp-youtube></p>' . PHP_EOL,
-				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281"><a placeholder href="https://youtu.be/kfVsfOSbJY0"><img ' . $loading_attribute . 'src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover"></img></a></amp-youtube></p>' . PHP_EOL,
+				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281" title="Rebecca Black - Friday"><a placeholder href="https://youtu.be/kfVsfOSbJY0"><img src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover" alt="Rebecca Black - Friday"></img></a></amp-youtube></p>' . PHP_EOL,
+				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281"><a placeholder href="https://youtu.be/kfVsfOSbJY0"><img src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover"></img></a></amp-youtube></p>' . PHP_EOL,
 			],
 
 			'url_with_querystring'             => [
 				'http://www.youtube.com/watch?v=kfVsfOSbJY0&hl=en&fs=1&w=425&h=349' . PHP_EOL,
-				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281" title="Rebecca Black - Friday"><a placeholder href="http://www.youtube.com/watch?v=kfVsfOSbJY0&amp;hl=en&amp;fs=1&amp;w=425&amp;h=349"><img ' . $loading_attribute . 'src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover" alt="Rebecca Black - Friday"></img></a></amp-youtube></p>' . PHP_EOL,
-				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281"><a placeholder href="http://www.youtube.com/watch?v=kfVsfOSbJY0&amp;hl=en&amp;fs=1&amp;w=425&amp;h=349"><img ' . $loading_attribute . 'src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover"></img></a></amp-youtube></p>' . PHP_EOL,
+				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281" title="Rebecca Black - Friday"><a placeholder href="http://www.youtube.com/watch?v=kfVsfOSbJY0&amp;hl=en&amp;fs=1&amp;w=425&amp;h=349"><img src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover" alt="Rebecca Black - Friday"></img></a></amp-youtube></p>' . PHP_EOL,
+				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281"><a placeholder href="http://www.youtube.com/watch?v=kfVsfOSbJY0&amp;hl=en&amp;fs=1&amp;w=425&amp;h=349"><img src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover"></img></a></amp-youtube></p>' . PHP_EOL,
 			],
 
 			// Several reports of invalid URLs that have multiple `?` in the URL.
@@ -174,7 +172,7 @@ public function get_conversion_data() {
 
 			'embed_url'                        => [
 				'https://www.youtube.com/embed/kfVsfOSbJY0' . PHP_EOL,
-				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281" title="Rebecca Black - Friday"><a placeholder href="https://youtube.com/watch?v=kfVsfOSbJY0"><img ' . $loading_attribute . 'src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover" alt="Rebecca Black - Friday"></img></a></amp-youtube></p>' . PHP_EOL,
+				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281" title="Rebecca Black - Friday"><a placeholder href="https://youtube.com/watch?v=kfVsfOSbJY0"><img src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover" alt="Rebecca Black - Friday"></img></a></amp-youtube></p>' . PHP_EOL,
 				'<p><amp-youtube data-videoid="kfVsfOSbJY0" layout="responsive" width="500" height="281"><a placeholder href="https://youtube.com/watch?v=kfVsfOSbJY0"><img src="https://i.ytimg.com/vi/kfVsfOSbJY0/hqdefault.jpg" layout="fill" object-fit="cover"></img></a></amp-youtube></p>' . PHP_EOL,
 			],
 		];

From 2a7ace20fd899c78f17926add6ef47007e2d8435 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Tue, 7 Jul 2020 05:23:34 -0400
Subject: [PATCH 03/47] Fix display of HTML captions in AMP carousel

---
 .../class-amp-gallery-embed-handler.php       | 17 +++++-
 .../class-amp-gallery-block-sanitizer.php     | 56 ++++++++++---------
 src/Component/CaptionedSlide.php              | 24 ++++----
 src/Component/Carousel.php                    | 22 +++-----
 src/Component/HasCaption.php                  |  8 ++-
 src/Dom/ElementList.php                       |  6 +-
 ...test-class-amp-gallery-block-sanitizer.php |  4 +-
 tests/php/test-dom-element-list.php           |  6 +-
 8 files changed, 79 insertions(+), 64 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index d49a30b06e6..79f166cb806 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -229,7 +229,9 @@ public function render( $args ) {
 			return '';
 		}
 
-		$images = new ElementList();
+		$images          = new ElementList();
+		$caption_counter = 0;
+
 		foreach ( $args['images'] as $props ) {
 			$image_atts = [
 				'src'    => $props['url'],
@@ -264,7 +266,18 @@ public function render( $args ) {
 			}
 
 			$caption = isset( $props['id'] ) ? wp_get_attachment_caption( $props['id'] ) : '';
-			$images  = $images->add( $image, $caption );
+
+			$caption_id   = 'amp-wp-caption-' . $caption_counter++;
+			$caption_html = sprintf( '<span id="%s">%s</span>', $caption_id, $caption );
+
+			// Create a temporary DOM document to load the HTML string.
+			$tmp_dom = AMP_DOM_Utils::get_dom_from_content( $caption_html );
+
+			// Import the loaded HTML element into the DOM we're working with.
+			$caption_node = $dom->importNode( $tmp_dom->getElementById( $caption_id ), true );
+			$dom->appendChild( $caption_node );
+
+			$images  = $images->add( $image, $dom->getElementById( $caption_id ) );
 		}
 
 		$amp_carousel  = new Carousel( $dom, $images );
diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index bce5d4d454f..df6be1aca73 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -69,12 +69,7 @@ public function sanitize() {
 				]
 			)
 		);
-		$query       = $this->dom->xpath->query( $expr );
-
-		$nodes = [];
-		foreach ( $query as $node ) {
-			$nodes[] = $node;
-		}
+		$nodes       = $this->dom->xpath->query( $expr );
 
 		foreach ( $nodes as $node ) {
 			// In WordPress 5.3, the Gallery block's <ul> is wrapped in a <figure class="wp-block-gallery">, so look for that node also.
@@ -107,23 +102,18 @@ public function sanitize() {
 
 			$images = new ElementList();
 
-			// If it's not AMP lightbox, look for links first.
+			// If it's not AMP lightbox, look for <amg-img> elements.
 			if ( ! $is_amp_lightbox ) {
-				foreach ( $node->getElementsByTagName( 'a' ) as $element ) {
-					$images = $images->add( $element, $this->possibly_get_caption_text( $element ) );
-				}
-			}
+				$img_elements = $node->getElementsByTagName( 'amp-img' );
 
-			// If not linking to anything then look for <amp-img>.
-			if ( 0 === count( $images ) ) {
-				foreach ( $node->getElementsByTagName( 'amp-img' ) as $element ) {
-					$images = $images->add( $element, $this->possibly_get_caption_text( $element ) );
+				// Skip if no images found.
+				if ( 0 === $img_elements->length ) {
+					continue;
 				}
-			}
 
-			// Skip if no images found.
-			if ( 0 === count( $images ) ) {
-				continue;
+				foreach ( $img_elements as $element ) {
+					$images = $images->add( $element, $this->get_caption_element( $element ) );
+				}
 			}
 
 			$amp_carousel = new Carousel( $this->dom, $images );
@@ -154,19 +144,35 @@ protected function add_lightbox_attributes_to_image_nodes( $element ) {
 	 * Gets the caption of an image, if it exists.
 	 *
 	 * @param DOMElement $element The element for which to search for a caption.
-	 * @return string The caption for the image, or ''.
+	 * @return DOMElement A <span> wrapping the content of the image <figcaption>, or null if the caption could not be found.
 	 */
-	public function possibly_get_caption_text( $element ) {
+	public function get_caption_element($element ) {
 		$caption_tag = 'figcaption';
+		$figcaption_element = null;
+
 		if ( isset( $element->nextSibling->nodeName ) && $caption_tag === $element->nextSibling->nodeName ) {
-			return $element->nextSibling->textContent;
+			$figcaption_element = $element->nextSibling;
 		}
 
 		// If 'Link To' is selected, the image will be wrapped in an <a>, so search for the sibling of the <a>.
-		if ( isset( $element->parentNode->nextSibling->nodeName ) && $caption_tag === $element->parentNode->nextSibling->nodeName ) {
-			return $element->parentNode->nextSibling->textContent;
+		if (
+			! $figcaption_element
+			&& isset( $element->parentNode->nextSibling->nodeName )
+			&& $caption_tag === $element->parentNode->nextSibling->nodeName
+		) {
+			$figcaption_element = $element->parentNode->nextSibling;
+		}
+
+		if ( ! $figcaption_element ) {
+			return null;
+		}
+
+		$caption_element = AMP_DOM_Utils::create_node( $this->dom, 'span', [] );
+
+		foreach ( iterator_to_array( $figcaption_element->childNodes ) as $childNode ) {
+			$caption_element->appendChild( $childNode );
 		}
 
-		return '';
+		return $caption_element;
 	}
 }
diff --git a/src/Component/CaptionedSlide.php b/src/Component/CaptionedSlide.php
index 435899519f5..8b38f761709 100644
--- a/src/Component/CaptionedSlide.php
+++ b/src/Component/CaptionedSlide.php
@@ -25,30 +25,30 @@ final class CaptionedSlide implements HasCaption {
 	private $slide_node;
 
 	/**
-	 * The caption text.
+	 * The caption node.
 	 *
-	 * @var string
+	 * @var DOMNode
 	 */
-	private $caption;
+	private $caption_node;
 
 	/**
 	 * Constructs the class.
 	 *
-	 * @param DOMNode $slide_node The slide node, possibly an image.
-	 * @param string  $caption    The caption text.
+	 * @param DOMNode $slide_node   The slide node, possibly an image.
+	 * @param DOMNode $caption_node The caption node.
 	 */
-	public function __construct( DOMNode $slide_node, $caption ) {
-		$this->slide_node = $slide_node;
-		$this->caption    = $caption;
+	public function __construct( DOMNode $slide_node, $caption_node ) {
+		$this->slide_node   = $slide_node;
+		$this->caption_node = $caption_node;
 	}
 
 	/**
-	 * Gets the caption text.
+	 * Gets the caption node.
 	 *
-	 * @return string The caption text.
+	 * @return DOMNode
 	 */
-	public function get_caption() {
-		return $this->caption;
+	public function get_caption_node() {
+		return $this->caption_node;
 	}
 
 	/**
diff --git a/src/Component/Carousel.php b/src/Component/Carousel.php
index 7d1c1e0bd38..6559ddc9728 100644
--- a/src/Component/Carousel.php
+++ b/src/Component/Carousel.php
@@ -81,7 +81,7 @@ public function get_dom_element() {
 
 		foreach ( $this->slides as $slide ) {
 			$slide_node      = $slide instanceof CaptionedSlide ? $slide->get_slide_node() : $slide;
-			$caption         = $slide instanceof HasCaption ? $slide->get_caption() : null;
+			$caption_node    = $slide instanceof HasCaption ? $slide->get_caption_node() : null;
 			$slide_container = AMP_DOM_Utils::create_node(
 				$this->dom,
 				'span', // This cannot be a <div> because if the gallery is inside of a <p>, then the DOM will break.
@@ -100,19 +100,13 @@ public function get_dom_element() {
 
 			$slide_container->appendChild( $slide_node );
 
-			// If there's a caption, wrap it and append it to the slide.
-			if ( $caption ) {
-				$caption_wrapper = AMP_DOM_Utils::create_node(
-					$this->dom,
-					'span', // This cannot be a <div> because if the gallery is inside of a <p>, then the DOM will break.
-					[ 'class' => 'amp-wp-gallery-caption' ]
-				);
-				$caption_span    = AMP_DOM_Utils::create_node( $this->dom, 'span', [] );
-				$text_node       = $this->dom->createTextNode( $caption );
-
-				$caption_span->appendChild( $text_node );
-				$caption_wrapper->appendChild( $caption_span );
-				$slide_container->appendChild( $caption_wrapper );
+			// If there's a caption, append it to the slide.
+			if ( $caption_node ) {
+				if ( $caption_node instanceof DOMElement && 'span' === $caption_node->nodeName ) {
+					$caption_node->setAttribute( 'class', 'amp-wp-gallery-caption' );
+				}
+
+				$slide_container->appendChild( $caption_node );
 			}
 
 			$amp_carousel->appendChild( $slide_container );
diff --git a/src/Component/HasCaption.php b/src/Component/HasCaption.php
index dc76625c22e..fc0002ad974 100644
--- a/src/Component/HasCaption.php
+++ b/src/Component/HasCaption.php
@@ -7,6 +7,8 @@
 
 namespace AmpProject\AmpWP\Component;
 
+use DOMNode;
+
 /**
  * Interface HasCaption
  *
@@ -16,9 +18,9 @@
 interface HasCaption {
 
 	/**
-	 * Gets the caption.
+	 * Gets the caption node.
 	 *
-	 * @return string The caption text.
+	 * @return DOMNode
 	 */
-	public function get_caption();
+	public function get_caption_node();
 }
diff --git a/src/Dom/ElementList.php b/src/Dom/ElementList.php
index b115be2d123..6ecf32646a4 100644
--- a/src/Dom/ElementList.php
+++ b/src/Dom/ElementList.php
@@ -32,12 +32,12 @@ final class ElementList implements IteratorAggregate, Countable {
 	 * Adds an element to the list, possibly with a caption.
 	 *
 	 * @param DOMElement $element The element to add, possibly an image.
-	 * @param string     $caption The caption to add, if any.
+	 * @param DOMElement $caption The caption element for the element.
 	 * @return ElementList A clone of this list, with the new element added.
 	 */
-	public function add( DOMElement $element, $caption = '' ) {
+	public function add( DOMElement $element, $caption = null ) {
 		$cloned_list             = clone $this;
-		$cloned_list->elements[] = empty( $caption ) ? $element : new CaptionedSlide( $element, $caption );
+		$cloned_list->elements[] = $caption === null ? $element : new CaptionedSlide( $element, $caption );
 		return $cloned_list;
 	}
 
diff --git a/tests/php/test-class-amp-gallery-block-sanitizer.php b/tests/php/test-class-amp-gallery-block-sanitizer.php
index 0ac5b1fc576..b75c25f5e05 100644
--- a/tests/php/test-class-amp-gallery-block-sanitizer.php
+++ b/tests/php/test-class-amp-gallery-block-sanitizer.php
@@ -205,7 +205,7 @@ public function get_caption_text_data() {
 	/**
 	 * Test possibly_get_caption_text.
 	 *
-	 * @covers \AMP_Gallery_Block_Sanitizer::possibly_get_caption_text()
+	 * @covers \AMP_Gallery_Block_Sanitizer::get_caption_element()
 	 *
 	 * @dataProvider get_caption_text_data
 	 * @param string      $source The markup source to test.
@@ -220,6 +220,6 @@ public function test_possibly_get_caption_text( $source, $expected, $element_nam
 			[ 'content_max_width' => 600 ]
 		);
 
-		$this->assertEquals( $expected, $sanitizer->possibly_get_caption_text( $element ) );
+		$this->assertEquals( $expected, $sanitizer->get_caption_element( $element ) );
 	}
 }
diff --git a/tests/php/test-dom-element-list.php b/tests/php/test-dom-element-list.php
index f50add9315d..495027307ad 100644
--- a/tests/php/test-dom-element-list.php
+++ b/tests/php/test-dom-element-list.php
@@ -92,13 +92,13 @@ public function test_dom_element_list_get_iterator( $images, $expected_count ) {
 	/**
 	 * Test get_caption.
 	 *
-	 * @covers \AmpProject\AmpWP\Component\CaptionedSlide::get_caption()
+	 * @covers \AmpProject\AmpWP\Component\CaptionedSlide::get_caption_node()
 	 */
 	public function test_get_caption() {
 		$image_node      = AMP_DOM_Utils::create_node( new Document(), 'amp-img', [] );
-		$caption         = 'This is a caption';
+		$caption         = new DOMText( 'This is a caption' );
 		$captioned_image = new CaptionedSlide( $image_node, $caption );
-		$this->assertEquals( $caption, $captioned_image->get_caption() );
+		$this->assertEquals( $caption, $captioned_image->get_caption_node() );
 	}
 
 	/**

From 6d055ad5d3f4c5253a5967623c5819b62b889114 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Tue, 7 Jul 2020 05:53:42 -0400
Subject: [PATCH 04/47] Fix lightbox not showing images

---
 .../class-amp-gallery-block-sanitizer.php     | 54 +++++++++----------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index df6be1aca73..efc43ca7eca 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -90,9 +90,16 @@ public function sanitize() {
 				continue;
 			}
 
+			$img_elements = $node->getElementsByTagName( 'amp-img' );
+
+			// Skip if no images found.
+			if ( 0 === $img_elements->length ) {
+				continue;
+			}
+
 			// If lightbox is set, we should add lightbox feature to the gallery images.
 			if ( $is_amp_lightbox ) {
-				$this->add_lightbox_attributes_to_image_nodes( $node );
+				$this->add_lightbox_attributes_to_image_nodes( $img_elements );
 			}
 
 			// If amp-carousel is not set, nothing else to do here.
@@ -100,23 +107,7 @@ public function sanitize() {
 				continue;
 			}
 
-			$images = new ElementList();
-
-			// If it's not AMP lightbox, look for <amg-img> elements.
-			if ( ! $is_amp_lightbox ) {
-				$img_elements = $node->getElementsByTagName( 'amp-img' );
-
-				// Skip if no images found.
-				if ( 0 === $img_elements->length ) {
-					continue;
-				}
-
-				foreach ( $img_elements as $element ) {
-					$images = $images->add( $element, $this->get_caption_element( $element ) );
-				}
-			}
-
-			$amp_carousel = new Carousel( $this->dom, $images );
+			$amp_carousel = $this->get_amp_carousel( $img_elements );
 			$gallery_node->parentNode->replaceChild( $amp_carousel->get_dom_element(), $gallery_node );
 		}
 		$this->did_convert_elements = true;
@@ -125,19 +116,28 @@ public function sanitize() {
 	/**
 	 * Set lightbox related attributes to <amp-img> within gallery.
 	 *
-	 * @param DOMElement $element The UL element.
+	 * @param DOMNodeList $img_elements List of image elements.
 	 */
-	protected function add_lightbox_attributes_to_image_nodes( $element ) {
-		$images     = $element->getElementsByTagName( 'amp-img' );
-		$num_images = $images->length;
-		if ( 0 === $num_images ) {
-			return;
+	protected function add_lightbox_attributes_to_image_nodes( DOMNodeList $img_elements ) {
+		foreach ( $img_elements as $img_element ) {
+			$img_element->setAttribute( 'lightbox', '' );
 		}
+	}
 
-		for ( $j = $num_images - 1; $j >= 0; $j-- ) {
-			$image_node = $images->item( $j );
-			$image_node->setAttribute( 'lightbox', '' );
+	/**
+	 * Create an AMP carousel component from the list of images specified.
+	 *
+	 * @param DOMNodeList $img_elements
+	 * @return Carousel
+	 */
+	protected function get_amp_carousel( DOMNodeList $img_elements ) {
+		$images = new ElementList();
+
+		foreach ( $img_elements as $element ) {
+			$images = $images->add( $element, $this->get_caption_element( $element ) );
 		}
+
+		return new Carousel( $this->dom, $images );
 	}
 
 	/**

From 19b45c96cb08b9600acad9c91f44bcd45579405e Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Mon, 20 Jul 2020 01:57:19 -0400
Subject: [PATCH 05/47] Refactor logic handling gallery embed shortcodes and
 blocks

---
 .../class-amp-gallery-embed-handler.php       | 344 ++++++------------
 .../class-amp-gallery-block-sanitizer.php     |  91 +----
 src/Component/CaptionedSlide.php              |   2 +-
 src/Component/Carousel.php                    |  20 +-
 src/Dom/ElementList.php                       |   7 +-
 src/Embed/HandlesGalleryEmbed.php             |  99 +++++
 tests/php/test-amp-carousel.php               |  38 +-
 tests/php/test-amp-gallery-embed-handler.php  | 189 ++++++----
 ...test-class-amp-gallery-block-sanitizer.php |  45 ++-
 tests/php/test-dom-element-list.php           |   6 +-
 10 files changed, 430 insertions(+), 411 deletions(-)
 create mode 100644 src/Embed/HandlesGalleryEmbed.php

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index 79f166cb806..aae9efcdc49 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -5,9 +5,8 @@
  * @package AMP
  */
 
+use AmpProject\AmpWP\Embed\HandlesGalleryEmbed;
 use AmpProject\Dom\Document;
-use AmpProject\AmpWP\Dom\ElementList;
-use AmpProject\AmpWP\Component\Carousel;
 
 /**
  * Class AMP_Gallery_Embed_Handler
@@ -16,277 +15,174 @@
  */
 class AMP_Gallery_Embed_Handler extends AMP_Base_Embed_Handler {
 
+	use HandlesGalleryEmbed;
+
 	/**
 	 * Register embed.
 	 */
 	public function register_embed() {
-		add_filter( 'post_gallery', [ $this, 'maybe_override_gallery' ], 10, 2 );
+		add_filter( 'post_gallery', [ $this, 'generate_gallery_markup' ], 10, 2 );
 		add_action( 'wp_print_styles', [ $this, 'print_styles' ] );
 	}
 
 	/**
-	 * Unregister embed.
-	 */
-	public function unregister_embed() {
-		remove_filter( 'post_gallery', [ $this, 'maybe_override_gallery' ], 10 );
-		remove_action( 'wp_print_styles', [ $this, 'print_styles' ] );
-	}
-
-	/**
-	 * Shortcode handler.
+	 * Override the output of gallery_shortcode().
 	 *
-	 * @param array $attr Shortcode attributes.
-	 * @return string Rendered gallery.
+	 * @param string $html  Markup to filter.
+	 * @param array  $attrs Shortcode attributes.
+	 * @return string Markup for the gallery.
 	 */
-	public function shortcode( $attr ) {
-		$post = get_post();
-
-		if ( ! empty( $attr['ids'] ) ) {
-			// 'ids' is explicitly ordered, unless you specify otherwise.
-			if ( empty( $attr['orderby'] ) ) {
-				$attr['orderby'] = 'post__in';
-			}
-			$attr['include'] = $attr['ids'];
-		}
+	public function generate_gallery_markup( $html, $attrs ) {
+		// Use <amp-carousel> for the gallery if requested via amp-carousel shortcode attribute, or use by default if in Reader mode.
+		// In AMP_Gallery_Block_Sanitizer, this is referred to as carousel_required.
+		$is_carousel = isset( $attrs['amp-carousel'] )
+			? rest_sanitize_boolean( $attrs['amp-carousel'] )
+			: amp_is_legacy();
 
-		$atts = shortcode_atts(
-			[
-				'order'   => 'ASC',
-				'orderby' => 'menu_order ID',
-				'id'      => $post ? $post->ID : 0,
-				'include' => '',
-				'exclude' => '',
-				'size'    => [ $this->args['width'], $this->args['height'] ],
-				'link'    => 'none',
-			],
-			$attr,
-			'gallery'
-		);
+		$is_lightbox = isset( $attrs['amp-lightbox'] ) && rest_sanitize_boolean( $attrs['amp-lightbox'] );
 
-		if ( ! empty( $attr['amp-lightbox'] ) ) {
-			$atts['lightbox'] = rest_sanitize_boolean( $attr['amp-lightbox'] );
+		if ( ! $is_carousel && ! $is_lightbox ) {
+			return $html;
 		}
 
-		$id = (int) $atts['id'];
-
-		if ( ! empty( $atts['include'] ) ) {
-			$attachments = get_posts(
-				[
-					'include'        => $atts['include'],
-					'post_status'    => 'inherit',
-					'post_type'      => 'attachment',
-					'post_mime_type' => 'image',
-					'order'          => $atts['order'],
-					'orderby'        => $atts['orderby'],
-					'fields'         => 'ids',
-				]
-			);
-		} elseif ( ! empty( $atts['exclude'] ) ) {
-			$attachments = get_children(
-				[
-					'post_parent'    => $id,
-					'exclude'        => $atts['exclude'],
-					'post_status'    => 'inherit',
-					'post_type'      => 'attachment',
-					'post_mime_type' => 'image',
-					'order'          => $atts['order'],
-					'orderby'        => $atts['orderby'],
-					'fields'         => 'ids',
-				]
-			);
-		} else {
-			$attachments = get_children(
-				[
-					'post_parent'    => $id,
-					'post_status'    => 'inherit',
-					'post_type'      => 'attachment',
-					'post_mime_type' => 'image',
-					'order'          => $atts['order'],
-					'orderby'        => $atts['orderby'],
-					'fields'         => 'ids',
-				]
-			);
+		if ( $is_carousel ) {
+			$gallery_size = isset( $attrs['size'] ) ? $attrs['size'] : null;
+
+			if ( $gallery_size && 'thumbnail' === $gallery_size ) {
+				/*
+				 * If the 'gallery' shortcode has a `size` attribute of `thumbnail`, prevent outputting an <amp-carousel>.
+				 * That will often get thumbnail images around 150 x 150,
+				 * while the <amp-carousel> usually has a width of 600 and a height of 480.
+				 * That often means very low-resolution images.
+				 * So fall back to the normal 'gallery' shortcode callback, gallery_shortcode().
+				 */
+				return $html;
+			}
+
+			if ( ! $gallery_size ) {
+				// Default to `large` if no `size` attribute is specified.
+				$attrs['size'] = 'large';
+			}
 		}
 
-		if ( empty( $attachments ) ) {
-			return '';
+		if ( $is_lightbox ) {
+			// Prevent wrapping the images in anchor tags if a lightbox is specified. If that is done the link will get
+			// preference over the lightbox when the image is clicked.
+			$attrs['link'] = 'none';
 		}
 
-		$urls = [];
-		foreach ( $attachments as $attachment_id ) {
-			list( $url, $width, $height ) = wp_get_attachment_image_src( $attachment_id, $atts['size'], true );
+		// Use `data` attributes to indicate which options are configured for the embed. These indications are later
+		// processed during sanitization of the embed in `::sanitize_raw_embeds`.
+		add_filter(
+			'gallery_style',
+			static function ( $style ) use ( $is_carousel, $is_lightbox ) {
+				$data_attrs = [];
 
-			if ( ! $url ) {
-				continue;
-			}
+				if ( $is_lightbox ) {
+					$data_attrs[] = 'data-amp-lightbox';
+				}
 
-			$href = null;
-			if ( empty( $atts['lightbox'] ) && ! empty( $atts['link'] ) ) {
-				if ( 'file' === $atts['link'] ) {
-					$href = $url;
-				} elseif ( 'post' === $atts['link'] ) {
-					$href = get_attachment_link( $attachment_id );
+				if ( $is_carousel ) {
+					$data_attrs[] = 'data-amp-carousel';
 				}
+
+				return str_replace( "class='", implode( ' ', $data_attrs ) . " class='", $style );
 			}
+		);
 
-			$urls[] = [
-				'href'   => $href,
-				'url'    => $url,
-				'srcset' => wp_get_attachment_image_srcset( $attachment_id, $atts['size'] ),
-				'width'  => $width,
-				'height' => $height,
-				'alt'    => trim( wp_strip_all_tags( get_post_meta( $attachment_id, '_wp_attachment_image_alt', true ) ) ), // Logic from wp_get_attachment_image().
-				'id'     => $attachment_id,
-			];
-		}
+		remove_filter( 'post_gallery', [ $this, 'generate_gallery_markup' ] );
+		$gallery_html = gallery_shortcode( $attrs );
+		add_filter( 'post_gallery', [ $this, 'generate_gallery_markup' ], 10, 2 );
 
-		$args = [
-			'images'   => $urls,
-			'lightbox' => ! empty( $atts['lightbox'] ),
-		];
+		return $gallery_html;
+	}
 
-		return $this->render( $args );
+	/**
+	 * Unregister embed.
+	 */
+	public function unregister_embed() {
+		remove_filter( 'post_gallery', [ $this, 'generate_gallery_markup' ] );
+		remove_action( 'wp_print_styles', [ $this, 'print_styles' ] );
 	}
 
 	/**
-	 * Override the output of gallery_shortcode() if amp-carousel is not false.
-	 *
-	 * The 'Gallery' widget also uses this function.
-	 * This ensures that it outputs an <amp-carousel>.
+	 * Sanitizes gallery raw embeds to become an amp-carousel and/or amp-image-lightbox, depending on configuration options.
 	 *
-	 * @param string $html Markup to filter, possibly ''.
-	 * @param array  $attributes Shortcode attributes.
-	 * @return string $html Markup for the gallery.
+	 * @param Document $dom DOM.
 	 */
-	public function maybe_override_gallery( $html, $attributes ) {
-		$is_lightbox = isset( $attributes['amp-lightbox'] ) && rest_sanitize_boolean( $attributes['amp-lightbox'] );
-
-		// Use <amp-carousel> for the gallery if requested via amp-carousel shortcode attribute, or use by default if in Reader mode.
-		$is_carousel = (
-			isset( $attributes['amp-carousel'] ) ?
-				rest_sanitize_boolean( $attributes['amp-carousel'] ) :
-				! current_theme_supports( 'amp' ) // In AMP_Gallery_Block_Sanitizer, this is referred to as carousel_required.
-		);
-
-		if ( ! $is_carousel ) {
-			if ( $is_lightbox ) {
-				$add_lightbox_attribute = static function ( $attr ) {
-					$attr['lightbox'] = '';
-					return $attr;
-				};
-
-				$set_link_attribute = static function ( $attributes ) {
-					$attributes['link'] = 'none';
-					return $attributes;
-				};
+	public function sanitize_raw_embeds( Document $dom ) {
+		$nodes = $dom->xpath->query( '//div[ contains( concat( " ", normalize-space( @class ), " " ), " gallery " ) and ( @data-amp-carousel or @data-amp-lightbox ) ]' );
 
-				remove_filter( 'post_gallery', [ $this, 'maybe_override_gallery' ], 10 );
-				add_filter( 'wp_get_attachment_image_attributes', $add_lightbox_attribute );
-				add_filter( 'shortcode_atts_gallery', $set_link_attribute, PHP_INT_MAX );
+		/** @var DOMElement $node */
+		foreach ( $nodes as $node ) {
+			$is_carousel  = $node->hasAttribute( 'data-amp-carousel' );
+			$is_lightbox  = $node->hasAttribute( 'data-amp-lightbox' );
+			$img_elements = $node->getElementsByTagName( 'img' );
 
-				$html = gallery_shortcode( $attributes );
+			$node->removeAttribute( 'data-amp-carousel' );
+			$node->removeAttribute( 'data-amp-lightbox' );
 
-				remove_filter( 'wp_get_attachment_image_attributes', $add_lightbox_attribute );
-				add_filter( 'post_gallery', [ $this, 'maybe_override_gallery' ], 10, 2 );
-				remove_filter( 'shortcode_atts_gallery', $set_link_attribute, PHP_INT_MAX );
-			}
-
-			return $html;
+			$this->process_gallery_embed( $is_carousel, $is_lightbox, $node, $img_elements );
 		}
-
-		if ( isset( $attributes['size'] ) && 'thumbnail' === $attributes['size'] ) {
-			/*
-			 * If the 'gallery' shortcode has a 'size' attribute of 'thumbnail', prevent outputting an <amp-carousel>.
-			 * That will often get thumbnail images around 150 x 150,
-			 * while the <amp-carousel> usually has a width of 600 and a height of 480.
-			 * That often means very low-resolution images.
-			 * So fall back to the normal 'gallery' shortcode callback, gallery_shortcode().
-			 */
-			return '';
-		}
-
-		return $this->shortcode( $attributes );
 	}
 
 	/**
-	 * Render.
+	 * Get the caption element for the specified image element.
 	 *
-	 * @param array $args Args.
-	 * @return string Rendered.
+	 * @param DOMElement $img_element Image element.
+	 * @return DOMElement|null The caption element, otherwise `null` if it could not be found.
 	 */
-	public function render( $args ) {
-		$dom                        = new Document();
-		$this->did_convert_elements = true;
-
-		$args = wp_parse_args(
-			$args,
-			[
-				'images' => false,
-			]
-		);
+	protected function get_caption_element( DOMElement $img_element ) {
+		$parent_element = $this->get_parent_container_for_image( $img_element );
 
-		if ( empty( $args['images'] ) ) {
-			return '';
+		if ( ! $parent_element instanceof DOMElement ) {
+			return null;
 		}
 
-		$images          = new ElementList();
-		$caption_counter = 0;
-
-		foreach ( $args['images'] as $props ) {
-			$image_atts = [
-				'src'    => $props['url'],
-				'width'  => $props['width'],
-				'height' => $props['height'],
-				'layout' => 'responsive',
-				'alt'    => $props['alt'],
-			];
-			if ( ! empty( $props['srcset'] ) ) {
-				$image_atts['srcset'] = $props['srcset'];
-			}
-
-			if ( ! empty( $args['lightbox'] ) ) {
-				$image_atts['lightbox'] = '';
-			}
-			$image = AMP_DOM_Utils::create_node(
-				$dom,
-				'img',
-				$image_atts
-			);
-
-			if ( ! empty( $props['href'] ) ) {
-				$previous_image = $image;
-				$image          = AMP_DOM_Utils::create_node(
-					$dom,
-					'a',
-					[
-						'href' => $props['href'],
-					]
-				);
-				$image->appendChild( $previous_image );
-			}
+		// The caption should be next immediate element located next to the parent container of the image.
+		$caption_element = $parent_element->nextSibling;
 
-			$caption = isset( $props['id'] ) ? wp_get_attachment_caption( $props['id'] ) : '';
-
-			$caption_id   = 'amp-wp-caption-' . $caption_counter++;
-			$caption_html = sprintf( '<span id="%s">%s</span>', $caption_id, $caption );
+		while (
+			$caption_element
+			&& ( ! $caption_element instanceof DOMElement || ! AMP_DOM_Utils::has_class( $caption_element, 'wp-caption-text' ) )
+		) {
+			$caption_element = $caption_element->nextSibling;
+		}
 
-			// Create a temporary DOM document to load the HTML string.
-			$tmp_dom = AMP_DOM_Utils::get_dom_from_content( $caption_html );
+		if ( $caption_element instanceof DOMElement && 'figcaption' !== $caption_element->nodeName ) {
+			// Transform the caption element into a `figcaption`. This not only allows the `amp-lightbox` to correctly
+			// detect and display the caption, but it is also semantically correct as the parent element will be a `figure`.
+			$figcaption_element = AMP_DOM_Utils::create_node( Document::fromNode( $caption_element ), 'figcaption', [] );
 
-			// Import the loaded HTML element into the DOM we're working with.
-			$caption_node = $dom->importNode( $tmp_dom->getElementById( $caption_id ), true );
-			$dom->appendChild( $caption_node );
+			while ( $caption_element->hasChildNodes() ) {
+				$child = $caption_element->childNodes->item( 0 );
+				$child = $caption_element->ownerDocument->importNode( $child, true );
+				$figcaption_element->appendChild( $child );
+			}
 
-			$images  = $images->add( $image, $dom->getElementById( $caption_id ) );
+			$caption_element = $figcaption_element;
 		}
 
-		$amp_carousel  = new Carousel( $dom, $images );
-		$carousel_node = $amp_carousel->get_dom_element();
+		return $caption_element instanceof DOMElement ? $caption_element : null;
+	}
 
-		// Prevent an error in get_content_from_dom_node() when it calls $node->parentNode->insertBefore().
-		$dom->appendChild( $carousel_node );
+	/**
+	 * Get the parent container for the specified image element.
+	 *
+	 * @param DOMElement $image_element Image element.
+	 * @return DOMElement|null The parent container, otherwise `null` if it could not be found.
+	 */
+	protected function get_parent_container_for_image( DOMElement $image_element ) {
+		$parent_element = $image_element->parentNode;
+
+		while (
+			$parent_element
+			&& ( ! $parent_element instanceof DOMElement || ! AMP_DOM_Utils::has_class( $parent_element, 'gallery-icon' ) )
+		) {
+			$parent_element = $parent_element->parentNode;
+		}
 
-		return $dom->saveHTML( $carousel_node );
+		return $parent_element;
 	}
 
 	/**
@@ -297,8 +193,6 @@ public function render( $args ) {
 	 * This rule is copied exactly from block-library/style.css, but the selector here has amp-img >.
 	 * The image sanitizer normally converts the <img> from that original stylesheet <amp-img>,
 	 * but that doesn't have the same effect as applying it to the <img>.
-	 *
-	 * @return void
 	 */
 	public function print_styles() {
 		?>
diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index efc43ca7eca..dd4739cd530 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -5,8 +5,7 @@
  * @package AMP
  */
 
-use AmpProject\AmpWP\Dom\ElementList;
-use AmpProject\AmpWP\Component\Carousel;
+use AmpProject\AmpWP\Embed\HandlesGalleryEmbed;
 
 /**
  * Class AMP_Gallery_Block_Sanitizer
@@ -15,6 +14,8 @@
  */
 class AMP_Gallery_Block_Sanitizer extends AMP_Base_Sanitizer {
 
+	use HandlesGalleryEmbed;
+
 	/**
 	 * Tag.
 	 *
@@ -77,102 +78,48 @@ public function sanitize() {
 			$attributes   = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $gallery_node );
 
 			$is_amp_lightbox = isset( $attributes['data-amp-lightbox'] ) && rest_sanitize_boolean( $attributes['data-amp-lightbox'] );
-			if ( $node->hasAttribute( 'data-amp-carousel' ) || $node->parentNode->hasAttribute( 'data-amp-carousel' ) ) {
-				$is_amp_carousel = rest_sanitize_boolean( $node->getAttribute( 'data-amp-carousel' ) ) || rest_sanitize_boolean( $node->parentNode->getAttribute( 'data-amp-carousel' ) );
+
+			if ( $gallery_node->hasAttribute( 'data-amp-carousel' ) ) {
+				$is_amp_carousel = rest_sanitize_boolean( $gallery_node->getAttribute( 'data-amp-carousel' ) );
 			} else {
 				// The carousel_required argument is set to true when the theme does not support AMP. However, it is no
 				// no longer strictly required. Rather, carousels are just enabled by default.
 				$is_amp_carousel = ! empty( $this->args['carousel_required'] );
 			}
 
-			// We are only looking for <ul> elements which have amp-carousel / amp-lightbox true.
-			if ( ! $is_amp_carousel && ! $is_amp_lightbox ) {
-				continue;
-			}
-
 			$img_elements = $node->getElementsByTagName( 'amp-img' );
 
-			// Skip if no images found.
-			if ( 0 === $img_elements->length ) {
-				continue;
-			}
-
-			// If lightbox is set, we should add lightbox feature to the gallery images.
-			if ( $is_amp_lightbox ) {
-				$this->add_lightbox_attributes_to_image_nodes( $img_elements );
-			}
-
-			// If amp-carousel is not set, nothing else to do here.
-			if ( ! $is_amp_carousel ) {
-				continue;
-			}
-
-			$amp_carousel = $this->get_amp_carousel( $img_elements );
-			$gallery_node->parentNode->replaceChild( $amp_carousel->get_dom_element(), $gallery_node );
+			$this->process_gallery_embed( $is_amp_carousel, $is_amp_lightbox, $gallery_node, $img_elements );
 		}
-		$this->did_convert_elements = true;
 	}
 
 	/**
-	 * Set lightbox related attributes to <amp-img> within gallery.
+	 * Get the caption element for the specified image element.
 	 *
-	 * @param DOMNodeList $img_elements List of image elements.
+	 * @param DOMElement $img_element Image element.
+	 * @return DOMElement|null The caption element, or `null` if the image has none.
 	 */
-	protected function add_lightbox_attributes_to_image_nodes( DOMNodeList $img_elements ) {
-		foreach ( $img_elements as $img_element ) {
-			$img_element->setAttribute( 'lightbox', '' );
-		}
-	}
-
-	/**
-	 * Create an AMP carousel component from the list of images specified.
-	 *
-	 * @param DOMNodeList $img_elements
-	 * @return Carousel
-	 */
-	protected function get_amp_carousel( DOMNodeList $img_elements ) {
-		$images = new ElementList();
-
-		foreach ( $img_elements as $element ) {
-			$images = $images->add( $element, $this->get_caption_element( $element ) );
-		}
-
-		return new Carousel( $this->dom, $images );
-	}
-
-	/**
-	 * Gets the caption of an image, if it exists.
-	 *
-	 * @param DOMElement $element The element for which to search for a caption.
-	 * @return DOMElement A <span> wrapping the content of the image <figcaption>, or null if the caption could not be found.
-	 */
-	public function get_caption_element($element ) {
-		$caption_tag = 'figcaption';
+	protected function get_caption_element( DOMElement $img_element ) {
+		$caption_tag        = 'figcaption';
 		$figcaption_element = null;
 
-		if ( isset( $element->nextSibling->nodeName ) && $caption_tag === $element->nextSibling->nodeName ) {
-			$figcaption_element = $element->nextSibling;
+		if ( isset( $img_element->nextSibling->nodeName ) && $caption_tag === $img_element->nextSibling->nodeName ) {
+			$figcaption_element = $img_element->nextSibling;
 		}
 
 		// If 'Link To' is selected, the image will be wrapped in an <a>, so search for the sibling of the <a>.
 		if (
 			! $figcaption_element
-			&& isset( $element->parentNode->nextSibling->nodeName )
-			&& $caption_tag === $element->parentNode->nextSibling->nodeName
+			&& isset( $img_element->parentNode->nextSibling->nodeName )
+			&& $caption_tag === $img_element->parentNode->nextSibling->nodeName
 		) {
-			$figcaption_element = $element->parentNode->nextSibling;
+			$figcaption_element = $img_element->parentNode->nextSibling;
 		}
 
-		if ( ! $figcaption_element ) {
+		if ( $figcaption_element instanceof DOMElement && 0 === $figcaption_element->childNodes->length ) {
 			return null;
 		}
 
-		$caption_element = AMP_DOM_Utils::create_node( $this->dom, 'span', [] );
-
-		foreach ( iterator_to_array( $figcaption_element->childNodes ) as $childNode ) {
-			$caption_element->appendChild( $childNode );
-		}
-
-		return $caption_element;
+		return $figcaption_element;
 	}
 }
diff --git a/src/Component/CaptionedSlide.php b/src/Component/CaptionedSlide.php
index 8b38f761709..ad6cd6fcbc5 100644
--- a/src/Component/CaptionedSlide.php
+++ b/src/Component/CaptionedSlide.php
@@ -37,7 +37,7 @@ final class CaptionedSlide implements HasCaption {
 	 * @param DOMNode $slide_node   The slide node, possibly an image.
 	 * @param DOMNode $caption_node The caption node.
 	 */
-	public function __construct( DOMNode $slide_node, $caption_node ) {
+	public function __construct( DOMNode $slide_node, DOMNode $caption_node ) {
 		$this->slide_node   = $slide_node;
 		$this->caption_node = $caption_node;
 	}
diff --git a/src/Component/Carousel.php b/src/Component/Carousel.php
index 6559ddc9728..afcf5a09eda 100644
--- a/src/Component/Carousel.php
+++ b/src/Component/Carousel.php
@@ -7,6 +7,7 @@
 
 namespace AmpProject\AmpWP\Component;
 
+use AmpProject\Attribute;
 use AmpProject\Dom\Document;
 use AmpProject\AmpWP\Dom\ElementList;
 use DOMElement;
@@ -84,7 +85,7 @@ public function get_dom_element() {
 			$caption_node    = $slide instanceof HasCaption ? $slide->get_caption_node() : null;
 			$slide_container = AMP_DOM_Utils::create_node(
 				$this->dom,
-				'span', // This cannot be a <div> because if the gallery is inside of a <p>, then the DOM will break.
+				'figure', // This cannot be a <div> because if the gallery is inside of a <p>, then the DOM will break.
 				[ 'class' => 'slide' ]
 			);
 
@@ -101,9 +102,20 @@ public function get_dom_element() {
 			$slide_container->appendChild( $slide_node );
 
 			// If there's a caption, append it to the slide.
-			if ( $caption_node ) {
-				if ( $caption_node instanceof DOMElement && 'span' === $caption_node->nodeName ) {
-					$caption_node->setAttribute( 'class', 'amp-wp-gallery-caption' );
+			if ( $caption_node instanceof DOMElement ) {
+				// If the caption is not a <figcaption>, wrap it in one.
+				if ( 'figcaption' !== $caption_node->nodeName ) {
+					$caption_content = $caption_node;
+					$caption_node    = AMP_DOM_Utils::create_node( $this->dom, 'figcaption', [] );
+					$caption_node->appendChild( $caption_content );
+				}
+
+				$caption_shortcode_class = 'wp-caption-text';
+				$caption_block_class     = 'wp-block-image';
+				$has_caption_class       = AMP_DOM_Utils::has_class( $caption_node, $caption_shortcode_class ) || AMP_DOM_Utils::has_class( $caption_node, $caption_block_class );
+
+				if ( ! $has_caption_class || ! $caption_node->hasAttribute( Attribute::CLASS_ ) ) {
+					$caption_node->setAttribute( Attribute::CLASS_, 'amp-wp-gallery-caption' );
 				}
 
 				$slide_container->appendChild( $caption_node );
diff --git a/src/Dom/ElementList.php b/src/Dom/ElementList.php
index 6ecf32646a4..93630121194 100644
--- a/src/Dom/ElementList.php
+++ b/src/Dom/ElementList.php
@@ -8,6 +8,7 @@
 namespace AmpProject\AmpWP\Dom;
 
 use AmpProject\AmpWP\Component\CaptionedSlide;
+use DOMNode;
 use IteratorAggregate;
 use Countable;
 use DOMElement;
@@ -31,11 +32,11 @@ final class ElementList implements IteratorAggregate, Countable {
 	/**
 	 * Adds an element to the list, possibly with a caption.
 	 *
-	 * @param DOMElement $element The element to add, possibly an image.
-	 * @param DOMElement $caption The caption element for the element.
+	 * @param DOMElement   $element The element to add, possibly an image.
+	 * @param DOMNode|null $caption The caption for the element.
 	 * @return ElementList A clone of this list, with the new element added.
 	 */
-	public function add( DOMElement $element, $caption = null ) {
+	public function add( DOMElement $element, DOMNode $caption = null ) {
 		$cloned_list             = clone $this;
 		$cloned_list->elements[] = $caption === null ? $element : new CaptionedSlide( $element, $caption );
 		return $cloned_list;
diff --git a/src/Embed/HandlesGalleryEmbed.php b/src/Embed/HandlesGalleryEmbed.php
new file mode 100644
index 00000000000..b29289f423b
--- /dev/null
+++ b/src/Embed/HandlesGalleryEmbed.php
@@ -0,0 +1,99 @@
+<?php
+/**
+ * Trait HandlesGalleryEmbed.
+ *
+ * @package AmpProject\AmpWP
+ */
+
+namespace AmpProject\AmpWP\Embed;
+
+use AmpProject\AmpWP\Component\Carousel;
+use AmpProject\AmpWP\Dom\ElementList;
+use AmpProject\Attribute;
+use AmpProject\Dom\Document;
+use DOMElement;
+use DOMNodeList;
+
+/**
+ * Trait HandlesGalleryEmbed
+ *
+ * Contains logic related to both gallery embed shortcodes and blocks.
+ *
+ * @since 2.0
+ */
+trait HandlesGalleryEmbed {
+
+	/**
+	 * Transforms the raw embed gallery embed to become AMP compatible.
+	 *
+	 * @param bool        $is_carousel Whether the embed should be transformed into an <amp-carousel>
+	 * @param bool        $is_lightbox Whether the gallery images should be shown in a lightbox.
+	 * @param DOMElement  $gallery_element Gallery element.
+	 * @param DOMNodeList $img_elements
+	 */
+	protected function process_gallery_embed( $is_carousel, $is_lightbox, DOMElement $gallery_element, DOMNodeList $img_elements ) {
+		// Bail if the embed does not support carousel or lightbox.
+		if ( ! $is_carousel && ! $is_lightbox ) {
+			return;
+		}
+
+		// Bail if there are no images.
+		if ( 0 === $img_elements->length ) {
+			return;
+		}
+
+		// If the carousel is not required but the lightbox is, add the `lightbox` attribute to each image and return.
+		if ( ! $is_carousel && $is_lightbox ) {
+			$this->add_lightbox_attribute_to_img_nodes( $img_elements );
+			return;
+		}
+
+		if ( $is_carousel ) {
+			$amp_carousel     = $this->generate_amp_carousel( $img_elements, $is_lightbox );
+			$carousel_element = $amp_carousel->get_dom_element();
+
+			if ( $is_lightbox ) {
+				$carousel_element->setAttribute( Attribute::LIGHTBOX, '' );
+			}
+
+			$gallery_element->parentNode->replaceChild( $carousel_element, $gallery_element );
+		}
+	}
+
+	/**
+	 * Create an AMP carousel component from the list of images specified.
+	 *
+	 * @param DOMNodeList $img_elements    List of images in the gallery.
+	 * @param boolean     $is_amp_lightbox Whether the gallery should have a lightbox.
+	 * @return Carousel An object containing markup for <amp-carousel>.
+	 */
+	protected function generate_amp_carousel( DOMNodeList $img_elements, $is_amp_lightbox ) {
+		$images = new ElementList();
+
+		foreach ( $img_elements as $img_element ) {
+			$element           = $img_element;
+			$parentElementName = $img_element->parentNode->nodeName;
+
+			if ( 'a' === $parentElementName && ! $is_amp_lightbox ) {
+				$element = $img_element->parentNode;
+			}
+
+			$images = $images->add( $element, $this->get_caption_element( $img_element ) );
+		}
+
+		$node = $img_elements->item( 0 );
+		return new Carousel( Document::fromNode( $node ), $images );
+	}
+
+	/**
+	 * Sets the `lightbox` attribute to each image in the specified list.
+	 *
+	 * @param DOMNodeList $img_elements List of image elements.
+	 */
+	protected function add_lightbox_attribute_to_img_nodes( DOMNodeList $img_elements ) {
+		/** @var DOMElement $img_element */
+		foreach ( $img_elements as $img_element ) {
+			$img_element->setAttribute( Attribute::LIGHTBOX, '' );
+		}
+	}
+}
diff --git a/tests/php/test-amp-carousel.php b/tests/php/test-amp-carousel.php
index 32857abdd27..06add4cb031 100644
--- a/tests/php/test-amp-carousel.php
+++ b/tests/php/test-amp-carousel.php
@@ -25,27 +25,35 @@ class Test_Carousel extends \WP_UnitTestCase {
 	 * @return array[] An associative array, including the slides and captions, the DOM, and the expected markup.
 	 */
 	public function get_carousel_data() {
-		$dom     = new Document();
-		$src     = 'https://example.com/img.png';
-		$width   = '1200';
-		$height  = '800';
-		$image   = AMP_DOM_Utils::create_node(
+		$dom    = new Document();
+		$src    = 'https://example.com/img.png';
+		$width  = '1200';
+		$height = '800';
+		$image  = AMP_DOM_Utils::create_node(
 			$dom,
 			'amp-img',
 			compact( 'src', 'width', 'height' )
 		);
-		$caption = 'Example caption';
+
+		$caption_element = AMP_DOM_Utils::create_node( $dom, 'a', [ 'href' => 'example.org' ] );
+		$caption_element->appendChild( new DOMText( 'Example caption' ) );
 
 		return [
-			'image_without_caption' => [
-				( new ElementList() )->add( $image, '' ),
+			'image_without_caption'   => [
+				( new ElementList() )->add( $image, null ),
+				$dom,
+				'<amp-carousel width="' . $width . '" height="' . $height . '" type="slides" layout="responsive"><figure class="slide"><amp-img src="' . $src . '" width="' . $width . '" height="' . $height . '" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
+			],
+			'image_with_text_caption' => [
+				( new ElementList() )->add( $image, new DOMText( 'Example caption' ) ),
 				$dom,
-				'<amp-carousel width="' . $width . '" height="' . $height . '" type="slides" layout="responsive"><span class="slide"><amp-img src="' . $src . '" width="' . $width . '" height="' . $height . '" layout="fill" object-fit="cover"></amp-img></span></amp-carousel>',
+				'<amp-carousel width="' . $width . '" height="' . $height . '" type="slides" layout="responsive"><figure class="slide"><amp-img src="' . $src . '" width="' . $width . '" height="' . $height . '" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption">Example caption</figcaption></figure></amp-carousel>',
 			],
-			'image_with_caption'    => [
-				( new ElementList() )->add( $image, $caption ),
+
+			'image_with_html_caption' => [
+				( new ElementList() )->add( $image, $caption_element ),
 				$dom,
-				'<amp-carousel width="' . $width . '" height="' . $height . '" type="slides" layout="responsive"><span class="slide"><amp-img src="' . $src . '" width="' . $width . '" height="' . $height . '" layout="fill" object-fit="cover"></amp-img><span class="amp-wp-gallery-caption"><span>' . $caption . '</span></span></span></amp-carousel>',
+				'<amp-carousel width="' . $width . '" height="' . $height . '" type="slides" layout="responsive"><figure class="slide"><amp-img src="' . $src . '" width="' . $width . '" height="' . $height . '" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption"><a href="example.org">Example caption</a></figcaption></figure></amp-carousel>',
 			],
 		];
 	}
@@ -129,15 +137,15 @@ public function get_data_carousel_dimensions() {
 				[ $wide_image_width, $wide_image_height ],
 			],
 			'image_with_0_height_should_not_affect_ratio' => [
-				( new ElementList() )->add( $image_with_0_height )->add( $wide_image, '' ),
+				( new ElementList() )->add( $image_with_0_height )->add( $wide_image, null ),
 				[ $wide_image_width, $wide_image_height ],
 			],
 			'two_images'                                  => [
-				( new ElementList() )->add( $narrow_image )->add( $wide_image, '' ),
+				( new ElementList() )->add( $narrow_image )->add( $wide_image, null ),
 				[ $wide_image_width, $wide_image_height ],
 			],
 			'two_images_order_changed'                    => [
-				( new ElementList() )->add( $wide_image )->add( $narrow_image, '' ),
+				( new ElementList() )->add( $wide_image )->add( $narrow_image, null ),
 				[ $wide_image_width, $wide_image_height ],
 			],
 		];
diff --git a/tests/php/test-amp-gallery-embed-handler.php b/tests/php/test-amp-gallery-embed-handler.php
index 85021a780dc..6e83acd7ac0 100644
--- a/tests/php/test-amp-gallery-embed-handler.php
+++ b/tests/php/test-amp-gallery-embed-handler.php
@@ -5,6 +5,8 @@
  * @package AMP
  */
 
+use AmpProject\AmpWP\Admin\ReaderThemes;
+use AmpProject\AmpWP\Option;
 use AmpProject\AmpWP\Tests\Helpers\WithoutBlockPreRendering;
 
 /**
@@ -14,6 +16,16 @@ class AMP_Gallery_Embed_Handler_Test extends WP_UnitTestCase {
 
 	use WithoutBlockPreRendering;
 
+	private static $original_amp_options;
+
+	public static function setUpBeforeClass() {
+		self::$original_amp_options = AMP_Options_Manager::get_options();
+	}
+
+	public static function tearDownAfterClass() {
+		AMP_Options_Manager::update_options( self::$original_amp_options );
+	}
+
 	/**
 	 * Tear down.
 	 */
@@ -44,7 +56,7 @@ public function tearDown() {
 	 * @return array[]
 	 */
 	public function get_conversion_data() {
-		$amp_carousel_caption = '<span class="amp-wp-gallery-caption"><span>' . self::CAPTION_TEXT . '</span></span>';
+		$amp_carousel_caption = '<figcaption class="amp-wp-gallery-caption"> ' . self::CAPTION_TEXT . ' </figcaption>';
 		$loading_attribute    = version_compare( get_bloginfo( 'version' ), '5.5-alpha', '>' ) ? 'loading="lazy"' : '';
 
 		return [
@@ -52,88 +64,99 @@ public function get_conversion_data() {
 				'[gallery ids=1]',
 				'',
 			],
-			'shortcode_with_valid_id'                 => [
-				'[gallery ids={{id1}}]',
-				'<amp-carousel width="50" height="50" type="slides" layout="responsive"><span class="slide"><img src="{{file1}}.jpg" width="50" height="50" layout="fill" alt="Alt text" object-fit="cover">' . $amp_carousel_caption . '</span></amp-carousel>',
-			],
-			'shortcode_with_multiple_ids'             => [
+			'shortcode_with_valid_ids_in_legacy_mode' => [
 				'[gallery ids={{id1}},{{id2}},{{id3}}]',
-				'<amp-carousel width="600" height="450" type="slides" layout="responsive">' .
-					'<span class="slide"><img src="{{file1}}.jpg" width="50" height="50" layout="fill" alt="Alt text" object-fit="cover">' . $amp_carousel_caption . '</span>' .
-					'<span class="slide"><img src="{{file2}}.jpg" width="600" height="450" layout="fill" alt="Alt text" srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" object-fit="cover"></span>' .
-					'<span class="slide"><img src="{{file3}}.jpg" width="100" height="100" layout="fill" alt="Alt text" object-fit="cover"></span>' .
-				'</amp-carousel>',
+				'<style type="text/css"> #gallery-3 { margin: auto; } #gallery-3 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-3 img { border: 2px solid #cfcfcf; } #gallery-3 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
+				<amp-carousel width="640" height="480" type="slides" layout="responsive">
+					<figure class="slide"><a href="{{file_url1}}"><img width="50" height="50" src="{{file1}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-3-{{id1}}" layout="fill" object-fit="cover"></a>' . $amp_carousel_caption . '</figure>
+					<figure class="slide"><a href="{{file_url2}}"><img width="640" height="480" src="{{file2}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" sizes="(max-width: 640px) 100vw, 640px" layout="fill" object-fit="cover"></a></figure>
+					<figure class="slide"><a href="{{file_url3}}"><img width="100" height="100" src="{{file3}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' layout="fill" object-fit="cover"></a></figure>
+				</amp-carousel>',
+				true,
 			],
-			'shortcode_linking_to_file'               => [
-				'[gallery link="file" ids={{id1}},{{id2}},{{id3}}]',
-				'<amp-carousel width="600" height="450" type="slides" layout="responsive">' .
-					'<span class="slide"><a href="{{file1}}.jpg"><img src="{{file1}}.jpg" width="50" height="50" layout="fill" alt="Alt text" object-fit="cover"></a>' . $amp_carousel_caption . '</span>' .
-					'<span class="slide"><a href="{{file2}}.jpg"><img src="{{file2}}.jpg" width="600" height="450" layout="fill" alt="Alt text" srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" object-fit="cover"></a></span>' .
-					'<span class="slide"><a href="{{file3}}.jpg"><img src="{{file3}}.jpg" width="100" height="100" layout="fill" alt="Alt text" object-fit="cover"></a></span>' .
-				'</amp-carousel>',
+			'shortcode_with_valid_ids'                => [
+				'[gallery ids={{id1}},{{id2}},{{id3}}]',
+				'<style type="text/css"> #gallery-4 { margin: auto; } #gallery-4 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-4 img { border: 2px solid #cfcfcf; } #gallery-4 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
+				<div id="gallery-4" class="gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail">
+					<dl class="gallery-item">
+						<dt class="gallery-icon landscape"><a href="{{file_url1}}"><img width="50" height="50" src="{{file1}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-4-{{id1}}"></a></dt>
+						<dd class="wp-caption-text gallery-caption" id="gallery-4-{{id1}}"> ' . self::CAPTION_TEXT . ' </dd>
+					</dl>
+					<dl class="gallery-item">
+						<dt class="gallery-icon landscape"><a href="{{file_url2}}"><img width="150" height="150" src="{{file2}}-150x150.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . '></a></dt>
+					</dl>
+					<dl class="gallery-item">
+						<dt class="gallery-icon landscape"><a href="{{file_url3}}"><img width="100" height="100" src="{{file3}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . '></a></dt>
+					</dl>
+					<br style="clear: both">
+				</div>',
 			],
 			'shortcode_with_carousel'                 => [
 				'[gallery amp-lightbox=false amp-carousel=true ids={{id1}},{{id2}},{{id3}}]',
-				'<amp-carousel width="600" height="450" type="slides" layout="responsive">' .
-					'<span class="slide"><img src="{{file1}}.jpg" width="50" height="50" layout="fill" alt="Alt text" object-fit="cover">' . $amp_carousel_caption . '</span>' .
-					'<span class="slide"><img src="{{file2}}.jpg" width="600" height="450" layout="fill" alt="Alt text" srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" object-fit="cover"></span>' .
-					'<span class="slide"><img src="{{file3}}.jpg" width="100" height="100" layout="fill" alt="Alt text" object-fit="cover"></span>' .
-				'</amp-carousel>',
+				'<style type="text/css"> #gallery-6 { margin: auto; } #gallery-6 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-6 img { border: 2px solid #cfcfcf; } #gallery-6 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
+				<amp-carousel width="640" height="480" type="slides" layout="responsive">
+					<figure class="slide"><a href="{{file_url1}}"><img width="50" height="50" src="{{file1}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-6-{{id1}}" layout="fill" object-fit="cover"></a>' . $amp_carousel_caption . '</figure>
+					<figure class="slide"><a href="{{file_url2}}"><img width="640" height="480" src="{{file2}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" sizes="(max-width: 640px) 100vw, 640px" layout="fill" object-fit="cover"></a></figure>
+					<figure class="slide"><a href="{{file_url3}}"><img width="100" height="100" src="{{file3}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' layout="fill" object-fit="cover"></a></figure>
+				</amp-carousel>',
 			],
 			'shortcode_with_carousel_linking_to_file' => [
 				'[gallery amp-lightbox=false amp-carousel=true link="file" ids={{id1}},{{id2}},{{id3}}]',
-				'<amp-carousel width="600" height="450" type="slides" layout="responsive">' .
-					'<span class="slide"><a href="{{file1}}.jpg"><img src="{{file1}}.jpg" width="50" height="50" layout="fill" alt="Alt text" object-fit="cover"></a>' . $amp_carousel_caption . '</span>' .
-					'<span class="slide"><a href="{{file2}}.jpg"><img src="{{file2}}.jpg" width="600" height="450" layout="fill" alt="Alt text" srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" object-fit="cover"></a></span>' .
-					'<span class="slide"><a href="{{file3}}.jpg"><img src="{{file3}}.jpg" width="100" height="100" layout="fill" alt="Alt text" object-fit="cover"></a></span>' .
-				'</amp-carousel>',
+				'<style type="text/css"> #gallery-8 { margin: auto; } #gallery-8 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-8 img { border: 2px solid #cfcfcf; } #gallery-8 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
+				<amp-carousel width="640" height="480" type="slides" layout="responsive">
+					<figure class="slide"><a href="{{file1}}.jpg"><img width="50" height="50" src="{{file1}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-8-{{id1}}" layout="fill" object-fit="cover"></a>' . $amp_carousel_caption . '</figure>
+					<figure class="slide"><a href="{{file2}}.jpg"><img width="640" height="480" src="{{file2}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" sizes="(max-width: 640px) 100vw, 640px" layout="fill" object-fit="cover"></a></figure>
+					<figure class="slide"><a href="{{file3}}.jpg"><img width="100" height="100" src="{{file3}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' layout="fill" object-fit="cover"></a></figure>
+				</amp-carousel>',
 			],
 			'shortcode_with_lightbox'                 => [
 				'[gallery amp-lightbox=true amp-carousel=false ids={{id1}},{{id2}},{{id3}}]',
-				'<style type=\'text/css\'> #gallery-8 { margin: auto; } #gallery-8 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-8 img { border: 2px solid #cfcfcf; } #gallery-8 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
-				<div id=\'gallery-8\' class=\'gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail\'>
-					<dl class=\'gallery-item\'><dt class=\'gallery-icon landscape\'>
-						<img width="50" height="50" src="{{file1}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-8-{{id1}}" lightbox="" />
-					</dt><dd class=\'wp-caption-text gallery-caption\' id=\'gallery-8-{{id1}}\'> ' . self::CAPTION_TEXT . ' </dd></dl>
-					<dl class=\'gallery-item\'><dt class=\'gallery-icon landscape\'>
-						<img width="150" height="150" src="{{file2}}-150x150.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' lightbox="" />
+				'<style type="text/css"> #gallery-10 { margin: auto; } #gallery-10 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-10 img { border: 2px solid #cfcfcf; } #gallery-10 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
+				<div id="gallery-10" class="gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail">
+					<dl class="gallery-item"><dt class="gallery-icon landscape">
+						<img width="50" height="50" src="{{file1}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-10-{{id1}}" lightbox="">
+					</dt><dd class="wp-caption-text gallery-caption" id="gallery-10-{{id1}}"> ' . self::CAPTION_TEXT . ' </dd></dl>
+					<dl class="gallery-item"><dt class="gallery-icon landscape">
+						<img width="150" height="150" src="{{file2}}-150x150.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' lightbox="">
 					</dt></dl>
-					<dl class=\'gallery-item\'><dt class=\'gallery-icon landscape\'>
-						<img width="100" height="100" src="{{file3}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' lightbox="" />
+					<dl class="gallery-item"><dt class="gallery-icon landscape">
+						<img width="100" height="100" src="{{file3}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' lightbox="">
 					</dt></dl>
-					<br style="clear: both" />
+					<br style="clear: both">
 				</div>',
 			],
 			'shortcode_with_lightbox_linking_to_file' => [
 				'[gallery amp-lightbox=true amp-carousel=false link="file" ids={{id1}},{{id2}},{{id3}}]',
-				'<style type=\'text/css\'> #gallery-10 { margin: auto; } #gallery-10 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-10 img { border: 2px solid #cfcfcf; } #gallery-10 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
-				<div id=\'gallery-10\' class=\'gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail\'>
-					<dl class=\'gallery-item\'><dt class=\'gallery-icon landscape\'>
-						<img width="50" height="50" src="{{file1}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-10-{{id1}}" lightbox="" />
-					</dt><dd class=\'wp-caption-text gallery-caption\' id=\'gallery-10-{{id1}}\'> ' . self::CAPTION_TEXT . ' </dd></dl>
-					<dl class=\'gallery-item\'><dt class=\'gallery-icon landscape\'>
-						<img width="150" height="150" src="{{file2}}-150x150.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' lightbox="" />
+				'<style type="text/css"> #gallery-12 { margin: auto; } #gallery-12 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-12 img { border: 2px solid #cfcfcf; } #gallery-12 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
+				<div id="gallery-12" class="gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail">
+					<dl class="gallery-item"><dt class="gallery-icon landscape">
+						<img width="50" height="50" src="{{file1}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-12-{{id1}}" lightbox="">
+					</dt><dd class="wp-caption-text gallery-caption" id="gallery-12-{{id1}}"> ' . self::CAPTION_TEXT . ' </dd></dl>
+					<dl class="gallery-item"><dt class="gallery-icon landscape">
+						<img width="150" height="150" src="{{file2}}-150x150.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' lightbox="">
 					</dt></dl>
-					<dl class=\'gallery-item\'><dt class=\'gallery-icon landscape\'>
-						<img width="100" height="100" src="{{file3}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' lightbox="" />
+					<dl class="gallery-item"><dt class="gallery-icon landscape">
+						<img width="100" height="100" src="{{file3}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' lightbox="">
 					</dt></dl>
-					<br style="clear: both" />
+					<br style="clear: both">
 				</div>',
 			],
 			'shortcode_with_lightbox_and_carousel'    => [
 				'[gallery amp-lightbox=true amp-carousel=true ids={{id1}},{{id2}},{{id3}}]',
-				'<amp-carousel width="600" height="450" type="slides" layout="responsive">' .
-					'<span class="slide"><img src="{{file1}}.jpg" width="50" height="50" layout="fill" alt="Alt text" lightbox="" object-fit="cover">' . $amp_carousel_caption . '</span>' .
-					'<span class="slide"><img src="{{file2}}.jpg" width="600" height="450" layout="fill" alt="Alt text" srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" lightbox="" object-fit="cover"></span>' .
-					'<span class="slide"><img src="{{file3}}.jpg" width="100" height="100" layout="fill" alt="Alt text" lightbox="" object-fit="cover"></span>' .
+				'<style type="text/css"> #gallery-14 { margin: auto; } #gallery-14 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-14 img { border: 2px solid #cfcfcf; } #gallery-14 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
+				<amp-carousel width="640" height="480" type="slides" layout="responsive" lightbox="">' .
+					'<figure class="slide"><img width="50" height="50" src="{{file1}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-14-{{id1}}" layout="fill" object-fit="cover">' . $amp_carousel_caption . '</figure>' .
+					'<figure class="slide"><img width="640" height="480" src="{{file2}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" sizes="(max-width: 640px) 100vw, 640px" layout="fill" object-fit="cover"></figure>' .
+					'<figure class="slide"><img width="100" height="100" src="{{file3}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' layout="fill" object-fit="cover"></figure>' .
 				'</amp-carousel>',
 			],
 			'shortcode_with_lightbox_and_carousel_linking_to_file' => [
 				'[gallery amp-lightbox=true amp-carousel=true link="file" ids={{id1}},{{id2}},{{id3}}]',
-				'<amp-carousel width="600" height="450" type="slides" layout="responsive">' .
-					'<span class="slide"><img src="{{file1}}.jpg" width="50" height="50" layout="fill" alt="Alt text" lightbox="" object-fit="cover">' . $amp_carousel_caption . '</span>' .
-					'<span class="slide"><img src="{{file2}}.jpg" width="600" height="450" layout="fill" alt="Alt text" srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" lightbox="" object-fit="cover"></span>' .
-					'<span class="slide"><img src="{{file3}}.jpg" width="100" height="100" layout="fill" alt="Alt text" lightbox="" object-fit="cover"></span>' .
+				'<style type="text/css"> #gallery-16 { margin: auto; } #gallery-16 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-16 img { border: 2px solid #cfcfcf; } #gallery-16 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
+				<amp-carousel width="640" height="480" type="slides" layout="responsive" lightbox="">' .
+					'<figure class="slide"><img width="50" height="50" src="{{file1}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-16-{{id1}}" layout="fill" object-fit="cover">' . $amp_carousel_caption . '</figure>' .
+					'<figure class="slide"><img width="640" height="480" src="{{file2}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' srcset="{{file2}}.jpg 640w, {{file2}}-300x225.jpg 300w" sizes="(max-width: 640px) 100vw, 640px" layout="fill" object-fit="cover"></figure>' .
+					'<figure class="slide"><img width="100" height="100" src="{{file3}}.jpg" class="attachment-large size-large" alt="Alt text" ' . $loading_attribute . ' layout="fill" object-fit="cover"></figure>' .
 				'</amp-carousel>',
 			],
 		];
@@ -145,22 +168,31 @@ public function get_conversion_data() {
 	 * @dataProvider get_conversion_data
 	 * @param string $source   Source.
 	 * @param string $expected Expected.
+	 * @param bool   $use_legacy_mode Whether to use legacy Reader mode.
 	 */
-	public function test__conversion( $source, $expected ) {
+	public function test__conversion( $source, $expected, $use_legacy_mode = false ) {
 		$source_files = [
 			DIR_TESTDATA . '/images/test-image.jpg',
 			DIR_TESTDATA . '/images/canola.jpg',
 			DIR_TESTDATA . '/images/gradient-square.jpg',
 		];
 
+		if ( $use_legacy_mode ) {
+			AMP_Options_Manager::update_option( Option::READER_THEME, ReaderThemes::DEFAULT_READER_THEME );
+			AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG );
+		} else {
+			AMP_Options_Manager::update_option( Option::READER_THEME, 'foobar' );
+		}
+
 		// When we generate new attachments, we neither control the IDs being
 		// generated, which we need for the gallery shortcode, nor the actual
 		// filenames, which will have an index appended because of filename
 		// collisions. So we write the tests in a way that they don't rely on
 		// this, and str-replace the dynamic portions into the expected and
 		// actual output after the fact.
-		$ids   = [];
-		$files = [];
+		$ids       = [];
+		$files     = [];
+		$file_urls = [];
 
 		foreach ( $source_files as $index => $source_file ) {
 			$ids[ $index ] = self::factory()->attachment
@@ -171,6 +203,8 @@ public function test__conversion( $source, $expected ) {
 				'.jpg'
 			);
 
+			$file_urls[ $index ] = get_attachment_link( $ids[ $index ] );
+
 			update_post_meta(
 				$ids[ $index ],
 				'_wp_attachment_image_alt',
@@ -188,7 +222,7 @@ public function test__conversion( $source, $expected ) {
 			}
 		}
 
-		$this->initialize_replacements( $ids, $files );
+		$this->initialize_replacements( $ids, $files, $file_urls );
 
 		$embed = new AMP_Gallery_Embed_Handler();
 		$embed->register_embed();
@@ -198,28 +232,37 @@ public function test__conversion( $source, $expected ) {
 			$this->normalize( $source )
 		);
 
+		$dom = AMP_DOM_Utils::get_dom_from_content( $filtered_content );
+		$embed->sanitize_raw_embeds( $dom );
+
+		$content = AMP_DOM_Utils::get_content_from_dom( $dom );
+
 		$this->assertEquals(
 			$this->normalize( $expected ),
-			$this->normalize( $filtered_content )
+			$this->normalize( $content )
 		);
 	}
 
 	/**
 	 * Initialize the associative array of replacements to perform.
 	 *
-	 * @param array<int>    $ids   Array of attachment post IDs.
-	 * @param array<string> $files Array of file URLs.
+	 * @param array<int>    $ids       Array of attachment post IDs.
+	 * @param array<string> $files     Array of file URLs.
+	 * @param array<string> $file_urls Array of file permalink URLs.
 	 */
-	private function initialize_replacements( $ids, $files ) {
+	private function initialize_replacements( $ids, $files, $file_urls ) {
 		$this->replacements = [
-			'{{id1}}'   => $ids[0],
-			'{{id2}}'   => $ids[1],
-			'{{id3}}'   => $ids[2],
-			'{{file1}}' => $files[0],
-			'{{file2}}' => $files[1],
-			'{{file3}}' => $files[2],
-			"\n"        => '', // Make tests ignore new lines.
-			'> <'       => '><', // Remove left-over space between elements.
+			'{{id1}}'       => $ids[0],
+			'{{id2}}'       => $ids[1],
+			'{{id3}}'       => $ids[2],
+			'{{file1}}'     => $files[0],
+			'{{file2}}'     => $files[1],
+			'{{file3}}'     => $files[2],
+			'{{file_url1}}' => $file_urls[0],
+			'{{file_url2}}' => $file_urls[1],
+			'{{file_url3}}' => $file_urls[2],
+			"\n"            => '', // Make tests ignore new lines.
+			'> <'           => '><', // Remove left-over space between elements.
 		];
 	}
 
@@ -236,7 +279,7 @@ private function normalize( $content ) {
 		$content = trim( preg_replace( '/\s+/', ' ', $content ) );
 
 		// Normalize attribute quote style for 5.3-alpha.
-		$content = str_replace( '<style type=\'text/css\'>', '<style type="text/css">', $content );
+		$content = str_replace( '<style type="text/css">', '<style type="text/css">', $content );
 
 		// Then we go through all previously defined replacements.
 		return str_replace(
diff --git a/tests/php/test-class-amp-gallery-block-sanitizer.php b/tests/php/test-class-amp-gallery-block-sanitizer.php
index b75c25f5e05..038db5fae33 100644
--- a/tests/php/test-class-amp-gallery-block-sanitizer.php
+++ b/tests/php/test-class-amp-gallery-block-sanitizer.php
@@ -5,11 +5,15 @@
  * @package AMP
  */
 
+use AmpProject\AmpWP\Tests\Helpers\PrivateAccess;
+
 /**
  * Class AMP_Gallery_Block_Sanitizer_Test
  */
 class AMP_Gallery_Block_Sanitizer_Test extends WP_UnitTestCase {
 
+	use PrivateAccess;
+
 	/**
 	 * Get data.
 	 *
@@ -39,18 +43,18 @@ public function get_data() {
 
 			'data_amp_with_carousel_and_link'     => [
 				'<ul class="wp-block-gallery" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
-				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><span class="slide"><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></a></span></amp-carousel>',
+				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><figure class="slide"><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></a></figure></amp-carousel>',
 			],
 
 			'data_amp_with_carousel_and_caption'  => [
 				'<ul class="wp-block-gallery" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img><figcaption>This is a caption</figcaption></figure></li></ul>',
-				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><span class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img><span class="amp-wp-gallery-caption"><span>This is a caption</span></span></span></amp-carousel>',
+				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption">This is a caption</figcaption></figure></amp-carousel>',
 			],
 
 			// WordPress 5.3 changed the markup for the Gallery block, wrapping it in a <figure>.
 			'data_amp_with_carousel_caption_5_3'  => [
 				'<figure class="wp-block-gallery" data-amp-carousel="true"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img><figcaption>This is a caption</figcaption></figure></li></ul></figure>',
-				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><span class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img><span class="amp-wp-gallery-caption"><span>This is a caption</span></span></span></amp-carousel>',
+				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption">This is a caption</figcaption></figure></amp-carousel>',
 			],
 
 			'data_amp_with_lightbox'              => [
@@ -75,12 +79,12 @@ public function get_data() {
 
 			'data_amp_with_lightbox_and_carousel' => [
 				'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="1234" height="567"></amp-img></a></figure></li></ul>',
-				'<amp-carousel width="1234" height="567" type="slides" layout="responsive"><span class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" lightbox="" layout="fill" object-fit="cover"></amp-img></span></amp-carousel>',
+				'<amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
 			],
 
 			'data_amp_with_lightbox_carousel_5_3' => [
 				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="1234" height="567"></amp-img></a></figure></li></ul></figure>',
-				'<amp-carousel width="1234" height="567" type="slides" layout="responsive"><span class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" lightbox="" layout="fill" object-fit="cover"></amp-img></span></amp-carousel>',
+				'<amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
 			],
 		];
 	}
@@ -124,22 +128,22 @@ public function get_reader_mode_data() {
 
 			'data_amp_with_carousel_and_caption'  => [
 				'<ul class="wp-block-gallery" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a><figcaption>Here is a caption</figcaption></figure></li></ul>',
-				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><span class="slide"><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></a><span class="amp-wp-gallery-caption"><span>Here is a caption</span></span></span></amp-carousel>',
+				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><figure class="slide"><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></a><figcaption class="amp-wp-gallery-caption">Here is a caption</figcaption></figure></amp-carousel>',
 			],
 
 			'data_amp_with_lightbox'              => [
 				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></figure></li></ul>',
-				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><span class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox="" layout="fill" object-fit="cover"></amp-img></span></amp-carousel>',
+				'<amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
 			],
 
 			'data_amp_with_lightbox_and_link'     => [
 				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
-				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><span class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox="" layout="fill" object-fit="cover"></amp-img></span></amp-carousel>',
+				'<amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
 			],
 
 			'data_amp_lightbox_carousel_and_link' => [
 				'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
-				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><span class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox="" layout="fill" object-fit="cover"></amp-img></span></amp-carousel>',
+				'<amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
 			],
 		];
 	}
@@ -176,27 +180,32 @@ public function get_caption_text_data() {
 		return [
 			'no_amp_img_or_anchor'       => [
 				'<div><img src="https://example.com/image.jpg"></div>',
-				'',
+				null,
 				'img',
 			],
 			'amp_img_with_empty_caption' => [
 				'<amp-img src="https://example.com/image.jpg"></amp-img><figcaption></figcaption>',
-				'',
+				null,
 				'amp-img',
 			],
 			'amp_img_with_caption'       => [
 				'<amp-img src="https://example.com/image.jpg"></amp-img><figcaption>This is a caption</figcaption>',
-				'This is a caption',
+				'<figcaption>This is a caption</figcaption>',
+				'amp-img',
+			],
+			'amp_img_with_html_caption'  => [
+				'<amp-img src="https://example.com/image.jpg"></amp-img><figcaption><span class="foobar">This is a caption</span></figcaption>',
+				'<figcaption><span class="foobar">This is a caption</span></figcaption>',
 				'amp-img',
 			],
 			'amp_img_wrapped_in_anchor_with_caption_in_div' => [
 				'<a href="https://example.com"><amp-img src="https://example.com/image.jpg"></amp-img></a><div>This is a caption</div>',
-				'',
+				null,
 				'a',
 			],
 			'amp_img_wrapped_in_anchor_with_caption_in_figcaption' => [
 				'<a href="https://example.com"><amp-img src="https://example.com/image.jpg"></amp-img></a><figcaption>This is a caption</figcaption>',
-				'This is a caption',
+				'<figcaption>This is a caption</figcaption>',
 				'a',
 			],
 		];
@@ -220,6 +229,12 @@ public function test_possibly_get_caption_text( $source, $expected, $element_nam
 			[ 'content_max_width' => 600 ]
 		);
 
-		$this->assertEquals( $expected, $sanitizer->get_caption_element( $element ) );
+		$actual = $this->call_private_method( $sanitizer, 'get_caption_element', [ $element ] );
+
+		if ( $actual instanceof DOMElement ) {
+			$actual = $dom->saveHTML( $actual );
+		}
+
+		$this->assertEquals( $expected, $actual );
 	}
 }
diff --git a/tests/php/test-dom-element-list.php b/tests/php/test-dom-element-list.php
index 495027307ad..ecb08260374 100644
--- a/tests/php/test-dom-element-list.php
+++ b/tests/php/test-dom-element-list.php
@@ -58,7 +58,7 @@ public function get_dom_element_list_data() {
 	public function test_dom_element_list_add( $images, $expected_count ) {
 		$dom_element_list = new ElementList();
 		foreach ( $images as $image ) {
-			$dom_element_list = $dom_element_list->add( $image, '' );
+			$dom_element_list = $dom_element_list->add( $image, null );
 		}
 
 		$this->assertEquals( $expected_count, $dom_element_list->count() );
@@ -77,7 +77,7 @@ public function test_dom_element_list_add( $images, $expected_count ) {
 	public function test_dom_element_list_get_iterator( $images, $expected_count ) {
 		$dom_element_list = new ElementList();
 		foreach ( $images as $image ) {
-			$dom_element_list = $dom_element_list->add( $image, '' );
+			$dom_element_list = $dom_element_list->add( $image, null );
 		}
 
 		$iteration_count = 0;
@@ -108,7 +108,7 @@ public function test_get_caption() {
 	 */
 	public function test_get_slide_node() {
 		$image_node = AMP_DOM_Utils::create_node( new Document(), 'amp-img', [] );
-		$amp_image  = new CaptionedSlide( $image_node, '' );
+		$amp_image  = new CaptionedSlide( $image_node, new DOMNode() );
 		$this->assertEquals( $image_node, $amp_image->get_slide_node() );
 	}
 }

From 8e261daf9bb9c5ba778b2629d37a2b2690415f3a Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Mon, 20 Jul 2020 02:13:34 -0400
Subject: [PATCH 06/47] Fix lint errors

---
 src/Dom/ElementList.php           |  2 +-
 src/Embed/HandlesGalleryEmbed.php | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/Dom/ElementList.php b/src/Dom/ElementList.php
index 93630121194..8b7cbfdedf4 100644
--- a/src/Dom/ElementList.php
+++ b/src/Dom/ElementList.php
@@ -38,7 +38,7 @@ final class ElementList implements IteratorAggregate, Countable {
 	 */
 	public function add( DOMElement $element, DOMNode $caption = null ) {
 		$cloned_list             = clone $this;
-		$cloned_list->elements[] = $caption === null ? $element : new CaptionedSlide( $element, $caption );
+		$cloned_list->elements[] = null === $caption ? $element : new CaptionedSlide( $element, $caption );
 		return $cloned_list;
 	}
 
diff --git a/src/Embed/HandlesGalleryEmbed.php b/src/Embed/HandlesGalleryEmbed.php
index b29289f423b..f7bea39d430 100644
--- a/src/Embed/HandlesGalleryEmbed.php
+++ b/src/Embed/HandlesGalleryEmbed.php
@@ -26,10 +26,10 @@ trait HandlesGalleryEmbed {
 	/**
 	 * Transforms the raw embed gallery embed to become AMP compatible.
 	 *
-	 * @param bool        $is_carousel Whether the embed should be transformed into an <amp-carousel>
-	 * @param bool        $is_lightbox Whether the gallery images should be shown in a lightbox.
+	 * @param bool        $is_carousel     Whether the embed should be transformed into an <amp-carousel>.
+	 * @param bool        $is_lightbox     Whether the gallery images should be shown in a lightbox.
 	 * @param DOMElement  $gallery_element Gallery element.
-	 * @param DOMNodeList $img_elements
+	 * @param DOMNodeList $img_elements    List of image elements in gallery.
 	 */
 	protected function process_gallery_embed( $is_carousel, $is_lightbox, DOMElement $gallery_element, DOMNodeList $img_elements ) {
 		// Bail if the embed does not support carousel or lightbox.
@@ -71,10 +71,10 @@ protected function generate_amp_carousel( DOMNodeList $img_elements, $is_amp_lig
 		$images = new ElementList();
 
 		foreach ( $img_elements as $img_element ) {
-			$element           = $img_element;
-			$parentElementName = $img_element->parentNode->nodeName;
+			$element             = $img_element;
+			$parent_element_name = $img_element->parentNode->nodeName;
 
-			if ( 'a' === $parentElementName && ! $is_amp_lightbox ) {
+			if ( 'a' === $parent_element_name && ! $is_amp_lightbox ) {
 				$element = $img_element->parentNode;
 			}
 

From fd3e9f2c4173c66cc07b100a18a08b768c5537cf Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Mon, 20 Jul 2020 02:43:26 -0400
Subject: [PATCH 07/47] Fix test errors

---
 src/Component/Carousel.php                   | 3 ++-
 tests/php/test-amp-gallery-embed-handler.php | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/Component/Carousel.php b/src/Component/Carousel.php
index afcf5a09eda..cd4e1e8ae9b 100644
--- a/src/Component/Carousel.php
+++ b/src/Component/Carousel.php
@@ -12,6 +12,7 @@
 use AmpProject\AmpWP\Dom\ElementList;
 use DOMElement;
 use AMP_DOM_Utils;
+use DOMNode;
 
 /**
  * Class Carousel
@@ -102,7 +103,7 @@ public function get_dom_element() {
 			$slide_container->appendChild( $slide_node );
 
 			// If there's a caption, append it to the slide.
-			if ( $caption_node instanceof DOMElement ) {
+			if ( $caption_node instanceof DOMNode ) {
 				// If the caption is not a <figcaption>, wrap it in one.
 				if ( 'figcaption' !== $caption_node->nodeName ) {
 					$caption_content = $caption_node;
diff --git a/tests/php/test-amp-gallery-embed-handler.php b/tests/php/test-amp-gallery-embed-handler.php
index 6e83acd7ac0..9fdce6f912b 100644
--- a/tests/php/test-amp-gallery-embed-handler.php
+++ b/tests/php/test-amp-gallery-embed-handler.php
@@ -83,10 +83,10 @@ public function get_conversion_data() {
 						<dd class="wp-caption-text gallery-caption" id="gallery-4-{{id1}}"> ' . self::CAPTION_TEXT . ' </dd>
 					</dl>
 					<dl class="gallery-item">
-						<dt class="gallery-icon landscape"><a href="{{file_url2}}"><img width="150" height="150" src="{{file2}}-150x150.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . '></a></dt>
+						<dt class="gallery-icon landscape"><a href="{{file_url2}}"><img width="150" height="150" src="{{file2}}-150x150.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text"' . ( $loading_attribute ? ( ' ' . $loading_attribute ) : '' ) . '></a></dt>
 					</dl>
 					<dl class="gallery-item">
-						<dt class="gallery-icon landscape"><a href="{{file_url3}}"><img width="100" height="100" src="{{file3}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . '></a></dt>
+						<dt class="gallery-icon landscape"><a href="{{file_url3}}"><img width="100" height="100" src="{{file3}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text"' . ( $loading_attribute ? ( ' ' . $loading_attribute ) : '' ) . '></a></dt>
 					</dl>
 					<br style="clear: both">
 				</div>',

From 03df2b207521840ef0d683acb3aeea503ca89ee6 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Mon, 20 Jul 2020 02:47:03 -0400
Subject: [PATCH 08/47] Indicate `$caption_node` is an instance of DOMElement

---
 src/Component/Carousel.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Component/Carousel.php b/src/Component/Carousel.php
index cd4e1e8ae9b..add966f230a 100644
--- a/src/Component/Carousel.php
+++ b/src/Component/Carousel.php
@@ -115,6 +115,7 @@ public function get_dom_element() {
 				$caption_block_class     = 'wp-block-image';
 				$has_caption_class       = AMP_DOM_Utils::has_class( $caption_node, $caption_shortcode_class ) || AMP_DOM_Utils::has_class( $caption_node, $caption_block_class );
 
+				/** @var DOMElement $caption_node */
 				if ( ! $has_caption_class || ! $caption_node->hasAttribute( Attribute::CLASS_ ) ) {
 					$caption_node->setAttribute( Attribute::CLASS_, 'amp-wp-gallery-caption' );
 				}

From 9467dccb56311d2c32b8d2fe3dd0175fd549ba21 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Mon, 20 Jul 2020 21:38:46 -0400
Subject: [PATCH 09/47] Use constants for tag names

---
 lib/common/src/Tag.php            | 58 ++++++++++++++++---------------
 src/Component/Carousel.php        |  7 ++--
 src/Embed/HandlesGalleryEmbed.php |  7 ++--
 3 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/lib/common/src/Tag.php b/lib/common/src/Tag.php
index ecd55f3c246..26e8a6ed71c 100644
--- a/lib/common/src/Tag.php
+++ b/lib/common/src/Tag.php
@@ -10,34 +10,36 @@
 interface Tag
 {
 
-    const A        = 'a';
-    const AREA     = 'area';
-    const BASE     = 'base';
-    const BASEFONT = 'basefont';
-    const BGSOUND  = 'bgsound';
-    const BODY     = 'body';
-    const BR       = 'br';
-    const COL      = 'col';
-    const EMBED    = 'embed';
-    const FORM     = 'form';
-    const FRAME    = 'frame';
-    const HEAD     = 'head';
-    const HR       = 'hr';
-    const HTML     = 'html';
-    const IMG      = 'img';
-    const INPUT    = 'input';
-    const KEYGEN   = 'keygen';
-    const LINK     = 'link';
-    const META     = 'meta';
-    const NOSCRIPT = 'noscript';
-    const PARAM    = 'param';
-    const SCRIPT   = 'script';
-    const SOURCE   = 'source';
-    const STYLE    = 'style';
-    const TEMPLATE = 'template';
-    const TITLE    = 'title';
-    const TRACK    = 'track';
-    const WBR      = 'wbr';
+    const A          = 'a';
+    const AREA       = 'area';
+    const BASE       = 'base';
+    const BASEFONT   = 'basefont';
+    const BGSOUND    = 'bgsound';
+    const BODY       = 'body';
+    const BR         = 'br';
+    const COL        = 'col';
+    const EMBED      = 'embed';
+    const FIGCAPTION = 'figcaption';
+    const FIGURE     = 'figure';
+    const FORM       = 'form';
+    const FRAME      = 'frame';
+    const HEAD       = 'head';
+    const HR         = 'hr';
+    const HTML       = 'html';
+    const IMG        = 'img';
+    const INPUT      = 'input';
+    const KEYGEN     = 'keygen';
+    const LINK       = 'link';
+    const META       = 'meta';
+    const NOSCRIPT   = 'noscript';
+    const PARAM      = 'param';
+    const SCRIPT     = 'script';
+    const SOURCE     = 'source';
+    const STYLE      = 'style';
+    const TEMPLATE   = 'template';
+    const TITLE      = 'title';
+    const TRACK      = 'track';
+    const WBR        = 'wbr';
 
     /**
      * HTML elements that are self-closing.
diff --git a/src/Component/Carousel.php b/src/Component/Carousel.php
index add966f230a..65e45bbd78a 100644
--- a/src/Component/Carousel.php
+++ b/src/Component/Carousel.php
@@ -10,6 +10,7 @@
 use AmpProject\Attribute;
 use AmpProject\Dom\Document;
 use AmpProject\AmpWP\Dom\ElementList;
+use AmpProject\Tag;
 use DOMElement;
 use AMP_DOM_Utils;
 use DOMNode;
@@ -86,7 +87,7 @@ public function get_dom_element() {
 			$caption_node    = $slide instanceof HasCaption ? $slide->get_caption_node() : null;
 			$slide_container = AMP_DOM_Utils::create_node(
 				$this->dom,
-				'figure', // This cannot be a <div> because if the gallery is inside of a <p>, then the DOM will break.
+				Tag::FIGURE, // This cannot be a <div> because if the gallery is inside of a <p>, then the DOM will break.
 				[ 'class' => 'slide' ]
 			);
 
@@ -105,9 +106,9 @@ public function get_dom_element() {
 			// If there's a caption, append it to the slide.
 			if ( $caption_node instanceof DOMNode ) {
 				// If the caption is not a <figcaption>, wrap it in one.
-				if ( 'figcaption' !== $caption_node->nodeName ) {
+				if ( Tag::FIGCAPTION !== $caption_node->nodeName ) {
 					$caption_content = $caption_node;
-					$caption_node    = AMP_DOM_Utils::create_node( $this->dom, 'figcaption', [] );
+					$caption_node    = AMP_DOM_Utils::create_node( $this->dom, Tag::FIGCAPTION, [] );
 					$caption_node->appendChild( $caption_content );
 				}
 
diff --git a/src/Embed/HandlesGalleryEmbed.php b/src/Embed/HandlesGalleryEmbed.php
index f7bea39d430..b8c3e00b12a 100644
--- a/src/Embed/HandlesGalleryEmbed.php
+++ b/src/Embed/HandlesGalleryEmbed.php
@@ -11,11 +11,12 @@
 use AmpProject\AmpWP\Dom\ElementList;
 use AmpProject\Attribute;
 use AmpProject\Dom\Document;
+use AmpProject\Tag;
 use DOMElement;
 use DOMNodeList;
 
 /**
- * Trait HandlesGalleryEmbed
+ * Trait HandlesGalleryEmbed.
  *
  * Contains logic related to both gallery embed shortcodes and blocks.
  *
@@ -24,7 +25,7 @@
 trait HandlesGalleryEmbed {
 
 	/**
-	 * Transforms the raw embed gallery embed to become AMP compatible.
+	 * Transforms the raw gallery embed to become AMP compatible.
 	 *
 	 * @param bool        $is_carousel     Whether the embed should be transformed into an <amp-carousel>.
 	 * @param bool        $is_lightbox     Whether the gallery images should be shown in a lightbox.
@@ -74,7 +75,7 @@ protected function generate_amp_carousel( DOMNodeList $img_elements, $is_amp_lig
 			$element             = $img_element;
 			$parent_element_name = $img_element->parentNode->nodeName;
 
-			if ( 'a' === $parent_element_name && ! $is_amp_lightbox ) {
+			if ( Tag::A === $parent_element_name && ! $is_amp_lightbox ) {
 				$element = $img_element->parentNode;
 			}
 

From bab1fc9edbc751938dd9f804c62b60ab950d04d9 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 21 Jul 2020 16:35:19 -0700
Subject: [PATCH 10/47] Prevent gallery_style filter from applying to other
 shortcodes

---
 .../class-amp-gallery-embed-handler.php       | 28 +++++++++----------
 src/Embed/HandlesGalleryEmbed.php             |  2 +-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index aae9efcdc49..e68e20f62c8 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -33,7 +33,7 @@ public function register_embed() {
 	 * @return string Markup for the gallery.
 	 */
 	public function generate_gallery_markup( $html, $attrs ) {
-		// Use <amp-carousel> for the gallery if requested via amp-carousel shortcode attribute, or use by default if in Reader mode.
+		// Use <amp-carousel> for the gallery if requested via amp-carousel shortcode attribute, or use by default if in legacy Reader mode.
 		// In AMP_Gallery_Block_Sanitizer, this is referred to as carousel_required.
 		$is_carousel = isset( $attrs['amp-carousel'] )
 			? rest_sanitize_boolean( $attrs['amp-carousel'] )
@@ -73,27 +73,27 @@ public function generate_gallery_markup( $html, $attrs ) {
 
 		// Use `data` attributes to indicate which options are configured for the embed. These indications are later
 		// processed during sanitization of the embed in `::sanitize_raw_embeds`.
-		add_filter(
-			'gallery_style',
-			static function ( $style ) use ( $is_carousel, $is_lightbox ) {
-				$data_attrs = [];
+		$filter_gallery_style = static function ( $style ) use ( $is_carousel, $is_lightbox ) {
+			$data_attrs = [];
 
-				if ( $is_lightbox ) {
-					$data_attrs[] = 'data-amp-lightbox';
-				}
-
-				if ( $is_carousel ) {
-					$data_attrs[] = 'data-amp-carousel';
-				}
+			if ( $is_lightbox ) {
+				$data_attrs[] = 'data-amp-lightbox';
+			}
 
-				return str_replace( "class='", implode( ' ', $data_attrs ) . " class='", $style );
+			if ( $is_carousel ) {
+				$data_attrs[] = 'data-amp-carousel';
 			}
-		);
+
+			return str_replace( "class='", implode( ' ', $data_attrs ) . " class='", $style );
+		};
+		add_filter( 'gallery_style', $filter_gallery_style );
 
 		remove_filter( 'post_gallery', [ $this, 'generate_gallery_markup' ] );
 		$gallery_html = gallery_shortcode( $attrs );
 		add_filter( 'post_gallery', [ $this, 'generate_gallery_markup' ], 10, 2 );
 
+		remove_filter( 'gallery_style', $filter_gallery_style );
+
 		return $gallery_html;
 	}
 
diff --git a/src/Embed/HandlesGalleryEmbed.php b/src/Embed/HandlesGalleryEmbed.php
index b8c3e00b12a..206222eacc7 100644
--- a/src/Embed/HandlesGalleryEmbed.php
+++ b/src/Embed/HandlesGalleryEmbed.php
@@ -18,7 +18,7 @@
 /**
  * Trait HandlesGalleryEmbed.
  *
- * Contains logic related to both gallery embed shortcodes and blocks.
+ * Contains logic related to both gallery shortcodes and blocks.
  *
  * @since 2.0
  */

From c097f62df57b2a8c46a69e54d27c9810aac8c940 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 21 Jul 2020 16:40:45 -0700
Subject: [PATCH 11/47] Harden matching the class attribute

---
 includes/embeds/class-amp-gallery-embed-handler.php | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index e68e20f62c8..11db4e9c086 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -84,7 +84,12 @@ public function generate_gallery_markup( $html, $attrs ) {
 				$data_attrs[] = 'data-amp-carousel';
 			}
 
-			return str_replace( "class='", implode( ' ', $data_attrs ) . " class='", $style );
+			return preg_replace(
+				'/(?=\sclass=[\'"])/',
+				implode( ' ', $data_attrs ) . ' ',
+				$style,
+				1
+			);
 		};
 		add_filter( 'gallery_style', $filter_gallery_style );
 

From b64cc1ec9c3995161e73e2b8cce78e2e76f82750 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 21 Jul 2020 16:41:22 -0700
Subject: [PATCH 12/47] Use __FUNCTION__ to refer to the current function

---
 includes/embeds/class-amp-gallery-embed-handler.php | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index 11db4e9c086..dcb1fd6c7a1 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -93,9 +93,10 @@ public function generate_gallery_markup( $html, $attrs ) {
 		};
 		add_filter( 'gallery_style', $filter_gallery_style );
 
-		remove_filter( 'post_gallery', [ $this, 'generate_gallery_markup' ] );
+		// Proceed to get the original shortcode markup and prevent infinite recursion by removing this filter callback and then restoring it.
+		remove_filter( 'post_gallery', [ $this, __FUNCTION__ ] );
 		$gallery_html = gallery_shortcode( $attrs );
-		add_filter( 'post_gallery', [ $this, 'generate_gallery_markup' ], 10, 2 );
+		add_filter( 'post_gallery', [ $this, __FUNCTION__ ], 10, 2 );
 
 		remove_filter( 'gallery_style', $filter_gallery_style );
 

From a6269b5c9f3163c366d9ebeb80bcd49784ba040a Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 21 Jul 2020 16:47:43 -0700
Subject: [PATCH 13/47] Improve means of preventing infinite recursion when
 calling gallery_shortcode()

---
 .../class-amp-gallery-embed-handler.php       | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index dcb1fd6c7a1..0f0346aa936 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -33,6 +33,23 @@ public function register_embed() {
 	 * @return string Markup for the gallery.
 	 */
 	public function generate_gallery_markup( $html, $attrs ) {
+		static $recursing = false;
+		if ( ! $recursing ) {
+			$recursing = true;
+			$html      = $this->filter_post_gallery_markup( $html, $attrs );
+			$recursing = false;
+		}
+		return $html;
+	}
+
+	/**
+	 * Filter the output of gallery_shortcode().
+	 *
+	 * @param string $html  Markup to filter.
+	 * @param array  $attrs Shortcode attributes.
+	 * @return string Markup for the gallery.
+	 */
+	protected function filter_post_gallery_markup( $html, $attrs ) {
 		// Use <amp-carousel> for the gallery if requested via amp-carousel shortcode attribute, or use by default if in legacy Reader mode.
 		// In AMP_Gallery_Block_Sanitizer, this is referred to as carousel_required.
 		$is_carousel = isset( $attrs['amp-carousel'] )
@@ -91,13 +108,9 @@ public function generate_gallery_markup( $html, $attrs ) {
 				1
 			);
 		};
-		add_filter( 'gallery_style', $filter_gallery_style );
 
-		// Proceed to get the original shortcode markup and prevent infinite recursion by removing this filter callback and then restoring it.
-		remove_filter( 'post_gallery', [ $this, __FUNCTION__ ] );
+		add_filter( 'gallery_style', $filter_gallery_style );
 		$gallery_html = gallery_shortcode( $attrs );
-		add_filter( 'post_gallery', [ $this, __FUNCTION__ ], 10, 2 );
-
 		remove_filter( 'gallery_style', $filter_gallery_style );
 
 		return $gallery_html;

From 9336e1389f278038f8d48af6dd2e2fbdb943ccef Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 21 Jul 2020 17:08:43 -0700
Subject: [PATCH 14/47] Reuse Tag::FIGCAPTION

---
 includes/embeds/class-amp-gallery-embed-handler.php       | 5 +++--
 includes/sanitizers/class-amp-gallery-block-sanitizer.php | 8 +++++---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index 0f0346aa936..319dad54bbb 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -7,6 +7,7 @@
 
 use AmpProject\AmpWP\Embed\HandlesGalleryEmbed;
 use AmpProject\Dom\Document;
+use AmpProject\Tag;
 
 /**
  * Class AMP_Gallery_Embed_Handler
@@ -168,10 +169,10 @@ protected function get_caption_element( DOMElement $img_element ) {
 			$caption_element = $caption_element->nextSibling;
 		}
 
-		if ( $caption_element instanceof DOMElement && 'figcaption' !== $caption_element->nodeName ) {
+		if ( $caption_element instanceof DOMElement && Tag::FIGCAPTION !== $caption_element->nodeName ) {
 			// Transform the caption element into a `figcaption`. This not only allows the `amp-lightbox` to correctly
 			// detect and display the caption, but it is also semantically correct as the parent element will be a `figure`.
-			$figcaption_element = AMP_DOM_Utils::create_node( Document::fromNode( $caption_element ), 'figcaption', [] );
+			$figcaption_element = AMP_DOM_Utils::create_node( Document::fromNode( $caption_element ), Tag::FIGCAPTION, [] );
 
 			while ( $caption_element->hasChildNodes() ) {
 				$child = $caption_element->childNodes->item( 0 );
diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index dd4739cd530..e968a955764 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -6,6 +6,7 @@
  */
 
 use AmpProject\AmpWP\Embed\HandlesGalleryEmbed;
+use AmpProject\Tag;
 
 /**
  * Class AMP_Gallery_Block_Sanitizer
@@ -73,6 +74,8 @@ public function sanitize() {
 		$nodes       = $this->dom->xpath->query( $expr );
 
 		foreach ( $nodes as $node ) {
+			/** @var DOMElement $node */
+
 			// In WordPress 5.3, the Gallery block's <ul> is wrapped in a <figure class="wp-block-gallery">, so look for that node also.
 			$gallery_node = isset( $node->parentNode ) && AMP_DOM_Utils::has_class( $node->parentNode, self::$class ) ? $node->parentNode : $node;
 			$attributes   = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $gallery_node );
@@ -100,10 +103,9 @@ public function sanitize() {
 	 * @return DOMElement|null The caption element, or `null` if the image has none.
 	 */
 	protected function get_caption_element( DOMElement $img_element ) {
-		$caption_tag        = 'figcaption';
 		$figcaption_element = null;
 
-		if ( isset( $img_element->nextSibling->nodeName ) && $caption_tag === $img_element->nextSibling->nodeName ) {
+		if ( isset( $img_element->nextSibling->nodeName ) && Tag::FIGCAPTION === $img_element->nextSibling->nodeName ) {
 			$figcaption_element = $img_element->nextSibling;
 		}
 
@@ -111,7 +113,7 @@ protected function get_caption_element( DOMElement $img_element ) {
 		if (
 			! $figcaption_element
 			&& isset( $img_element->parentNode->nextSibling->nodeName )
-			&& $caption_tag === $img_element->parentNode->nextSibling->nodeName
+			&& Tag::FIGCAPTION === $img_element->parentNode->nextSibling->nodeName
 		) {
 			$figcaption_element = $img_element->parentNode->nextSibling;
 		}

From 81a800492774bbd4f219da8d8e7b37d2f24a1d7f Mon Sep 17 00:00:00 2001
From: Pierre Gordon <16200219+pierlon@users.noreply.github.com>
Date: Wed, 22 Jul 2020 00:25:54 +0000
Subject: [PATCH 15/47] Make insertion of `data` attributes more robust

Co-authored-by: Weston Ruter <westonruter@google.com>
---
 includes/embeds/class-amp-gallery-embed-handler.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index 319dad54bbb..2148f82c1b9 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -103,8 +103,8 @@ protected function filter_post_gallery_markup( $html, $attrs ) {
 			}
 
 			return preg_replace(
-				'/(?=\sclass=[\'"])/',
-				implode( ' ', $data_attrs ) . ' ',
+				'/(?<=<div\b)/',
+				' ' . implode( ' ', $data_attrs ) . ' ',
 				$style,
 				1
 			);

From 2421fb39341df797a7f7832d1b69f3b220084035 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <16200219+pierlon@users.noreply.github.com>
Date: Wed, 22 Jul 2020 00:29:59 +0000
Subject: [PATCH 16/47] Remove re-importing of element

Co-authored-by: Weston Ruter <westonruter@google.com>
---
 includes/embeds/class-amp-gallery-embed-handler.php | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index 2148f82c1b9..5e9f62555eb 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -174,10 +174,8 @@ protected function get_caption_element( DOMElement $img_element ) {
 			// detect and display the caption, but it is also semantically correct as the parent element will be a `figure`.
 			$figcaption_element = AMP_DOM_Utils::create_node( Document::fromNode( $caption_element ), Tag::FIGCAPTION, [] );
 
-			while ( $caption_element->hasChildNodes() ) {
-				$child = $caption_element->childNodes->item( 0 );
-				$child = $caption_element->ownerDocument->importNode( $child, true );
-				$figcaption_element->appendChild( $child );
+			while ( $caption_element->firstChild ) {
+				$figcaption_element->appendChild( $caption_element->firstChild );
 			}
 
 			$caption_element = $figcaption_element;

From 7d9a41d0867d2e44f8a57e50d02184637ea435f8 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <16200219+pierlon@users.noreply.github.com>
Date: Wed, 22 Jul 2020 00:36:40 +0000
Subject: [PATCH 17/47] Use already retrieved attributes

Co-authored-by: Weston Ruter <westonruter@google.com>
---
 includes/sanitizers/class-amp-gallery-block-sanitizer.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index e968a955764..abdb7f2c776 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -82,8 +82,8 @@ public function sanitize() {
 
 			$is_amp_lightbox = isset( $attributes['data-amp-lightbox'] ) && rest_sanitize_boolean( $attributes['data-amp-lightbox'] );
 
-			if ( $gallery_node->hasAttribute( 'data-amp-carousel' ) ) {
-				$is_amp_carousel = rest_sanitize_boolean( $gallery_node->getAttribute( 'data-amp-carousel' ) );
+			if ( isset( $attributes['data-amp-carousel'] ) ) {
+				$is_amp_carousel = rest_sanitize_boolean( $attributes['data-amp-carousel'] );
 			} else {
 				// The carousel_required argument is set to true when the theme does not support AMP. However, it is no
 				// no longer strictly required. Rather, carousels are just enabled by default.

From 0bd44f9f1dc40c136c6c276c19cef29ba912c114 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Tue, 21 Jul 2020 21:09:04 -0400
Subject: [PATCH 18/47] Enforce `DOMElement` type for caption slide

---
 src/Component/CaptionedSlide.php    | 23 ++++++++++++-----------
 src/Component/Carousel.php          | 23 +++++++++++------------
 src/Component/HasCaption.php        |  2 +-
 src/Dom/ElementList.php             |  7 +++----
 tests/php/test-amp-carousel.php     |  8 +-------
 tests/php/test-dom-element-list.php | 18 +++++++++++-------
 6 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/src/Component/CaptionedSlide.php b/src/Component/CaptionedSlide.php
index ad6cd6fcbc5..cdfd7fc3a11 100644
--- a/src/Component/CaptionedSlide.php
+++ b/src/Component/CaptionedSlide.php
@@ -7,6 +7,7 @@
 
 namespace AmpProject\AmpWP\Component;
 
+use DOMElement;
 use DOMNode;
 
 /**
@@ -27,28 +28,28 @@ final class CaptionedSlide implements HasCaption {
 	/**
 	 * The caption node.
 	 *
-	 * @var DOMNode
+	 * @var DOMElement
 	 */
-	private $caption_node;
+	private $caption_element;
 
 	/**
 	 * Constructs the class.
 	 *
-	 * @param DOMNode $slide_node   The slide node, possibly an image.
-	 * @param DOMNode $caption_node The caption node.
+	 * @param DOMNode    $slide_node      The slide node, possibly an image.
+	 * @param DOMElement $caption_element The caption element.
 	 */
-	public function __construct( DOMNode $slide_node, DOMNode $caption_node ) {
-		$this->slide_node   = $slide_node;
-		$this->caption_node = $caption_node;
+	public function __construct( DOMNode $slide_node, DOMElement $caption_element ) {
+		$this->slide_node      = $slide_node;
+		$this->caption_element = $caption_element;
 	}
 
 	/**
-	 * Gets the caption node.
+	 * Gets the caption element.
 	 *
-	 * @return DOMNode
+	 * @return DOMElement
 	 */
-	public function get_caption_node() {
-		return $this->caption_node;
+	public function get_caption_element() {
+		return $this->caption_element;
 	}
 
 	/**
diff --git a/src/Component/Carousel.php b/src/Component/Carousel.php
index 65e45bbd78a..4ba1af0b91b 100644
--- a/src/Component/Carousel.php
+++ b/src/Component/Carousel.php
@@ -13,7 +13,6 @@
 use AmpProject\Tag;
 use DOMElement;
 use AMP_DOM_Utils;
-use DOMNode;
 
 /**
  * Class Carousel
@@ -84,7 +83,7 @@ public function get_dom_element() {
 
 		foreach ( $this->slides as $slide ) {
 			$slide_node      = $slide instanceof CaptionedSlide ? $slide->get_slide_node() : $slide;
-			$caption_node    = $slide instanceof HasCaption ? $slide->get_caption_node() : null;
+			$caption_element = $slide instanceof HasCaption ? $slide->get_caption_element() : null;
 			$slide_container = AMP_DOM_Utils::create_node(
 				$this->dom,
 				Tag::FIGURE, // This cannot be a <div> because if the gallery is inside of a <p>, then the DOM will break.
@@ -104,24 +103,24 @@ public function get_dom_element() {
 			$slide_container->appendChild( $slide_node );
 
 			// If there's a caption, append it to the slide.
-			if ( $caption_node instanceof DOMNode ) {
+			if ( null !== $caption_element ) {
 				// If the caption is not a <figcaption>, wrap it in one.
-				if ( Tag::FIGCAPTION !== $caption_node->nodeName ) {
-					$caption_content = $caption_node;
-					$caption_node    = AMP_DOM_Utils::create_node( $this->dom, Tag::FIGCAPTION, [] );
-					$caption_node->appendChild( $caption_content );
+				if ( Tag::FIGCAPTION !== $caption_element->nodeName ) {
+					$caption_content = $caption_element;
+					$caption_element = AMP_DOM_Utils::create_node( $this->dom, Tag::FIGCAPTION, [] );
+					$caption_element->appendChild( $caption_content );
 				}
 
 				$caption_shortcode_class = 'wp-caption-text';
 				$caption_block_class     = 'wp-block-image';
-				$has_caption_class       = AMP_DOM_Utils::has_class( $caption_node, $caption_shortcode_class ) || AMP_DOM_Utils::has_class( $caption_node, $caption_block_class );
+				$has_caption_class       = AMP_DOM_Utils::has_class( $caption_element, $caption_shortcode_class ) || AMP_DOM_Utils::has_class( $caption_element, $caption_block_class );
 
-				/** @var DOMElement $caption_node */
-				if ( ! $has_caption_class || ! $caption_node->hasAttribute( Attribute::CLASS_ ) ) {
-					$caption_node->setAttribute( Attribute::CLASS_, 'amp-wp-gallery-caption' );
+				/** @var DOMElement $caption_element */
+				if ( ! $has_caption_class || ! $caption_element->hasAttribute( Attribute::CLASS_ ) ) {
+					$caption_element->setAttribute( Attribute::CLASS_, 'amp-wp-gallery-caption' );
 				}
 
-				$slide_container->appendChild( $caption_node );
+				$slide_container->appendChild( $caption_element );
 			}
 
 			$amp_carousel->appendChild( $slide_container );
diff --git a/src/Component/HasCaption.php b/src/Component/HasCaption.php
index fc0002ad974..d90d37b4f9e 100644
--- a/src/Component/HasCaption.php
+++ b/src/Component/HasCaption.php
@@ -22,5 +22,5 @@ interface HasCaption {
 	 *
 	 * @return DOMNode
 	 */
-	public function get_caption_node();
+	public function get_caption_element();
 }
diff --git a/src/Dom/ElementList.php b/src/Dom/ElementList.php
index 8b7cbfdedf4..252c6769a73 100644
--- a/src/Dom/ElementList.php
+++ b/src/Dom/ElementList.php
@@ -8,7 +8,6 @@
 namespace AmpProject\AmpWP\Dom;
 
 use AmpProject\AmpWP\Component\CaptionedSlide;
-use DOMNode;
 use IteratorAggregate;
 use Countable;
 use DOMElement;
@@ -32,11 +31,11 @@ final class ElementList implements IteratorAggregate, Countable {
 	/**
 	 * Adds an element to the list, possibly with a caption.
 	 *
-	 * @param DOMElement   $element The element to add, possibly an image.
-	 * @param DOMNode|null $caption The caption for the element.
+	 * @param DOMElement      $element The element to add, possibly an image.
+	 * @param DOMElement|null $caption The caption for the element.
 	 * @return ElementList A clone of this list, with the new element added.
 	 */
-	public function add( DOMElement $element, DOMNode $caption = null ) {
+	public function add( DOMElement $element, DOMElement $caption = null ) {
 		$cloned_list             = clone $this;
 		$cloned_list->elements[] = null === $caption ? $element : new CaptionedSlide( $element, $caption );
 		return $cloned_list;
diff --git a/tests/php/test-amp-carousel.php b/tests/php/test-amp-carousel.php
index 06add4cb031..712ee54000f 100644
--- a/tests/php/test-amp-carousel.php
+++ b/tests/php/test-amp-carousel.php
@@ -15,7 +15,7 @@
  *
  * @covers \AmpProject\AmpWP\Component\Carousel
  */
-class Test_Carousel extends \WP_UnitTestCase {
+class Test_Carousel extends WP_UnitTestCase {
 
 	use PrivateAccess;
 
@@ -44,12 +44,6 @@ public function get_carousel_data() {
 				$dom,
 				'<amp-carousel width="' . $width . '" height="' . $height . '" type="slides" layout="responsive"><figure class="slide"><amp-img src="' . $src . '" width="' . $width . '" height="' . $height . '" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
 			],
-			'image_with_text_caption' => [
-				( new ElementList() )->add( $image, new DOMText( 'Example caption' ) ),
-				$dom,
-				'<amp-carousel width="' . $width . '" height="' . $height . '" type="slides" layout="responsive"><figure class="slide"><amp-img src="' . $src . '" width="' . $width . '" height="' . $height . '" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption">Example caption</figcaption></figure></amp-carousel>',
-			],
-
 			'image_with_html_caption' => [
 				( new ElementList() )->add( $image, $caption_element ),
 				$dom,
diff --git a/tests/php/test-dom-element-list.php b/tests/php/test-dom-element-list.php
index ecb08260374..cb5f2f3a0a3 100644
--- a/tests/php/test-dom-element-list.php
+++ b/tests/php/test-dom-element-list.php
@@ -77,7 +77,7 @@ public function test_dom_element_list_add( $images, $expected_count ) {
 	public function test_dom_element_list_get_iterator( $images, $expected_count ) {
 		$dom_element_list = new ElementList();
 		foreach ( $images as $image ) {
-			$dom_element_list = $dom_element_list->add( $image, null );
+			$dom_element_list = $dom_element_list->add( $image );
 		}
 
 		$iteration_count = 0;
@@ -92,13 +92,17 @@ public function test_dom_element_list_get_iterator( $images, $expected_count ) {
 	/**
 	 * Test get_caption.
 	 *
-	 * @covers \AmpProject\AmpWP\Component\CaptionedSlide::get_caption_node()
+	 * @covers \AmpProject\AmpWP\Component\CaptionedSlide::get_caption_element()
 	 */
 	public function test_get_caption() {
-		$image_node      = AMP_DOM_Utils::create_node( new Document(), 'amp-img', [] );
-		$caption         = new DOMText( 'This is a caption' );
-		$captioned_image = new CaptionedSlide( $image_node, $caption );
-		$this->assertEquals( $caption, $captioned_image->get_caption_node() );
+		$dom = new Document();
+
+		$image_node      = AMP_DOM_Utils::create_node( $dom, 'amp-img', [] );
+		$caption_element = AMP_DOM_Utils::create_node( $dom, 'span', [] );
+		$caption_element->appendChild( new DOMText( 'This is a caption' ) );
+
+		$captioned_image = new CaptionedSlide( $image_node, $caption_element );
+		$this->assertEquals( $caption_element, $captioned_image->get_caption_element() );
 	}
 
 	/**
@@ -108,7 +112,7 @@ public function test_get_caption() {
 	 */
 	public function test_get_slide_node() {
 		$image_node = AMP_DOM_Utils::create_node( new Document(), 'amp-img', [] );
-		$amp_image  = new CaptionedSlide( $image_node, new DOMNode() );
+		$amp_image  = new CaptionedSlide( $image_node, new DOMElement( 'foo' ) );
 		$this->assertEquals( $image_node, $amp_image->get_slide_node() );
 	}
 }

From 0ce7c511da660b2cb4a67697f0d4a6d714f91a7d Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Tue, 21 Jul 2020 21:00:46 -0700
Subject: [PATCH 19/47] Enforce slide node as being an element same as caption

---
 .../class-amp-gallery-embed-handler.php       |  2 +-
 src/Component/CaptionedSlide.php              | 19 +++++++++----------
 src/Component/Carousel.php                    |  4 ++--
 tests/php/test-dom-element-list.php           |  8 ++++----
 4 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index 5e9f62555eb..f56b8c8a8df 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -104,7 +104,7 @@ protected function filter_post_gallery_markup( $html, $attrs ) {
 
 			return preg_replace(
 				'/(?<=<div\b)/',
-				' ' . implode( ' ', $data_attrs ) . ' ',
+				' ' . implode( ' ', $data_attrs ),
 				$style,
 				1
 			);
diff --git a/src/Component/CaptionedSlide.php b/src/Component/CaptionedSlide.php
index cdfd7fc3a11..313b959698e 100644
--- a/src/Component/CaptionedSlide.php
+++ b/src/Component/CaptionedSlide.php
@@ -8,7 +8,6 @@
 namespace AmpProject\AmpWP\Component;
 
 use DOMElement;
-use DOMNode;
 
 /**
  * Class CaptionedSlide
@@ -21,9 +20,9 @@ final class CaptionedSlide implements HasCaption {
 	/**
 	 * The slide node, possibly an image.
 	 *
-	 * @var DOMNode
+	 * @var DOMElement
 	 */
-	private $slide_node;
+	private $slide_element;
 
 	/**
 	 * The caption node.
@@ -35,11 +34,11 @@ final class CaptionedSlide implements HasCaption {
 	/**
 	 * Constructs the class.
 	 *
-	 * @param DOMNode    $slide_node      The slide node, possibly an image.
+	 * @param DOMElement $slide_element   The slide node, possibly an image.
 	 * @param DOMElement $caption_element The caption element.
 	 */
-	public function __construct( DOMNode $slide_node, DOMElement $caption_element ) {
-		$this->slide_node      = $slide_node;
+	public function __construct( DOMElement $slide_element, DOMElement $caption_element ) {
+		$this->slide_element   = $slide_element;
 		$this->caption_element = $caption_element;
 	}
 
@@ -53,11 +52,11 @@ public function get_caption_element() {
 	}
 
 	/**
-	 * Gets the slide node.
+	 * Gets the slide element.
 	 *
-	 * @return DOMNode
+	 * @return DOMElement
 	 */
-	public function get_slide_node() {
-		return $this->slide_node;
+	public function get_slide_element() {
+		return $this->slide_element;
 	}
 }
diff --git a/src/Component/Carousel.php b/src/Component/Carousel.php
index 4ba1af0b91b..91811590e8f 100644
--- a/src/Component/Carousel.php
+++ b/src/Component/Carousel.php
@@ -82,7 +82,7 @@ public function get_dom_element() {
 		);
 
 		foreach ( $this->slides as $slide ) {
-			$slide_node      = $slide instanceof CaptionedSlide ? $slide->get_slide_node() : $slide;
+			$slide_node      = $slide instanceof CaptionedSlide ? $slide->get_slide_element() : $slide;
 			$caption_element = $slide instanceof HasCaption ? $slide->get_caption_element() : null;
 			$slide_container = AMP_DOM_Utils::create_node(
 				$this->dom,
@@ -152,7 +152,7 @@ private function get_dimensions() {
 		$carousel_height  = 0;
 
 		foreach ( $this->slides as $slide ) {
-			$slide_node = $slide instanceof CaptionedSlide ? $slide->get_slide_node() : $slide;
+			$slide_node = $slide instanceof CaptionedSlide ? $slide->get_slide_element() : $slide;
 			// Account for an <amp-img> that's wrapped in an <a>.
 			if ( ! $this->is_image_element( $slide_node ) && $slide_node->firstChild instanceof DOMElement && $this->is_image_element( $slide_node->firstChild ) ) {
 				$slide_node = $slide_node->firstChild;
diff --git a/tests/php/test-dom-element-list.php b/tests/php/test-dom-element-list.php
index cb5f2f3a0a3..eb62caad13f 100644
--- a/tests/php/test-dom-element-list.php
+++ b/tests/php/test-dom-element-list.php
@@ -106,13 +106,13 @@ public function test_get_caption() {
 	}
 
 	/**
-	 * Test get_slide_node.
+	 * Test get_slide_element.
 	 *
-	 * @covers \AmpProject\AmpWP\Component\CaptionedSlide::get_slide_node()
+	 * @covers \AmpProject\AmpWP\Component\CaptionedSlide::get_slide_element()
 	 */
-	public function test_get_slide_node() {
+	public function test_get_slide_element() {
 		$image_node = AMP_DOM_Utils::create_node( new Document(), 'amp-img', [] );
 		$amp_image  = new CaptionedSlide( $image_node, new DOMElement( 'foo' ) );
-		$this->assertEquals( $image_node, $amp_image->get_slide_node() );
+		$this->assertEquals( $image_node, $amp_image->get_slide_element() );
 	}
 }

From adc609ddae4c18ef1a58774f18811615b6f3bc37 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Wed, 22 Jul 2020 00:57:02 -0400
Subject: [PATCH 20/47] Ensure gallery caption is retained

---
 includes/sanitizers/class-amp-gallery-block-sanitizer.php | 5 ++++-
 tests/php/test-class-amp-gallery-block-sanitizer.php      | 5 +++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index abdb7f2c776..c2f1b9133bc 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -80,6 +80,9 @@ public function sanitize() {
 			$gallery_node = isset( $node->parentNode ) && AMP_DOM_Utils::has_class( $node->parentNode, self::$class ) ? $node->parentNode : $node;
 			$attributes   = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $gallery_node );
 
+			$gallery_node->removeAttribute( 'data-amp-carousel' );
+			$gallery_node->removeAttribute( 'data-amp-lightbox' );
+
 			$is_amp_lightbox = isset( $attributes['data-amp-lightbox'] ) && rest_sanitize_boolean( $attributes['data-amp-lightbox'] );
 
 			if ( isset( $attributes['data-amp-carousel'] ) ) {
@@ -92,7 +95,7 @@ public function sanitize() {
 
 			$img_elements = $node->getElementsByTagName( 'amp-img' );
 
-			$this->process_gallery_embed( $is_amp_carousel, $is_amp_lightbox, $gallery_node, $img_elements );
+			$this->process_gallery_embed( $is_amp_carousel, $is_amp_lightbox, $node, $img_elements );
 		}
 	}
 
diff --git a/tests/php/test-class-amp-gallery-block-sanitizer.php b/tests/php/test-class-amp-gallery-block-sanitizer.php
index 038db5fae33..568a7ea9fe6 100644
--- a/tests/php/test-class-amp-gallery-block-sanitizer.php
+++ b/tests/php/test-class-amp-gallery-block-sanitizer.php
@@ -145,6 +145,11 @@ public function get_reader_mode_data() {
 				'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
 				'<amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
 			],
+
+			'data_amp_lightbox_carousel_and_link_with_gallery_caption' => [
+				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><ul class="blocks-gallery-grid"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul><figcaption>Caption for gallery</figcaption></figure>',
+				'<figure class="wp-block-gallery"><amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel><figcaption>Caption for gallery</figcaption></figure>',
+			],
 		];
 	}
 

From 0c568e624daed842c3c5a44eaef350b0b64deed4 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Wed, 22 Jul 2020 01:29:18 -0400
Subject: [PATCH 21/47] Fix failing tests

---
 ...test-class-amp-gallery-block-sanitizer.php | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tests/php/test-class-amp-gallery-block-sanitizer.php b/tests/php/test-class-amp-gallery-block-sanitizer.php
index 568a7ea9fe6..649db9dceb4 100644
--- a/tests/php/test-class-amp-gallery-block-sanitizer.php
+++ b/tests/php/test-class-amp-gallery-block-sanitizer.php
@@ -54,27 +54,27 @@ public function get_data() {
 			// WordPress 5.3 changed the markup for the Gallery block, wrapping it in a <figure>.
 			'data_amp_with_carousel_caption_5_3'  => [
 				'<figure class="wp-block-gallery" data-amp-carousel="true"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img><figcaption>This is a caption</figcaption></figure></li></ul></figure>',
-				'<amp-carousel width="600" height="400" type="slides" layout="responsive"><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption">This is a caption</figcaption></figure></amp-carousel>',
+				'<figure class="wp-block-gallery"><amp-carousel width="600" height="400" type="slides" layout="responsive"><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption">This is a caption</figcaption></figure></amp-carousel></figure>',
 			],
 
 			'data_amp_with_lightbox'              => [
 				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></figure></li></ul>',
-				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></figure></li></ul>',
+				'<ul class="wp-block-gallery"><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></figure></li></ul>',
 			],
 
 			'data_amp_with_lightbox_and_link'     => [
 				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
-				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></a></figure></li></ul>',
+				'<ul class="wp-block-gallery"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></a></figure></li></ul>',
 			],
 
 			'data_amp_with_lightbox_5_3'          => [
 				'<figure class="wp-block-gallery" data-amp-lightbox="true"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></figure></li></ul></figure>',
-				'<figure class="wp-block-gallery" data-amp-lightbox="true"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></figure></li></ul></figure>',
+				'<figure class="wp-block-gallery"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></figure></li></ul></figure>',
 			],
 
 			'data_amp_with_lightbox_and_link_5_3' => [
 				'<figure class="wp-block-gallery" data-amp-lightbox="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul></figure>',
-				'<figure class="wp-block-gallery" data-amp-lightbox="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></a></figure></li></ul></figure>',
+				'<figure class="wp-block-gallery"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></a></figure></li></ul></figure>',
 			],
 
 			'data_amp_with_lightbox_and_carousel' => [
@@ -84,7 +84,12 @@ public function get_data() {
 
 			'data_amp_with_lightbox_carousel_5_3' => [
 				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="1234" height="567"></amp-img></a></figure></li></ul></figure>',
-				'<amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
+				'<figure class="wp-block-gallery"><amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel></figure>',
+			],
+
+			'data_amp_with_lightbox_carousel_gallery_caption_5_3' => [
+				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="1234" height="567"></amp-img></a></figure></li></ul><figcaption>This is the gallery caption</figcaption></figure>',
+				'<figure class="wp-block-gallery"><amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel><figcaption>This is the gallery caption</figcaption></figure>',
 			],
 		];
 	}
@@ -145,11 +150,6 @@ public function get_reader_mode_data() {
 				'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
 				'<amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel>',
 			],
-
-			'data_amp_lightbox_carousel_and_link_with_gallery_caption' => [
-				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><ul class="blocks-gallery-grid"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul><figcaption>Caption for gallery</figcaption></figure>',
-				'<figure class="wp-block-gallery"><amp-carousel width="600" height="400" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel><figcaption>Caption for gallery</figcaption></figure>',
-			],
 		];
 	}
 

From a4becff931c5b96f5b2619941acd256243515e98 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Wed, 22 Jul 2020 21:06:30 -0400
Subject: [PATCH 22/47] Remove deprecated AMP props from blocks that may
 contain them

---
 assets/src/block-editor/helpers/index.js | 51 ++++++++++++++++++++++++
 assets/src/block-editor/index.js         |  3 +-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index dabfabcec4c..f6d76c986ec 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -164,6 +164,57 @@ export const addAMPAttributes = ( settings, name ) => {
 	return settings;
 };
 
+/**
+ * Remove AMP deprecated props from any blocks which may contain them.
+ *
+ * @param {Object} settings Block settings.
+ *
+ * @return {Object} Modified block settings.
+ */
+export const removeDeprecatedAmpProps = ( settings ) => {
+	const mappedAttributes = {
+		ampCarousel: 'data-amp-carousel',
+		ampLightbox: 'data-amp-lightbox',
+		ampNoLoading: 'data-amp-noloading',
+		ampNoLayout: 'data-amp-layout',
+	};
+	const deprecatedProps = {};
+	const maybeBlockHasDeprecatedProp = Object.keys( mappedAttributes )
+		.some( ( attribute ) => Object.keys( settings.attributes ).includes( attribute ) );
+
+	if ( maybeBlockHasDeprecatedProp ) {
+		// At minimum, the `attributes`, and `save` props are required.
+		const deprecatedConfig = {
+			attributes: settings.attributes,
+			save( blockData ) {
+				// Render the block.
+				let element = settings.save( blockData );
+				const { attributes: blockAttributes } = blockData;
+
+				// Collect all deprecated props that would be set on the saved HTML block in the post.
+				Object.keys( mappedAttributes ).forEach( ( attribute ) => {
+					const deprecatedPropSet = Object.keys( blockAttributes ).includes( attribute ) && blockAttributes[ attribute ];
+
+					if ( deprecatedPropSet ) {
+						deprecatedProps[ mappedAttributes[ attribute ] ] = blockAttributes[ attribute ];
+					}
+				} );
+
+				// Re-render the block with deprecated props.
+				const props = { ...element.props, ...deprecatedProps };
+				element = cloneElement( element, props );
+
+				// Gutenberg will now compare the difference and update the block accordingly without the deprecated props.
+				return element;
+			},
+		};
+
+		settings.deprecated.unshift( deprecatedConfig );
+	}
+
+	return settings;
+};
+
 /**
  * Filters blocks' save function.
  *
diff --git a/assets/src/block-editor/index.js b/assets/src/block-editor/index.js
index f625e6f2fdd..fd887c31f6c 100644
--- a/assets/src/block-editor/index.js
+++ b/assets/src/block-editor/index.js
@@ -13,7 +13,7 @@ import domReady from '@wordpress/dom-ready';
 import { withFeaturedImageNotice } from '../common/components';
 import { getMinimumFeaturedImageDimensions } from '../common/helpers';
 import { withMediaLibraryNotice, AMPPreview } from './components';
-import { addAMPAttributes, filterBlocksEdit, filterBlocksSave, renderPreviewButton } from './helpers';
+import { addAMPAttributes, filterBlocksEdit, filterBlocksSave, renderPreviewButton, removeDeprecatedAmpProps } from './helpers';
 import './store';
 
 const {
@@ -29,6 +29,7 @@ plugins.keys().forEach( ( modulePath ) => {
 } );
 
 addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', addAMPAttributes );
+addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/deprecateProps', removeDeprecatedAmpProps );
 addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', filterBlocksSave );
 addFilter( 'editor.BlockEdit', 'ampEditorBlocks/filterEdit', filterBlocksEdit, 20 );
 addFilter( 'editor.PostFeaturedImage', 'ampEditorBlocks/withFeaturedImageNotice', withFeaturedImageNotice );

From dc7007135b298a00abe9cf233c9368ef703a6f58 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Wed, 22 Jul 2020 18:49:15 -0700
Subject: [PATCH 23/47] Define deprecated array if not defined already

---
 assets/src/block-editor/helpers/index.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index f6d76c986ec..1efb3196ccd 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -209,6 +209,9 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 			},
 		};
 
+		if ( ! settings.deprecated ) {
+			settings.deprecated = [];
+		}
 		settings.deprecated.unshift( deprecatedConfig );
 	}
 

From edbd6303cb4354e082657c5118b55d8da4756e87 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Wed, 22 Jul 2020 18:57:55 -0700
Subject: [PATCH 24/47] Improve jsdoc for static analysis

---
 assets/src/block-editor/helpers/index.js | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index 1efb3196ccd..d58ebf64014 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -366,6 +366,8 @@ export const filterBlocksEdit = ( BlockEdit ) => {
  * Set width and height in case of image block.
  *
  * @param {Object} props Props.
+ * @param {Function} props.setAttributes Callback to set attributes.
+ * @param {Object} props.attributes Attributes.
  * @param {string} layout Layout.
  */
 export const setImageBlockLayoutAttributes = ( props, layout ) => {
@@ -496,6 +498,9 @@ AmpNoloadingToggle.propTypes = {
  * @todo Consider wrapping the render function to delete the original font size in text settings when ampFitText.
  *
  * @param {Object} props Props.
+ * @param {Function} props.setAttributes Callback to set attributes.
+ * @param {Object} props.attributes Attributes.
+ * @param {boolean} props.isSelected Is selected.
  *
  * @return {ReactElement} Inspector Controls.
  */

From f0e9d77a881f5c196ea06da504f30845978722be Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Wed, 22 Jul 2020 19:14:05 -0700
Subject: [PATCH 25/47] Fix ampLayout attribute name

---
 assets/src/block-editor/helpers/index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index d58ebf64014..82e99c9b014 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -176,7 +176,7 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 		ampCarousel: 'data-amp-carousel',
 		ampLightbox: 'data-amp-lightbox',
 		ampNoLoading: 'data-amp-noloading',
-		ampNoLayout: 'data-amp-layout',
+		ampLayout: 'data-amp-layout',
 	};
 	const deprecatedProps = {};
 	const maybeBlockHasDeprecatedProp = Object.keys( mappedAttributes )

From d5fa7976a46c57c9ee4827dc961739c82f14ecb2 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Wed, 22 Jul 2020 22:42:44 -0400
Subject: [PATCH 26/47] Reset `deprecatedProps` object when done re-rendering
 block

---
 assets/src/block-editor/helpers/index.js | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index 82e99c9b014..01dd3045cbd 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -178,7 +178,7 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 		ampNoLoading: 'data-amp-noloading',
 		ampLayout: 'data-amp-layout',
 	};
-	const deprecatedProps = {};
+	let deprecatedProps = {};
 	const maybeBlockHasDeprecatedProp = Object.keys( mappedAttributes )
 		.some( ( attribute ) => Object.keys( settings.attributes ).includes( attribute ) );
 
@@ -200,10 +200,17 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 					}
 				} );
 
+				// If there are not any deprecated props, return the unmodified element.
+				if ( 0 === Object.keys( deprecatedProps ).length ) {
+					return element;
+				}
+
 				// Re-render the block with deprecated props.
 				const props = { ...element.props, ...deprecatedProps };
 				element = cloneElement( element, props );
 
+				deprecatedProps = {};
+
 				// Gutenberg will now compare the difference and update the block accordingly without the deprecated props.
 				return element;
 			},

From 7b1b7657c5c7ac39ecb767e360aefd5ce7e129a6 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Wed, 22 Jul 2020 22:44:31 -0400
Subject: [PATCH 27/47] Fix propType for `ampNoLoading` toggle

---
 assets/src/block-editor/helpers/index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index 01dd3045cbd..c059913856a 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -494,7 +494,7 @@ const AmpNoloadingToggle = ( props ) => {
 
 AmpNoloadingToggle.propTypes = {
 	attributes: PropTypes.shape( {
-		ampNoLoading: PropTypes.string,
+		ampNoLoading: PropTypes.bool,
 	} ),
 	setAttributes: PropTypes.func.isRequired,
 };

From 32f224ec39efd31db5eaf43f1734e2a0bdb11e37 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Wed, 22 Jul 2020 22:54:57 -0400
Subject: [PATCH 28/47] Move `deprecatedProps` into `save` method

---
 assets/src/block-editor/helpers/index.js | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index c059913856a..3afdc3fea86 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -178,7 +178,6 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 		ampNoLoading: 'data-amp-noloading',
 		ampLayout: 'data-amp-layout',
 	};
-	let deprecatedProps = {};
 	const maybeBlockHasDeprecatedProp = Object.keys( mappedAttributes )
 		.some( ( attribute ) => Object.keys( settings.attributes ).includes( attribute ) );
 
@@ -187,6 +186,8 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 		const deprecatedConfig = {
 			attributes: settings.attributes,
 			save( blockData ) {
+				const deprecatedProps = {};
+
 				// Render the block.
 				let element = settings.save( blockData );
 				const { attributes: blockAttributes } = blockData;
@@ -209,8 +210,6 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 				const props = { ...element.props, ...deprecatedProps };
 				element = cloneElement( element, props );
 
-				deprecatedProps = {};
-
 				// Gutenberg will now compare the difference and update the block accordingly without the deprecated props.
 				return element;
 			},

From f7be98b50582bd6f40d2ab40be8929812978609c Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Thu, 23 Jul 2020 02:19:47 -0400
Subject: [PATCH 29/47] Escape block property value

---
 includes/embeds/class-amp-core-block-handler.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php
index 4bb6a31ed81..b8ef996b856 100644
--- a/includes/embeds/class-amp-core-block-handler.php
+++ b/includes/embeds/class-amp-core-block-handler.php
@@ -87,8 +87,8 @@ public function filter_rendered_block( $block_content, $block ) {
 			];
 			foreach ( $prop_attribute_mapping as $prop => $attr ) {
 				if ( isset( $block['attrs'][ $prop ] ) ) {
-					$property_value       = rest_sanitize_boolean( $block['attrs'][ $prop ] );
-					$injected_attributes .= sprintf( ' %s="%s"', $attr, esc_attr( wp_json_encode( $property_value ) ) );
+					$property_value       = $block['attrs'][ $prop ];
+					$injected_attributes .= sprintf( ' %s="%s"', $attr, esc_attr( $property_value ) );
 				}
 			}
 			if ( $injected_attributes ) {

From 405359bf8ecd22f87763e56d24ef78c98cc71949 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <16200219+pierlon@users.noreply.github.com>
Date: Thu, 23 Jul 2020 06:28:23 +0000
Subject: [PATCH 30/47] Simplify check for deprecated prop

Co-authored-by: Weston Ruter <westonruter@google.com>
---
 assets/src/block-editor/helpers/index.js | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index 3afdc3fea86..b9c36de732a 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -194,9 +194,7 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 
 				// Collect all deprecated props that would be set on the saved HTML block in the post.
 				Object.keys( mappedAttributes ).forEach( ( attribute ) => {
-					const deprecatedPropSet = Object.keys( blockAttributes ).includes( attribute ) && blockAttributes[ attribute ];
-
-					if ( deprecatedPropSet ) {
+					if ( attribute in blockAttributes && blockAttributes[ attribute ] ) {
 						deprecatedProps[ mappedAttributes[ attribute ] ] = blockAttributes[ attribute ];
 					}
 				} );

From b7de0f1423bcaaf708dbbc0955cbac339c743bee Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Wed, 22 Jul 2020 23:32:22 -0700
Subject: [PATCH 31/47] Use for/of loop; move mappedAttributes out of
 removeDeprecatedAmpProps

---
 assets/src/block-editor/helpers/index.js | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index b9c36de732a..49752231bb2 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -82,6 +82,13 @@ const ampLayoutOptions = [
 	},
 ];
 
+const mappedAttributes = {
+	ampCarousel: 'data-amp-carousel',
+	ampLightbox: 'data-amp-lightbox',
+	ampNoLoading: 'data-amp-noloading',
+	ampLayout: 'data-amp-layout',
+};
+
 /**
  * Add AMP attributes to every core block.
  *
@@ -172,12 +179,6 @@ export const addAMPAttributes = ( settings, name ) => {
  * @return {Object} Modified block settings.
  */
 export const removeDeprecatedAmpProps = ( settings ) => {
-	const mappedAttributes = {
-		ampCarousel: 'data-amp-carousel',
-		ampLightbox: 'data-amp-lightbox',
-		ampNoLoading: 'data-amp-noloading',
-		ampLayout: 'data-amp-layout',
-	};
 	const maybeBlockHasDeprecatedProp = Object.keys( mappedAttributes )
 		.some( ( attribute ) => Object.keys( settings.attributes ).includes( attribute ) );
 
@@ -193,11 +194,11 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 				const { attributes: blockAttributes } = blockData;
 
 				// Collect all deprecated props that would be set on the saved HTML block in the post.
-				Object.keys( mappedAttributes ).forEach( ( attribute ) => {
-					if ( attribute in blockAttributes && blockAttributes[ attribute ] ) {
-						deprecatedProps[ mappedAttributes[ attribute ] ] = blockAttributes[ attribute ];
+				for ( const [ blockAttributeName, htmlAttributeName ] of Object.entries( mappedAttributes ) ) {
+					if ( blockAttributeName in blockAttributes && blockAttributes[ blockAttributeName ] ) {
+						deprecatedProps[ htmlAttributeName ] = blockAttributes[ blockAttributeName ];
 					}
-				} );
+				}
 
 				// If there are not any deprecated props, return the unmodified element.
 				if ( 0 === Object.keys( deprecatedProps ).length ) {

From da088e0c1ca86580d1ac69721424f988b1a29868 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Wed, 22 Jul 2020 23:42:50 -0700
Subject: [PATCH 32/47] Ensure PHP boolean is stringified as true/false

---
 includes/embeds/class-amp-core-block-handler.php          | 6 +++++-
 includes/sanitizers/class-amp-gallery-block-sanitizer.php | 3 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php
index b8ef996b856..e29c315b62f 100644
--- a/includes/embeds/class-amp-core-block-handler.php
+++ b/includes/embeds/class-amp-core-block-handler.php
@@ -87,7 +87,11 @@ public function filter_rendered_block( $block_content, $block ) {
 			];
 			foreach ( $prop_attribute_mapping as $prop => $attr ) {
 				if ( isset( $block['attrs'][ $prop ] ) ) {
-					$property_value       = $block['attrs'][ $prop ];
+					$property_value = $block['attrs'][ $prop ];
+					if ( is_bool( $property_value ) ) {
+						$property_value = wp_json_encode( $property_value );
+					}
+
 					$injected_attributes .= sprintf( ' %s="%s"', $attr, esc_attr( $property_value ) );
 				}
 			}
diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index c2f1b9133bc..ff6f8e91c7b 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -80,9 +80,6 @@ public function sanitize() {
 			$gallery_node = isset( $node->parentNode ) && AMP_DOM_Utils::has_class( $node->parentNode, self::$class ) ? $node->parentNode : $node;
 			$attributes   = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $gallery_node );
 
-			$gallery_node->removeAttribute( 'data-amp-carousel' );
-			$gallery_node->removeAttribute( 'data-amp-lightbox' );
-
 			$is_amp_lightbox = isset( $attributes['data-amp-lightbox'] ) && rest_sanitize_boolean( $attributes['data-amp-lightbox'] );
 
 			if ( isset( $attributes['data-amp-carousel'] ) ) {

From fd4f8b69b49d329d84ec6d94412bd943346c5ff6 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Thu, 23 Jul 2020 00:00:52 -0700
Subject: [PATCH 33/47] Eliminate gratuitous use of wp_json_encode()

---
 includes/embeds/class-amp-core-block-handler.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php
index e29c315b62f..b0fd39a20a7 100644
--- a/includes/embeds/class-amp-core-block-handler.php
+++ b/includes/embeds/class-amp-core-block-handler.php
@@ -89,7 +89,7 @@ public function filter_rendered_block( $block_content, $block ) {
 				if ( isset( $block['attrs'][ $prop ] ) ) {
 					$property_value = $block['attrs'][ $prop ];
 					if ( is_bool( $property_value ) ) {
-						$property_value = wp_json_encode( $property_value );
+						$property_value = $property_value ? 'true' : 'false';
 					}
 
 					$injected_attributes .= sprintf( ' %s="%s"', $attr, esc_attr( $property_value ) );

From 7d9b3ed4fa0fba117f5cf719d8ebb2737d9c9588 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Thu, 23 Jul 2020 03:11:41 -0400
Subject: [PATCH 34/47] Fix issue with image captions not showing in
 amp-carousel when rendered from gallery shortcode

---
 src/Component/Carousel.php                         |  6 ++----
 .../php/test-class-amp-gallery-block-sanitizer.php | 14 +++++++-------
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/Component/Carousel.php b/src/Component/Carousel.php
index 91811590e8f..11d265e0715 100644
--- a/src/Component/Carousel.php
+++ b/src/Component/Carousel.php
@@ -111,12 +111,10 @@ public function get_dom_element() {
 					$caption_element->appendChild( $caption_content );
 				}
 
-				$caption_shortcode_class = 'wp-caption-text';
-				$caption_block_class     = 'wp-block-image';
-				$has_caption_class       = AMP_DOM_Utils::has_class( $caption_element, $caption_shortcode_class ) || AMP_DOM_Utils::has_class( $caption_element, $caption_block_class );
+				$has_caption_class = AMP_DOM_Utils::has_class( $caption_element, 'amp-wp-gallery-caption' );
 
 				/** @var DOMElement $caption_element */
-				if ( ! $has_caption_class || ! $caption_element->hasAttribute( Attribute::CLASS_ ) ) {
+				if ( ! $has_caption_class ) {
 					$caption_element->setAttribute( Attribute::CLASS_, 'amp-wp-gallery-caption' );
 				}
 
diff --git a/tests/php/test-class-amp-gallery-block-sanitizer.php b/tests/php/test-class-amp-gallery-block-sanitizer.php
index 649db9dceb4..e745f77f602 100644
--- a/tests/php/test-class-amp-gallery-block-sanitizer.php
+++ b/tests/php/test-class-amp-gallery-block-sanitizer.php
@@ -54,27 +54,27 @@ public function get_data() {
 			// WordPress 5.3 changed the markup for the Gallery block, wrapping it in a <figure>.
 			'data_amp_with_carousel_caption_5_3'  => [
 				'<figure class="wp-block-gallery" data-amp-carousel="true"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img><figcaption>This is a caption</figcaption></figure></li></ul></figure>',
-				'<figure class="wp-block-gallery"><amp-carousel width="600" height="400" type="slides" layout="responsive"><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption">This is a caption</figcaption></figure></amp-carousel></figure>',
+				'<figure class="wp-block-gallery" data-amp-carousel="true"><amp-carousel width="600" height="400" type="slides" layout="responsive"><figure class="slide"><amp-img src="http://example.com/img.png" width="600" height="400" layout="fill" object-fit="cover"></amp-img><figcaption class="amp-wp-gallery-caption">This is a caption</figcaption></figure></amp-carousel></figure>',
 			],
 
 			'data_amp_with_lightbox'              => [
 				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></figure></li></ul>',
-				'<ul class="wp-block-gallery"><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></figure></li></ul>',
+				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></figure></li></ul>',
 			],
 
 			'data_amp_with_lightbox_and_link'     => [
 				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
-				'<ul class="wp-block-gallery"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></a></figure></li></ul>',
+				'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></a></figure></li></ul>',
 			],
 
 			'data_amp_with_lightbox_5_3'          => [
 				'<figure class="wp-block-gallery" data-amp-lightbox="true"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></figure></li></ul></figure>',
-				'<figure class="wp-block-gallery"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></figure></li></ul></figure>',
+				'<figure class="wp-block-gallery" data-amp-lightbox="true"><ul><li class="blocks-gallery-item"><figure><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></figure></li></ul></figure>',
 			],
 
 			'data_amp_with_lightbox_and_link_5_3' => [
 				'<figure class="wp-block-gallery" data-amp-lightbox="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul></figure>',
-				'<figure class="wp-block-gallery"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></a></figure></li></ul></figure>',
+				'<figure class="wp-block-gallery" data-amp-lightbox="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" lightbox=""></amp-img></a></figure></li></ul></figure>',
 			],
 
 			'data_amp_with_lightbox_and_carousel' => [
@@ -84,12 +84,12 @@ public function get_data() {
 
 			'data_amp_with_lightbox_carousel_5_3' => [
 				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="1234" height="567"></amp-img></a></figure></li></ul></figure>',
-				'<figure class="wp-block-gallery"><amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel></figure>',
+				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel></figure>',
 			],
 
 			'data_amp_with_lightbox_carousel_gallery_caption_5_3' => [
 				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><ul><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="1234" height="567"></amp-img></a></figure></li></ul><figcaption>This is the gallery caption</figcaption></figure>',
-				'<figure class="wp-block-gallery"><amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel><figcaption>This is the gallery caption</figcaption></figure>',
+				'<figure class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><amp-carousel width="1234" height="567" type="slides" layout="responsive" lightbox=""><figure class="slide"><amp-img src="http://example.com/img.png" width="1234" height="567" layout="fill" object-fit="cover"></amp-img></figure></amp-carousel><figcaption>This is the gallery caption</figcaption></figure>',
 			],
 		];
 	}

From 48343af28949081e31bbbcdb7733bf5bb4dd62b1 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Thu, 23 Jul 2020 23:27:50 -0700
Subject: [PATCH 35/47] Supply missing attribute value for gallery shortcode
 output

---
 includes/embeds/class-amp-gallery-embed-handler.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index f56b8c8a8df..0d2f7357875 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -95,11 +95,11 @@ protected function filter_post_gallery_markup( $html, $attrs ) {
 			$data_attrs = [];
 
 			if ( $is_lightbox ) {
-				$data_attrs[] = 'data-amp-lightbox';
+				$data_attrs[] = 'data-amp-lightbox="true"';
 			}
 
 			if ( $is_carousel ) {
-				$data_attrs[] = 'data-amp-carousel';
+				$data_attrs[] = 'data-amp-carousel="true"';
 			}
 
 			return preg_replace(

From bb85a0249733acda837c4f5c86de17aacb973aba Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Thu, 23 Jul 2020 23:50:22 -0700
Subject: [PATCH 36/47] Ensure data-amp-carousel=true attribute is present for
 proper styling of gallery block

---
 includes/sanitizers/class-amp-gallery-block-sanitizer.php | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
index ff6f8e91c7b..f8ea9abac1c 100644
--- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php
+++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php
@@ -90,6 +90,11 @@ public function sanitize() {
 				$is_amp_carousel = ! empty( $this->args['carousel_required'] );
 			}
 
+			// Ensure data-amp-carousel=true attribute is present for proper styling of block.
+			if ( $is_amp_carousel ) {
+				$gallery_node->setAttribute( 'data-amp-carousel', 'true' );
+			}
+
 			$img_elements = $node->getElementsByTagName( 'amp-img' );
 
 			$this->process_gallery_embed( $is_amp_carousel, $is_amp_lightbox, $node, $img_elements );

From 9aa0552118e5a4f28ade98c2d81949570cf96a0a Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Thu, 23 Jul 2020 23:51:38 -0700
Subject: [PATCH 37/47] Prevent removal of data attributes in
 sanitize_raw_embeds and fix boolean checks

---
 includes/embeds/class-amp-gallery-embed-handler.php | 7 ++-----
 tests/php/test-amp-gallery-embed-handler.php        | 4 ++--
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/includes/embeds/class-amp-gallery-embed-handler.php b/includes/embeds/class-amp-gallery-embed-handler.php
index 0d2f7357875..8ebff60c3ce 100644
--- a/includes/embeds/class-amp-gallery-embed-handler.php
+++ b/includes/embeds/class-amp-gallery-embed-handler.php
@@ -135,13 +135,10 @@ public function sanitize_raw_embeds( Document $dom ) {
 
 		/** @var DOMElement $node */
 		foreach ( $nodes as $node ) {
-			$is_carousel  = $node->hasAttribute( 'data-amp-carousel' );
-			$is_lightbox  = $node->hasAttribute( 'data-amp-lightbox' );
+			$is_carousel  = $node->hasAttribute( 'data-amp-carousel' ) && rest_sanitize_boolean( $node->getAttribute( 'data-amp-carousel' ) );
+			$is_lightbox  = $node->hasAttribute( 'data-amp-lightbox' ) && rest_sanitize_boolean( $node->getAttribute( 'data-amp-lightbox' ) );
 			$img_elements = $node->getElementsByTagName( 'img' );
 
-			$node->removeAttribute( 'data-amp-carousel' );
-			$node->removeAttribute( 'data-amp-lightbox' );
-
 			$this->process_gallery_embed( $is_carousel, $is_lightbox, $node, $img_elements );
 		}
 	}
diff --git a/tests/php/test-amp-gallery-embed-handler.php b/tests/php/test-amp-gallery-embed-handler.php
index 9fdce6f912b..fb639cf4fbc 100644
--- a/tests/php/test-amp-gallery-embed-handler.php
+++ b/tests/php/test-amp-gallery-embed-handler.php
@@ -112,7 +112,7 @@ public function get_conversion_data() {
 			'shortcode_with_lightbox'                 => [
 				'[gallery amp-lightbox=true amp-carousel=false ids={{id1}},{{id2}},{{id3}}]',
 				'<style type="text/css"> #gallery-10 { margin: auto; } #gallery-10 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-10 img { border: 2px solid #cfcfcf; } #gallery-10 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
-				<div id="gallery-10" class="gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail">
+				<div data-amp-lightbox="true" id="gallery-10" class="gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail">
 					<dl class="gallery-item"><dt class="gallery-icon landscape">
 						<img width="50" height="50" src="{{file1}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-10-{{id1}}" lightbox="">
 					</dt><dd class="wp-caption-text gallery-caption" id="gallery-10-{{id1}}"> ' . self::CAPTION_TEXT . ' </dd></dl>
@@ -128,7 +128,7 @@ public function get_conversion_data() {
 			'shortcode_with_lightbox_linking_to_file' => [
 				'[gallery amp-lightbox=true amp-carousel=false link="file" ids={{id1}},{{id2}},{{id3}}]',
 				'<style type="text/css"> #gallery-12 { margin: auto; } #gallery-12 .gallery-item { float: left; margin-top: 10px; text-align: center; width: 33%; } #gallery-12 img { border: 2px solid #cfcfcf; } #gallery-12 .gallery-caption { margin-left: 0; } /* see gallery_shortcode() in wp-includes/media.php */ </style>
-				<div id="gallery-12" class="gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail">
+				<div data-amp-lightbox="true" id="gallery-12" class="gallery galleryid-0 gallery-columns-3 gallery-size-thumbnail">
 					<dl class="gallery-item"><dt class="gallery-icon landscape">
 						<img width="50" height="50" src="{{file1}}.jpg" class="attachment-thumbnail size-thumbnail" alt="Alt text" ' . $loading_attribute . ' aria-describedby="gallery-12-{{id1}}" lightbox="">
 					</dt><dd class="wp-caption-text gallery-caption" id="gallery-12-{{id1}}"> ' . self::CAPTION_TEXT . ' </dd></dl>

From 4bc1439cc2e50bcfaa816d9dc665ccd14f6ac038 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Fri, 24 Jul 2020 00:17:36 -0700
Subject: [PATCH 38/47] Use DOMElement as return type for
 HasCaption::get_caption_element()

---
 src/Component/HasCaption.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Component/HasCaption.php b/src/Component/HasCaption.php
index d90d37b4f9e..9b63681fe85 100644
--- a/src/Component/HasCaption.php
+++ b/src/Component/HasCaption.php
@@ -7,7 +7,7 @@
 
 namespace AmpProject\AmpWP\Component;
 
-use DOMNode;
+use DOMElement;
 
 /**
  * Interface HasCaption
@@ -20,7 +20,7 @@ interface HasCaption {
 	/**
 	 * Gets the caption node.
 	 *
-	 * @return DOMNode
+	 * @return DOMElement
 	 */
 	public function get_caption_element();
 }

From 6687931e0dbb1c5bc58e6e75202341c594f3e9a5 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Fri, 24 Jul 2020 04:08:08 -0400
Subject: [PATCH 39/47] Modify the block attributes during its deprecation

---
 assets/src/block-editor/helpers/index.js | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index 49752231bb2..33f04de082a 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -103,9 +103,11 @@ export const addAMPAttributes = ( settings, name ) => {
 		if ( ! settings.attributes ) {
 			settings.attributes = {};
 		}
+		// The following attributes have to default to `false` so that the gallery block can be updated successfully if
+		// it has deprecated props.
 		settings.attributes.ampCarousel = {
 			type: 'boolean',
-			default: ! select( 'amp/block-editor' ).hasThemeSupport(), // @todo We could just default this to false now even in Reader mode since block styles are loaded.
+			default: false,
 		};
 		settings.attributes.ampLightbox = {
 			type: 'boolean',
@@ -175,10 +177,11 @@ export const addAMPAttributes = ( settings, name ) => {
  * Remove AMP deprecated props from any blocks which may contain them.
  *
  * @param {Object} settings Block settings.
+ * @param {string} name     Block name.
  *
  * @return {Object} Modified block settings.
  */
-export const removeDeprecatedAmpProps = ( settings ) => {
+export const removeDeprecatedAmpProps = ( settings, name ) => {
 	const maybeBlockHasDeprecatedProp = Object.keys( mappedAttributes )
 		.some( ( attribute ) => Object.keys( settings.attributes ).includes( attribute ) );
 
@@ -205,6 +208,11 @@ export const removeDeprecatedAmpProps = ( settings ) => {
 					return element;
 				}
 
+				// Once we know the block will be updated successfully, we can now modify the block's attributes.
+				if ( 'core/gallery' === name ) {
+					settings.attributes.ampCarousel.default = ! select( 'amp/block-editor' ).hasThemeSupport(); // @todo We could just default this to false now even in Reader mode since block styles are loaded.
+				}
+
 				// Re-render the block with deprecated props.
 				const props = { ...element.props, ...deprecatedProps };
 				element = cloneElement( element, props );

From b2901f0962ee8816232a32f0d45206bc937acd21 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Fri, 24 Jul 2020 11:36:22 -0700
Subject: [PATCH 40/47] Introduce ObsoleteBlockAttributeRemover service

Partially reverts 6687931e
---
 assets/src/block-editor/helpers/index.js |  4 +-
 assets/src/block-editor/index.js         |  2 +-
 src/AmpWpPlugin.php                      |  1 +
 src/ObsoleteBlockAttributeRemover.php    | 97 ++++++++++++++++++++++++
 4 files changed, 100 insertions(+), 4 deletions(-)
 create mode 100644 src/ObsoleteBlockAttributeRemover.php

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index 33f04de082a..bb42e3b3d6a 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -103,11 +103,9 @@ export const addAMPAttributes = ( settings, name ) => {
 		if ( ! settings.attributes ) {
 			settings.attributes = {};
 		}
-		// The following attributes have to default to `false` so that the gallery block can be updated successfully if
-		// it has deprecated props.
 		settings.attributes.ampCarousel = {
 			type: 'boolean',
-			default: false,
+			default: ! select( 'amp/block-editor' ).hasThemeSupport(), // @todo We could just default this to false now even in Reader mode since block styles are loaded.
 		};
 		settings.attributes.ampLightbox = {
 			type: 'boolean',
diff --git a/assets/src/block-editor/index.js b/assets/src/block-editor/index.js
index fd887c31f6c..5f946b4f871 100644
--- a/assets/src/block-editor/index.js
+++ b/assets/src/block-editor/index.js
@@ -29,7 +29,7 @@ plugins.keys().forEach( ( modulePath ) => {
 } );
 
 addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', addAMPAttributes );
-addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/deprecateProps', removeDeprecatedAmpProps );
+//addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/deprecateProps', removeDeprecatedAmpProps );
 addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', filterBlocksSave );
 addFilter( 'editor.BlockEdit', 'ampEditorBlocks/filterEdit', filterBlocksEdit, 20 );
 addFilter( 'editor.PostFeaturedImage', 'ampEditorBlocks/withFeaturedImageNotice', withFeaturedImageNotice );
diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php
index 948202cc90c..f7380983ecc 100644
--- a/src/AmpWpPlugin.php
+++ b/src/AmpWpPlugin.php
@@ -77,6 +77,7 @@ protected function get_service_classes() {
 			'amp_slug_customization_watcher'   => AmpSlugCustomizationWatcher::class,
 			'rest.options_controller'          => OptionsRESTController::class,
 			'server_timing'                    => ServerTiming::class,
+			'obsolete_block_attribute_remover' => ObsoleteBlockAttributeRemover::class,
 		];
 	}
 
diff --git a/src/ObsoleteBlockAttributeRemover.php b/src/ObsoleteBlockAttributeRemover.php
new file mode 100644
index 00000000000..afe8871965e
--- /dev/null
+++ b/src/ObsoleteBlockAttributeRemover.php
@@ -0,0 +1,97 @@
+<?php
+/**
+ * Class ObsoleteBlockAttributeRemover.
+ *
+ * @package AmpProject\AmpWP
+ */
+
+namespace AmpProject\AmpWP;
+
+use AmpProject\AmpWP\Infrastructure\Delayed;
+use AmpProject\AmpWP\Infrastructure\Registerable;
+use AmpProject\AmpWP\Infrastructure\Service;
+use WP_REST_Response;
+
+/**
+ * Removes obsolete data-amp-* attributes from block markup in post content.
+ *
+ * These HTML attributes serve as processing instructions to control how the sanitizers handle converting the HTML to AMP.
+ * For each HTML attribute there is also a block attribute, so if there is a data-amp-carousel HTML attribute then there
+ * is also an ampCarousel block attribute. The block attributes were originally mirrored onto the HTML attributes because
+ * the 'render_block' filter was not available in Gutenberg (or WordPress Core) when this was first implemented; now that
+ * this filter is available, there is no need to duplicate the mirror the attributes, and so they are injected into the
+ * root HTML element via `AMP_Core_Block_Handler::filter_rendered_block()`. In hindsight, instead of having the data
+ * mirrored between block attributes and HTML attributes, the block attributes should have perhaps used an 'attribute'
+ * as the block attribute 'source'. Then again, that may have complicated things yet further to migrate away from using
+ * these data attributes. A key reason for why these HTML data-* attributes are bad is that they cause block validation
+ * errors. If someone creates a Gallery block and enables a carousel, then if they go and deactivate the AMP plugin,
+ * this block will then show as having a block validation error. If, however, we restrict the block attributes to only
+ * be in the block comment, then no block validation errors occurs. Also, since the 'render_block' filter is now
+ * available, the reason for storing these attributes as data-amp-* HTML attributes in post_content is now obsolete.
+ *
+ * @see AMP_Core_Block_Handler::filter_rendered_block()
+ * @link https://github.com/ampproject/amp-wp/pull/4775
+ *
+ * @package AmpProject\AmpWP
+ */
+final class ObsoleteBlockAttributeRemover implements Service, Registerable, Delayed {
+
+	/**
+	 * Obsolete attributes.
+	 *
+	 * @var string[]
+	 */
+	const OBSOLETE_ATTRIBUTES = [
+		'data-amp-carousel',
+		'data-amp-lightbox',
+		'data-amp-noloading',
+		'data-amp-layout',
+	];
+
+	/**
+	 * Pattern matching the obsolete attribute key/value pair in an HTML start tag.
+	 *
+	 * @var string
+	 */
+	protected $obsolete_attribute_pattern;
+
+	/**
+	 * Get registration action.
+	 *
+	 * @return string
+	 */
+	public static function get_registration_action() {
+		return 'rest_api_init';
+	}
+
+	/**
+	 * Register the service with the system.
+	 *
+	 * @return void
+	 */
+	public function register() {
+		$this->obsolete_attribute_pattern = sprintf( '/\s(%s)="[^"]*+"/', implode( '|', self::OBSOLETE_ATTRIBUTES ) );
+		foreach ( get_post_types_by_support( 'editor' ) as $post_type ) {
+			add_filter( "rest_prepare_{$post_type}", [ $this, 'filter_rest_prepare_post' ] );
+		}
+	}
+
+	/**
+	 * Filter post response object to purge obsolete attributes from the raw content.
+	 *
+	 * @param WP_REST_Response $response Response.
+	 * @return WP_REST_Response Response.
+	 */
+	public function filter_rest_prepare_post( WP_REST_Response $response ) {
+		if ( isset( $response->data['content']['raw'] ) ) {
+			$response->data['content']['raw'] = preg_replace_callback(
+				'#(?P<before_html_attrs><!--\s*wp:\w+.*?-->\s*<\w+\S*?)(?P<html_attrs>\s+[^>]*+)\s*>#s',
+				function ( $matches ) {
+					return $matches['before_html_attrs'] . preg_replace( $this->obsolete_attribute_pattern, '', $matches['html_attrs'] ) . '>';
+				},
+				$response->data['content']['raw']
+			);
+		}
+		return $response;
+	}
+}

From e3c7afb7a27acdaf6cf83058fd2e5a00d810b03a Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Fri, 24 Jul 2020 11:41:11 -0700
Subject: [PATCH 41/47] Improve logic for obtaining obsolete attribute pattern

---
 src/ObsoleteBlockAttributeRemover.php | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/ObsoleteBlockAttributeRemover.php b/src/ObsoleteBlockAttributeRemover.php
index afe8871965e..28741f01b45 100644
--- a/src/ObsoleteBlockAttributeRemover.php
+++ b/src/ObsoleteBlockAttributeRemover.php
@@ -43,18 +43,11 @@ final class ObsoleteBlockAttributeRemover implements Service, Registerable, Dela
 	 */
 	const OBSOLETE_ATTRIBUTES = [
 		'data-amp-carousel',
+		'data-amp-layout',
 		'data-amp-lightbox',
 		'data-amp-noloading',
-		'data-amp-layout',
 	];
 
-	/**
-	 * Pattern matching the obsolete attribute key/value pair in an HTML start tag.
-	 *
-	 * @var string
-	 */
-	protected $obsolete_attribute_pattern;
-
 	/**
 	 * Get registration action.
 	 *
@@ -70,12 +63,24 @@ public static function get_registration_action() {
 	 * @return void
 	 */
 	public function register() {
-		$this->obsolete_attribute_pattern = sprintf( '/\s(%s)="[^"]*+"/', implode( '|', self::OBSOLETE_ATTRIBUTES ) );
 		foreach ( get_post_types_by_support( 'editor' ) as $post_type ) {
 			add_filter( "rest_prepare_{$post_type}", [ $this, 'filter_rest_prepare_post' ] );
 		}
 	}
 
+	/**
+	 * Get obsolete attribute regular expression to match the obsolete attribute key/value pair in an HTML start tag..
+	 *
+	 * @return string Regular expression pattern.
+	 */
+	protected function get_obsolete_attribute_pattern() {
+		static $pattern = null;
+		if ( ! $pattern ) {
+			$pattern = sprintf( '/\s(%s)="[^"]*+"/', implode( '|', self::OBSOLETE_ATTRIBUTES ) );
+		}
+		return $pattern;
+	}
+
 	/**
 	 * Filter post response object to purge obsolete attributes from the raw content.
 	 *
@@ -87,7 +92,7 @@ public function filter_rest_prepare_post( WP_REST_Response $response ) {
 			$response->data['content']['raw'] = preg_replace_callback(
 				'#(?P<before_html_attrs><!--\s*wp:\w+.*?-->\s*<\w+\S*?)(?P<html_attrs>\s+[^>]*+)\s*>#s',
 				function ( $matches ) {
-					return $matches['before_html_attrs'] . preg_replace( $this->obsolete_attribute_pattern, '', $matches['html_attrs'] ) . '>';
+					return $matches['before_html_attrs'] . preg_replace( $this->get_obsolete_attribute_pattern(), '', $matches['html_attrs'] ) . '>';
 				},
 				$response->data['content']['raw']
 			);

From 48eb9d1f0b6cfabfe585fdd95b3825673168bbd2 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Fri, 24 Jul 2020 12:25:49 -0700
Subject: [PATCH 42/47] Improve phpdoc for ObsoleteBlockAttributeRemover

---
 src/ObsoleteBlockAttributeRemover.php | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/ObsoleteBlockAttributeRemover.php b/src/ObsoleteBlockAttributeRemover.php
index 28741f01b45..7ab70a87815 100644
--- a/src/ObsoleteBlockAttributeRemover.php
+++ b/src/ObsoleteBlockAttributeRemover.php
@@ -15,21 +15,22 @@
 /**
  * Removes obsolete data-amp-* attributes from block markup in post content.
  *
- * These HTML attributes serve as processing instructions to control how the sanitizers handle converting the HTML to AMP.
+ * These HTML attributes serve as processing instructions to control how the sanitizers handle converting HTML to AMP.
  * For each HTML attribute there is also a block attribute, so if there is a data-amp-carousel HTML attribute then there
  * is also an ampCarousel block attribute. The block attributes were originally mirrored onto the HTML attributes because
  * the 'render_block' filter was not available in Gutenberg (or WordPress Core) when this was first implemented; now that
- * this filter is available, there is no need to duplicate the mirror the attributes, and so they are injected into the
+ * this filter is available, there is no need to duplicate/mirror the attributes, and so they are injected into the
  * root HTML element via `AMP_Core_Block_Handler::filter_rendered_block()`. In hindsight, instead of having the data
  * mirrored between block attributes and HTML attributes, the block attributes should have perhaps used an 'attribute'
  * as the block attribute 'source'. Then again, that may have complicated things yet further to migrate away from using
  * these data attributes. A key reason for why these HTML data-* attributes are bad is that they cause block validation
  * errors. If someone creates a Gallery block and enables a carousel, then if they go and deactivate the AMP plugin,
  * this block will then show as having a block validation error. If, however, we restrict the block attributes to only
- * be in the block comment, then no block validation errors occurs. Also, since the 'render_block' filter is now
- * available, the reason for storing these attributes as data-amp-* HTML attributes in post_content is now obsolete.
+ * be in the block comment, then no block validation errors occur. Also, since the 'render_block' filter is now
+ * available, the reason for storing these block attributes as data-amp-* HTML attributes in post_content is now obsolete.
  *
  * @see AMP_Core_Block_Handler::filter_rendered_block()
+ * @see AMP_Gallery_Block_Sanitizer
  * @link https://github.com/ampproject/amp-wp/pull/4775
  *
  * @package AmpProject\AmpWP

From 1fa592327b63780611cd8e4d433596169f80ba4f Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Fri, 24 Jul 2020 13:21:35 -0700
Subject: [PATCH 43/47] Harden pattern to match block comment and start tag

---
 src/ObsoleteBlockAttributeRemover.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ObsoleteBlockAttributeRemover.php b/src/ObsoleteBlockAttributeRemover.php
index 7ab70a87815..89a3338a117 100644
--- a/src/ObsoleteBlockAttributeRemover.php
+++ b/src/ObsoleteBlockAttributeRemover.php
@@ -91,9 +91,9 @@ protected function get_obsolete_attribute_pattern() {
 	public function filter_rest_prepare_post( WP_REST_Response $response ) {
 		if ( isset( $response->data['content']['raw'] ) ) {
 			$response->data['content']['raw'] = preg_replace_callback(
-				'#(?P<before_html_attrs><!--\s*wp:\w+.*?-->\s*<\w+\S*?)(?P<html_attrs>\s+[^>]*+)\s*>#s',
+				'#(?P<block_comment><!--\s*+wp:\w+.*?-->\s*+)(?P<start_tag><[a-z][a-z0-9_:-]*+\s[^>]*+>)#s',
 				function ( $matches ) {
-					return $matches['before_html_attrs'] . preg_replace( $this->get_obsolete_attribute_pattern(), '', $matches['html_attrs'] ) . '>';
+					return $matches['block_comment'] . preg_replace( $this->get_obsolete_attribute_pattern(), '', $matches['start_tag'] );
 				},
 				$response->data['content']['raw']
 			);

From 974535372bc7913837e3ebb4defdf89c2f299217 Mon Sep 17 00:00:00 2001
From: Pierre Gordon <pierregordon@protonmail.com>
Date: Fri, 24 Jul 2020 16:21:39 -0400
Subject: [PATCH 44/47] Revert block deprecation logic

---
 assets/src/block-editor/helpers/index.js | 65 ------------------------
 assets/src/block-editor/index.js         |  3 +-
 2 files changed, 1 insertion(+), 67 deletions(-)

diff --git a/assets/src/block-editor/helpers/index.js b/assets/src/block-editor/helpers/index.js
index bb42e3b3d6a..76697361e28 100644
--- a/assets/src/block-editor/helpers/index.js
+++ b/assets/src/block-editor/helpers/index.js
@@ -82,13 +82,6 @@ const ampLayoutOptions = [
 	},
 ];
 
-const mappedAttributes = {
-	ampCarousel: 'data-amp-carousel',
-	ampLightbox: 'data-amp-lightbox',
-	ampNoLoading: 'data-amp-noloading',
-	ampLayout: 'data-amp-layout',
-};
-
 /**
  * Add AMP attributes to every core block.
  *
@@ -171,64 +164,6 @@ export const addAMPAttributes = ( settings, name ) => {
 	return settings;
 };
 
-/**
- * Remove AMP deprecated props from any blocks which may contain them.
- *
- * @param {Object} settings Block settings.
- * @param {string} name     Block name.
- *
- * @return {Object} Modified block settings.
- */
-export const removeDeprecatedAmpProps = ( settings, name ) => {
-	const maybeBlockHasDeprecatedProp = Object.keys( mappedAttributes )
-		.some( ( attribute ) => Object.keys( settings.attributes ).includes( attribute ) );
-
-	if ( maybeBlockHasDeprecatedProp ) {
-		// At minimum, the `attributes`, and `save` props are required.
-		const deprecatedConfig = {
-			attributes: settings.attributes,
-			save( blockData ) {
-				const deprecatedProps = {};
-
-				// Render the block.
-				let element = settings.save( blockData );
-				const { attributes: blockAttributes } = blockData;
-
-				// Collect all deprecated props that would be set on the saved HTML block in the post.
-				for ( const [ blockAttributeName, htmlAttributeName ] of Object.entries( mappedAttributes ) ) {
-					if ( blockAttributeName in blockAttributes && blockAttributes[ blockAttributeName ] ) {
-						deprecatedProps[ htmlAttributeName ] = blockAttributes[ blockAttributeName ];
-					}
-				}
-
-				// If there are not any deprecated props, return the unmodified element.
-				if ( 0 === Object.keys( deprecatedProps ).length ) {
-					return element;
-				}
-
-				// Once we know the block will be updated successfully, we can now modify the block's attributes.
-				if ( 'core/gallery' === name ) {
-					settings.attributes.ampCarousel.default = ! select( 'amp/block-editor' ).hasThemeSupport(); // @todo We could just default this to false now even in Reader mode since block styles are loaded.
-				}
-
-				// Re-render the block with deprecated props.
-				const props = { ...element.props, ...deprecatedProps };
-				element = cloneElement( element, props );
-
-				// Gutenberg will now compare the difference and update the block accordingly without the deprecated props.
-				return element;
-			},
-		};
-
-		if ( ! settings.deprecated ) {
-			settings.deprecated = [];
-		}
-		settings.deprecated.unshift( deprecatedConfig );
-	}
-
-	return settings;
-};
-
 /**
  * Filters blocks' save function.
  *
diff --git a/assets/src/block-editor/index.js b/assets/src/block-editor/index.js
index 5f946b4f871..f625e6f2fdd 100644
--- a/assets/src/block-editor/index.js
+++ b/assets/src/block-editor/index.js
@@ -13,7 +13,7 @@ import domReady from '@wordpress/dom-ready';
 import { withFeaturedImageNotice } from '../common/components';
 import { getMinimumFeaturedImageDimensions } from '../common/helpers';
 import { withMediaLibraryNotice, AMPPreview } from './components';
-import { addAMPAttributes, filterBlocksEdit, filterBlocksSave, renderPreviewButton, removeDeprecatedAmpProps } from './helpers';
+import { addAMPAttributes, filterBlocksEdit, filterBlocksSave, renderPreviewButton } from './helpers';
 import './store';
 
 const {
@@ -29,7 +29,6 @@ plugins.keys().forEach( ( modulePath ) => {
 } );
 
 addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', addAMPAttributes );
-//addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/deprecateProps', removeDeprecatedAmpProps );
 addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', filterBlocksSave );
 addFilter( 'editor.BlockEdit', 'ampEditorBlocks/filterEdit', filterBlocksEdit, 20 );
 addFilter( 'editor.PostFeaturedImage', 'ampEditorBlocks/withFeaturedImageNotice', withFeaturedImageNotice );

From 6eb20db2594b59891dca93905fc40daa4e43c250 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Fri, 24 Jul 2020 13:44:07 -0700
Subject: [PATCH 45/47] Skip adding data attributes for gallery shortcodes

---
 includes/embeds/class-amp-core-block-handler.php | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php
index b0fd39a20a7..bd4e27430d1 100644
--- a/includes/embeds/class-amp-core-block-handler.php
+++ b/includes/embeds/class-amp-core-block-handler.php
@@ -76,14 +76,17 @@ public function unregister_embed() {
 	 * @return string Filtered block content.
 	 */
 	public function filter_rendered_block( $block_content, $block ) {
+		if ( ! isset( $block['blockName'] ) ) {
+			return $block_content;
+		}
 
-		if ( isset( $block['attrs'] ) ) {
+		if ( isset( $block['attrs'] ) && 'core/shortcode' !== $block['blockName'] ) {
 			$injected_attributes    = '';
 			$prop_attribute_mapping = [
+				'ampCarousel'  => 'data-amp-carousel',
 				'ampLayout'    => 'data-amp-layout',
-				'ampNoLoading' => 'data-amp-noloading',
 				'ampLightbox'  => 'data-amp-lightbox',
-				'ampCarousel'  => 'data-amp-carousel',
+				'ampNoLoading' => 'data-amp-noloading',
 			];
 			foreach ( $prop_attribute_mapping as $prop => $attr ) {
 				if ( isset( $block['attrs'][ $prop ] ) ) {
@@ -100,9 +103,6 @@ public function filter_rendered_block( $block_content, $block ) {
 			}
 		}
 
-		if ( ! isset( $block['blockName'] ) ) {
-			return $block_content;
-		}
 		if ( isset( $this->block_ampify_methods[ $block['blockName'] ] ) ) {
 			$method_name   = $this->block_ampify_methods[ $block['blockName'] ];
 			$block_content = $this->{$method_name}( $block_content, $block );

From 88d5ae975642c6963a9d86d833243f0f5cd497f9 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Fri, 24 Jul 2020 14:25:58 -0700
Subject: [PATCH 46/47] Add tests for ObsoleteBlockAttributeRemover

---
 .../src/ObsoleteBlockAttributeRemoverTest.php | 149 ++++++++++++++++++
 1 file changed, 149 insertions(+)
 create mode 100644 tests/php/src/ObsoleteBlockAttributeRemoverTest.php

diff --git a/tests/php/src/ObsoleteBlockAttributeRemoverTest.php b/tests/php/src/ObsoleteBlockAttributeRemoverTest.php
new file mode 100644
index 00000000000..1bb87d9bb2f
--- /dev/null
+++ b/tests/php/src/ObsoleteBlockAttributeRemoverTest.php
@@ -0,0 +1,149 @@
+<?php
+
+namespace AmpProject\AmpWP\Tests;
+
+use AmpProject\AmpWP\Infrastructure\Delayed;
+use AmpProject\AmpWP\Infrastructure\Registerable;
+use AmpProject\AmpWP\Infrastructure\Service;
+use AmpProject\AmpWP\ObsoleteBlockAttributeRemover;
+use AmpProject\AmpWP\Tests\Helpers\AssertContainsCompatibility;
+use WP_UnitTestCase;
+use WP_REST_Response;
+
+/** @covers ObsoleteBlockAttributeRemover */
+final class ObsoleteBlockAttributeRemoverTest extends WP_UnitTestCase {
+
+	use AssertContainsCompatibility;
+
+	/** @var ObsoleteBlockAttributeRemover */
+	private $instance;
+
+	const PROP_ATTRIBUTE_MAPPING = [
+		'ampCarousel'  => 'data-amp-carousel',
+		'ampLayout'    => 'data-amp-layout',
+		'ampLightbox'  => 'data-amp-lightbox',
+		'ampNoLoading' => 'data-amp-noloading',
+	];
+
+	public function setUp() {
+		parent::setUp();
+		$this->instance = new ObsoleteBlockAttributeRemover();
+	}
+
+	/** @covers ObsoleteBlockAttributeRemover::__construct() */
+	public function test_it_can_be_initialized() {
+		$this->assertInstanceOf( Service::class, $this->instance );
+		$this->assertInstanceOf( Registerable::class, $this->instance );
+		$this->assertInstanceOf( Delayed::class, $this->instance );
+	}
+
+	/** @covers ObsoleteBlockAttributeRemover::get_registration_action() */
+	public function test_get_registration_action() {
+		$this->assertEquals( 'rest_api_init', ObsoleteBlockAttributeRemover::get_registration_action() );
+	}
+
+	/** @covers ObsoleteBlockAttributeRemover::register() */
+	public function test_register() {
+		$editor_post_types = get_post_types_by_support( 'editor' );
+		$this->assertNotEmpty( $editor_post_types );
+		$this->instance->register();
+		foreach ( $editor_post_types as $editor_post_type ) {
+			$this->assertEquals( 10, has_filter( "rest_prepare_{$editor_post_type}", [ $this->instance, 'filter_rest_prepare_post' ] ) );
+		}
+	}
+
+	/** @return array */
+	public function get_block_data() {
+		return [
+			'gallery_carousel_lightbox' => [
+				'
+				<!-- wp:gallery {"ids":[1869,770,760],"ampCarousel":true,"ampLightbox":true} -->
+				<figure class="wp-block-gallery columns-3 is-cropped" data-amp-lightbox="true" data-amp-carousel="true"><ul class="blocks-gallery-grid"><li class="blocks-gallery-item"><figure><img src="https://wordpressdev.lndo.site/content/uploads/2020/07/American_bison_k5680-1-1024x668.jpg" alt="" data-id="1869" data-full-url="https://wordpressdev.lndo.site/content/uploads/2020/07/American_bison_k5680-1-scaled.jpg" data-link="https://wordpressdev.lndo.site/2020/07/12/implement-transformer-in-optimizer-for-media-sizes-and-heights/american_bison_k5680-1/" class="wp-image-1869"/></figure></li><li class="blocks-gallery-item"><figure><img src="https://wordpressdev.lndo.site/content/uploads/2011/07/img_0767.jpg" alt="Huatulco Coastline" data-id="770" data-full-url="https://wordpressdev.lndo.site/content/uploads/2011/07/img_0767.jpg" data-link="https://wordpressdev.lndo.site/2010/09/10/post-format-gallery/img_0767/" class="wp-image-770"/><figcaption class="blocks-gallery-item__caption">Coastline in Huatulco, Oaxaca, Mexico</figcaption></figure></li><li class="blocks-gallery-item"><figure><img src="https://wordpressdev.lndo.site/content/uploads/2011/07/dsc09114-1024x768.jpg" alt="Sydney Harbor Bridge" data-id="760" data-full-url="https://wordpressdev.lndo.site/content/uploads/2011/07/dsc09114.jpg" data-link="https://wordpressdev.lndo.site/2010/09/10/post-format-gallery/dsc09114/" class="wp-image-760"/><figcaption class="blocks-gallery-item__caption">Sydney Harbor Bridge</figcaption></figure></li></ul><figcaption class="blocks-gallery-caption">Carousel and Lightbox</figcaption></figure>
+				<!-- /wp:gallery -->
+				',
+				2,
+			],
+			'image_block_multi_attrs'   => [
+				'
+				<!-- wp:image {"id":1045,"width":336,"height":221,"sizeSlug":"large","ampLightbox":true,"ampLayout":"fixed","ampNoLoading":true} -->
+				<figure class="wp-block-image size-large is-resized" data-amp-layout="fixed" data-amp-noloading="true" data-amp-lightbox="true"><img src="https://wordpressdev.lndo.site/content/uploads/2013/03/unicorn-wallpaper-1024x768.jpg" alt="Unicorn Wallpaper" class="wp-image-1045" width="336" height="221"/><figcaption>Noloading and lightbox and fixed layout</figcaption></figure>
+				<!-- /wp:image -->
+				',
+				3,
+			],
+			'image_block'               => [
+				'
+				<!-- wp:image {"id":1869,"width":354,"height":233,"sizeSlug":"large","ampLayout":"fixed"} -->
+				<figure class="wp-block-image size-large is-resized" data-amp-layout="fixed"><img src="https://wordpressdev.lndo.site/content/uploads/2020/07/American_bison_k5680-1-1024x668.jpg" alt="" class="wp-image-1869" width="354" height="233"/><figcaption>Fixed layout</figcaption></figure>
+				<!-- /wp:image -->
+				',
+				1, // Expect props.
+			],
+			'amp_fit_text'              => [
+				'
+				<!-- wp:paragraph {"ampFitText":true} -->
+				<amp-fit-text layout="fixed-height" min-font-size="6" max-font-size="72" height="100"><p>Here is amp-fit-text!</p></amp-fit-text>
+				<!-- /wp:paragraph -->
+				',
+				0, // Expect props.
+			],
+		];
+	}
+
+	/**
+	 * @covers ObsoleteBlockAttributeRemover::test_filter_rest_prepare_post()
+	 *
+	 * @dataProvider get_block_data
+	 * @param string $block_content
+	 * @param int    $expected_prop_count
+	 */
+	public function test_filter_rest_prepare_post_raw( $block_content, $expected_prop_count ) {
+		$block_content = str_replace( "\t", '', trim( $block_content ) );
+
+		$parsed_blocks = parse_blocks( $block_content );
+		$this->assertCount( 1, $parsed_blocks );
+		$parsed_block = array_shift( $parsed_blocks );
+
+		$response = new WP_REST_Response(
+			[
+				'content' => [
+					'raw'      => $block_content,
+					'rendered' => preg_replace( '/<!--.+?-->/s', '', $block_content ),
+				],
+			]
+		);
+
+		$filtered_response = $this->instance->filter_rest_prepare_post( clone $response );
+		if ( $expected_prop_count > 0 ) {
+			$this->assertNotEquals( $response->data['content']['raw'], $filtered_response->data['content']['raw'] );
+		} else {
+			$this->assertEquals( $response->data['content']['raw'], $filtered_response->data['content']['raw'] );
+		}
+		$this->assertEquals( $response->data['content']['rendered'], $filtered_response->data['content']['rendered'] );
+
+		$present_count = 0;
+		foreach ( self::PROP_ATTRIBUTE_MAPPING as $prop => $attribute ) {
+			if ( isset( $parsed_block['attrs'][ $prop ] ) ) {
+				$this->assertStringContains( "$attribute=", $response->data['content']['raw'] );
+				$this->assertStringNotContains( "$attribute=", $filtered_response->data['content']['raw'] );
+				$present_count++;
+			}
+		}
+
+		$this->assertEquals( $expected_prop_count, $present_count );
+	}
+
+	/** @covers ObsoleteBlockAttributeRemover::test_filter_rest_prepare_post() */
+	public function test_filter_rest_prepare_post_rendered_only() {
+		$response = new WP_REST_Response(
+			[
+				'content' => [
+					'rendered' => '<amp-fit-text layout="fixed-height" min-font-size="6" max-font-size="72" height="100"><p>Here is amp-fit-text!</p></amp-fit-text>',
+				],
+			]
+		);
+
+		$filtered_response = $this->instance->filter_rest_prepare_post( clone $response );
+		$this->assertEquals( $response->data['content']['rendered'], $filtered_response->data['content']['rendered'] );
+	}
+}

From 9d7260fa1a2698aa031632a7351aef9d93f72607 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Fri, 24 Jul 2020 14:56:51 -0700
Subject: [PATCH 47/47] Fix test failure in WP 4.9

---
 tests/php/src/ObsoleteBlockAttributeRemoverTest.php | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/php/src/ObsoleteBlockAttributeRemoverTest.php b/tests/php/src/ObsoleteBlockAttributeRemoverTest.php
index 1bb87d9bb2f..613a3695e9e 100644
--- a/tests/php/src/ObsoleteBlockAttributeRemoverTest.php
+++ b/tests/php/src/ObsoleteBlockAttributeRemoverTest.php
@@ -98,6 +98,9 @@ public function get_block_data() {
 	 * @param int    $expected_prop_count
 	 */
 	public function test_filter_rest_prepare_post_raw( $block_content, $expected_prop_count ) {
+		if ( ! function_exists( 'parse_blocks' ) ) {
+			$this->markTestSkipped();
+		}
 		$block_content = str_replace( "\t", '', trim( $block_content ) );
 
 		$parsed_blocks = parse_blocks( $block_content );