Merge #1026

ampproject · May 16, 2018 · dd924ec · dd924ec
2 parents c3aad13 + 589bf18
commit dd924ec
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 172 deletions.
diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js
@@ -43,6 +43,7 @@ var ampEditorBlocks = ( function() {
 		wp.hooks.addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', component.addAMPAttributes );
 		wp.hooks.addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', component.filterBlocksSave );
 		wp.hooks.addFilter( 'blocks.BlockEdit', 'ampEditorBlocks/filterEdit', component.filterBlocksEdit );
+		wp.hooks.addFilter( 'blocks.getSaveContent.extraProps', 'ampEditorBlocks/addExtraAttributes', component.addAMPExtraProps );
 	};
 
 	/**
@@ -96,6 +97,30 @@ var ampEditorBlocks = ( function() {
 		return layoutOptions;
 	};
 
+	/**
+	 * Add extra data-amp-layout attribute to save to DB.
+	 *
+	 * @param {Object} props Properties.
+	 * @param {string} blockType Block type.
+	 * @param {Object} attributes Attributes.
+	 * @return {*} Props.
+	 */
+	component.addAMPExtraProps = function addAMPExtraProps( props, blockType, attributes ) {
+		var ampAttributes = {};
+		if ( ! attributes.ampLayout && ! attributes.ampNoLoading ) {
+			return props;
+		}
+
+		if ( attributes.ampLayout ) {
+			ampAttributes[ 'data-amp-layout' ] = attributes.ampLayout;
+		}
+		if ( attributes.ampNoLoading ) {
+			ampAttributes[ 'data-amp-noloading' ] = attributes.ampNoLoading;
+		}
+
+		return _.assign( ampAttributes, props );
+	};
+
 	/**
 	 * Add AMP attributes (in this test case just ampLayout) to every core block.
 	 *
@@ -159,11 +184,6 @@ var ampEditorBlocks = ( function() {
 
 			ampLayout = attributes.ampLayout;
 
-			// Lets remove amp-related classes from edit view.
-			if ( component.hasClassAmpAttributes( attributes.className || '' ) ) {
-				props.setAttributes( { className: component.removeAmpAttributesFromClassName( attributes.className ) } );
-			}
-
 			if ( 'core/shortcode' === name ) {
 				// Lets remove amp-carousel from edit view.
 				if ( component.hasGalleryShortcodeCarouselAttribute( attributes.text || '' ) ) {
@@ -444,10 +464,7 @@ var ampEditorBlocks = ( function() {
 	 * @return {*} Output element.
 	 */
 	component.filterBlocksSave = function filterBlocksSave( element, blockType, attributes ) {
-		var text,
-			ampClassName = element.props.className || '',
-			props = element.props;
-
+		var text;
 		if ( 'core/shortcode' === blockType.name && component.isGalleryShortcode( attributes ) ) {
 			if ( ! attributes.ampLightbox ) {
 				if ( component.hasGalleryShortcodeLightboxAttribute( attributes.text || '' ) ) {
@@ -494,29 +511,7 @@ var ampEditorBlocks = ( function() {
 				);
 			}
 
-			return element;
-		}
-
-		// In case AMP attributes, add info to classname.
-		if ( component.hasAmpLayoutSet( attributes || '' ) ) {
-			ampClassName += ' amp-layout-' + attributes.ampLayout;
-		}
-		if ( component.hasAmpNoLoadingSet( attributes || '' ) ) {
-			ampClassName += ' amp-noloading';
-		}
-		if ( component.hasAmpCarouselSet( attributes || '' ) ) {
-			ampClassName += ' amp-carousel';
-		}
-		if ( component.hasAmpLightboxSet( attributes || '' ) ) {
-			ampClassName += ' amp-lightbox';
-		}
 
-		if ( '' !== ampClassName && attributes.className !== ampClassName ) {
-			props = _.extend( {}, props, { className: ampClassName.trim() } );
-			return wp.element.createElement(
-				element.type,
-				props
-			);
 		}
 		return element;
 	};
@@ -601,33 +596,6 @@ var ampEditorBlocks = ( function() {
 		return -1 !== text.indexOf( 'amp-lightbox=true' );
 	};
 
-	/**
-	 * Check if className has AMP attributes in it.
-	 *
-	 * @param {string} className Classname.
-	 * @return {boolean} If has attributes.
-	 */
-	component.hasClassAmpAttributes = function hasClassAmpAttributes( className ) {
-		return -1 !== className.indexOf( 'amp-' );
-	};
-
-	/**
-	 * Remove AMP related attributes from classname.
-	 *
-	 * @param {string} className Original className.
-	 * @return {string} Modified className.
-	 */
-	component.removeAmpAttributesFromClassName = function removeAmpAttributesFromClassName( className ) {
-		var splits = className.split( ' ' );
-		var modifiedClass = '';
-		_.each( splits, function( split ) {
-			if ( -1 === split.indexOf( 'amp-' ) ) {
-				modifiedClass += ' ' + split;
-			}
-		} );
-		return modifiedClass;
-	};
-
 	/**
 	 * Check if shortcode is gallery shortcode.
 	 *

diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php
@@ -21,75 +21,6 @@ public function init() {
 		}
 	}
 
-	/**
-	 * Whenever a post type is registered, hook into saving the post.
-	 *
-	 * @param string $post_type The newly registered post type.
-	 * @return string That same post type.
-	 */
-	public static function register_post_pre_insert_functions( $post_type ) {
-		add_filter( "rest_pre_insert_{$post_type}", 'AMP_Editor_Blocks::filter_rest_pre_insert_post', 11, 1 );
-		return $post_type;
-	}
-
-	/**
-	 * Filter pre_insert_post to ensure className is saved to post_content, too.
-	 *
-	 * @param WP_Post $prepared_post Prepared post.
-	 * @return mixed Prepared post.
-	 */
-	public static function filter_rest_pre_insert_post( $prepared_post ) {
-		if ( ! class_exists( 'Gutenberg_PEG_Parser' ) ) {
-			return $prepared_post;
-		}
-		$parser = new Gutenberg_PEG_Parser();
-		$blocks = $parser->parse( _gutenberg_utf8_split( $prepared_post->post_content ) );
-		foreach ( $blocks as $block ) {
-			if ( isset( $block['innerHTML'] ) ) {
-
-				// Get the class of the figure.
-				preg_match( "'<figure class=\"(.*?)\"'si", $block['innerHTML'], $match );
-
-				// Lets check for gallery block class.
-				if ( empty( $match ) ) {
-					preg_match( "'<ul class=\"(.*?)\"'si", $block['innerHTML'], $match );
-				}
-
-				if ( empty( $match ) ) {
-					continue;
-				}
-				$class_names = explode( ' ', $match[1] );
-				$class_attr  = array();
-				if ( isset( $block['attrs']['className'] ) ) {
-					$class_attr[] = $block['attrs']['className'];
-				}
-
-				// Take everything with amp-*.
-				foreach ( $class_names as $class_name ) {
-					if ( false !== strpos( $class_name, 'amp-' ) ) {
-						$class_attr[] = $class_name;
-					}
-				}
-
-				if ( empty( $class_attr ) ) {
-					continue;
-				}
-				$new_attributes = wp_json_encode( array_merge(
-					$block['attrs'],
-					array(
-						'className' => implode( ' ', $class_attr ),
-					)
-				), 64 /* JSON_UNESCAPED_SLASHES */ );
-				$to_replace     = wp_json_encode( $block['attrs'], 64 /* JSON_UNESCAPED_SLASHES */ );
-
-				// Replace the classname attribute with the modified one.
-				$content                     = str_replace( $to_replace, $new_attributes, $prepared_post->post_content );
-				$prepared_post->post_content = $content;
-			}
-		}
-		return $prepared_post;
-	}
-
 	/**
 	 * Whitelist used data-amp-* attributes.
 	 *
@@ -100,6 +31,7 @@ public function whitelist_block_atts_in_wp_kses_allowed_html( $context ) {
 		foreach ( $context as $tag ) {
 			$tag['data-amp-layout']              = true;
 			$tag['data-amp-noloading']           = true;
+			$tag['data-amp-lightbox']            = true;
 			$tag['data-close-button-aria-label'] = true;
 		}
 		return $context;

diff --git a/includes/admin/functions.php b/includes/admin/functions.php
@@ -161,6 +161,3 @@ function amp_editor_core_blocks() {
 	$editor_blocks = new AMP_Editor_Blocks();
 	$editor_blocks->init();
 }
-
-// Add filter for registering posts.
-add_filter( 'registered_post_type', 'AMP_Editor_Blocks::register_post_pre_insert_functions', 11 );
diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php
@@ -368,18 +368,14 @@ public function get_data_amp_attributes( $node ) {
 		$parent_node = $node->parentNode;
 		if ( 'figure' === $parent_node->tagName ) {
 			$parent_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $parent_node );
-			if ( isset( $parent_attributes['class'] ) ) {
-				$classes = explode( ' ', $parent_attributes['class'] );
-
-				foreach ( $classes as $class_name ) {
-					if ( 0 === strpos( $class_name, 'amp-layout-' ) ) {
-						$attributes['layout'] = str_replace( 'amp-layout-', '', $class_name );
-					} elseif ( 'amp-noloading' === $class_name ) {
-						$attributes['noloading'] = true;
-					} elseif ( 'amp-lightbox' === $class_name ) {
-						$attributes['lightbox'] = true;
-					}
-				}
+			if ( isset( $parent_attributes['data-amp-layout'] ) ) {
+				$attributes['layout'] = $parent_attributes['data-amp-layout'];
+			}
+			if ( isset( $parent_attributes['data-amp-noloading'] ) && true === filter_var( $parent_attributes['data-amp-noloading'], FILTER_VALIDATE_BOOLEAN ) ) {
+				$attributes['noloading'] = $parent_attributes['data-amp-noloading'];
+			}
+			if ( isset( $parent_attributes['data-amp-lightbox'] ) && true === filter_var( $parent_attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN ) ) {
+				$attributes['lightbox'] = true;
 			}
 		}
 

diff --git a/includes/sanitizers/class-amp-block-sanitizer.php b/includes/sanitizers/class-amp-block-sanitizer.php
@@ -47,8 +47,12 @@ public function sanitize() {
 				continue;
 			}
 
-			// We are looking for <figure> elements with AMP classes only.
-			if ( false === strpos( $attributes['class'], 'amp-layout-' ) && false === strpos( $attributes['class'], 'amp-noloading' ) ) {
+			// We are looking for <figure> elements with layout attribute only.
+			if (
+				! isset( $attributes['data-amp-layout'] ) &&
+				! isset( $attributes['data-amp-noloading'] ) &&
+				! isset( $attributes['data-amp-lightbox'] )
+			) {
 				continue;
 			}
 
@@ -81,14 +85,14 @@ public function sanitize() {
 	 */
 	protected function set_attributes( $node, $parent_node, $attributes ) {
 
-		$classes = explode( ' ', $attributes['class'] );
-
-		foreach ( $classes as $class_name ) {
-			if ( 0 === strpos( $class_name, 'amp-layout-' ) ) {
-				$node->setAttribute( 'layout', str_replace( 'amp-layout-', '', $class_name ) );
-			} elseif ( 'amp-noloading' === $class_name ) {
-				$node->setAttribute( 'noloading', '' );
-			}
+		if ( isset( $attributes['data-amp-layout'] ) ) {
+			$node->setAttribute( 'layout', $attributes['data-amp-layout'] );
+		}
+		if ( isset( $attributes['data-amp-noloading'] ) && true === filter_var( $attributes['data-amp-noloading'], FILTER_VALIDATE_BOOLEAN ) ) {
+			$node->setAttribute( 'noloading', '' );
+		}
+		if ( isset( $attributes['data-amp-lightbox'] ) && true === filter_var( $attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN ) ) {
+			$node->setAttribute( 'lightbox', '' );
 		}
 
 		$layout = $node->getAttribute( 'layout' );

diff --git a/tests/test-amp-audio-converter.php b/tests/test-amp-audio-converter.php
@@ -60,8 +60,8 @@ public function get_data() {
 			),
 
 			'audio_with_layout_from_editor_fixed_height' => array(
-				'<figure class="amp-layout-fixed-height"><audio src="https://example.com/audio/file.ogg" width="100" height="100"></audio></figure>',
-				'<figure class="amp-layout-fixed-height"><amp-audio src="https://example.com/audio/file.ogg" width="auto" height="100" layout="fixed-height"></amp-audio></figure>',
+				'<figure data-amp-layout="fixed-height"><audio src="https://example.com/audio/file.ogg" width="100" height="100"></audio></figure>',
+				'<figure data-amp-layout="fixed-height"><amp-audio src="https://example.com/audio/file.ogg" width="auto" height="100" layout="fixed-height"></amp-audio></figure>',
 			),
 
 			'multiple_same_audio' => array(

diff --git a/tests/test-amp-img-sanitizer.php b/tests/test-amp-img-sanitizer.php
@@ -1,14 +1,14 @@
 <?php
 /**
- * Tests AMP_Style_Sanitizer.
+ * Class AMP_Img_Sanitizer_Test
  *
  * @package AMP
  */
 
 /**
  * Class AMP_Img_Sanitizer_Test
  *
- * @covers AMP_Style_Sanitizer
+ * @covers AMP_Img_Sanitizer
  */
 class AMP_Img_Sanitizer_Test extends WP_UnitTestCase {
 
@@ -58,13 +58,13 @@ public function get_data() {
 			),
 
 			'image_with_layout_from_editor'            => array(
-				'<figure class="amp-layout-fill"><img src="http://placehold.it/300x300" height="300" width="300" /></figure>',
-				'<figure class="amp-layout-fill" style="position:relative; width: 100%; height: 300px;"><amp-img src="http://placehold.it/300x300" layout="fill" class="amp-wp-enforced-sizes"></amp-img></figure>',
+				'<figure data-amp-layout="fill"><img src="http://placehold.it/300x300" height="300" width="300" /></figure>',
+				'<figure data-amp-layout="fill" style="position:relative; width: 100%; height: 300px;"><amp-img src="http://placehold.it/300x300" layout="fill" class="amp-wp-enforced-sizes"></amp-img></figure>',
 			),
 
 			'image_with_noloading_from_editor'         => array(
-				'<figure class="amp-noloading"><img src="http://placehold.it/300x300" height="300" width="300" /></figure>',
-				'<figure class="amp-noloading"><amp-img src="http://placehold.it/300x300" height="300" width="300" noloading="" class="amp-wp-enforced-sizes" layout="intrinsic"></amp-img></figure>',
+				'<figure data-amp-noloading="true"><img src="http://placehold.it/300x300" height="300" width="300" /></figure>',
+				'<figure data-amp-noloading="true"><amp-img src="http://placehold.it/300x300" height="300" width="300" noloading="" class="amp-wp-enforced-sizes" layout="intrinsic"></amp-img></figure>',
 			),
 
 			'image_with_spaces_only_src'               => array(

diff --git a/tests/test-amp-video-sanitizer.php b/tests/test-amp-video-sanitizer.php
@@ -59,18 +59,18 @@ public function get_data() {
 			),
 
 			'video_with_layout_from_editor_fill' => array(
-				'<figure class="amp-layout-fill"><video src="https://example.com/file.mp4" height="100" width="100"></video></figure>',
-				'<figure class="amp-layout-fill" style="position:relative; width: 100%; height: 100px;"><amp-video src="https://example.com/file.mp4" layout="fill"></amp-video></figure>',
+				'<figure data-amp-layout="fill"><video src="https://example.com/file.mp4" height="100" width="100"></video></figure>',
+				'<figure data-amp-layout="fill" style="position:relative; width: 100%; height: 100px;"><amp-video src="https://example.com/file.mp4" layout="fill"></amp-video></figure>',
 			),
 
 			'video_with_layout_from_editor_fixed' => array(
-				'<figure class="amp-layout-fixed"><video src="https://example.com/file.mp4" width="100"></video></figure>',
-				'<figure class="amp-layout-fixed"><amp-video src="https://example.com/file.mp4" width="100" layout="fixed" height="400"></amp-video></figure>',
+				'<figure data-amp-layout="fixed"><video src="https://example.com/file.mp4" width="100"></video></figure>',
+				'<figure data-amp-layout="fixed"><amp-video src="https://example.com/file.mp4" width="100" layout="fixed" height="400"></amp-video></figure>',
 			),
 
 			'video_with_noloading_from_editor' => array(
-				'<figure class="amp-noloading"><video src="https://example.com/file.mp4" height="100" width="100"></video></figure>',
-				'<figure class="amp-noloading"><amp-video src="https://example.com/file.mp4" height="100" width="100" noloading="" layout="responsive"></amp-video></figure>',
+				'<figure data-amp-noloading="true"><video src="https://example.com/file.mp4" height="100" width="100"></video></figure>',
+				'<figure data-amp-noloading="true"><amp-video src="https://example.com/file.mp4" height="100" width="100" noloading="" layout="responsive"></amp-video></figure>',
 			),
 
 			'multiple_same_video' => array(

diff --git a/tests/test-class-amp-base-sanitizer.php b/tests/test-class-amp-base-sanitizer.php
@@ -256,14 +256,15 @@ public function test_get_data_amp_attributes() {
 		$figure       = $dom_document->createElement( $tag );
 		$amp_img      = $dom_document->createElement( 'amp-img' );
 		$figure->appendChild( $amp_img );
-		$figure->setAttribute( 'class', 'amp-noloading amp-layout-fixed' );
+		$figure->setAttribute( 'data-amp-noloading', 'true' );
+		$figure->setAttribute( 'data-amp-layout', 'fixed' );
 
 		$sanitizer = new AMP_Test_Stub_Sanitizer( new DOMDocument(), array() );
 		$amp_args  = $sanitizer->get_data_amp_attributes( $amp_img );
 
 		$expected_args = array(
 			'layout'    => 'fixed',
-			'noloading' => true,
+			'noloading' => 'true',
 		);
 
 		$this->assertEquals( $expected_args, $amp_args );

diff --git a/tests/test-class-amp-block-sanitizer.php b/tests/test-class-amp-block-sanitizer.php
@@ -33,13 +33,13 @@ public function get_data() {
 			),
 
 			'data_amp_noloading'   => array(
-				'<figure class="wp-block-embed amp-noloading"><amp-facebook></amp-facebook></figure>',
-				'<figure class="wp-block-embed amp-noloading"><amp-facebook noloading="" layout="fixed-height"></amp-facebook></figure>',
+				'<figure class="wp-block-embed" data-amp-noloading="true"><amp-facebook></amp-facebook></figure>',
+				'<figure class="wp-block-embed" data-amp-noloading="true"><amp-facebook noloading="" layout="fixed-height"></amp-facebook></figure>',
 			),
 
 			'data_amp_layout_fill' => array(
-				'<figure class="wp-block-embed amp-layout-fill"><amp-facebook width="100"></amp-facebook></figure>',
-				'<figure class="wp-block-embed amp-layout-fill" style="position:relative; width: 100%; height: 400px;"><amp-facebook layout="fill"></amp-facebook></figure>',
+				'<figure class="wp-block-embed" data-amp-layout="fill"><amp-facebook width="100"></amp-facebook></figure>',
+				'<figure class="wp-block-embed" data-amp-layout="fill" style="position:relative; width: 100%; height: 400px;"><amp-facebook layout="fill"></amp-facebook></figure>',
 			),
 		);
 	}