Validate height, width & layout attributes

includes/amp-helper-functions.php

@@ -916,6 +916,7 @@ function amp_get_content_sanitizers( $post = null ) {
 			'include_manifest_comment' => ( defined( 'WP_DEBUG' ) && WP_DEBUG ) ? 'always' : 'when_excessive',
 		],
 		'AMP_Meta_Sanitizer'              => [],
+		'AMP_Layout_Sanitizer'            => [],
 		'AMP_Tag_And_Attribute_Sanitizer' => [], // Note: This whitelist sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch.
 	];
 

includes/class-amp-autoloader.php

@@ -88,6 +88,7 @@ class AMP_Autoloader {
 		'AMP_Style_Sanitizer'                => 'includes/sanitizers/class-amp-style-sanitizer',
 		'AMP_Script_Sanitizer'               => 'includes/sanitizers/class-amp-script-sanitizer',
 		'AMP_Embed_Sanitizer'                => 'includes/sanitizers/class-amp-embed-sanitizer',
+		'AMP_Layout_Sanitizer'               => 'includes/sanitizers/class-amp-layout-sanitizer',
 		'AMP_Tag_And_Attribute_Sanitizer'    => 'includes/sanitizers/class-amp-tag-and-attribute-sanitizer',
 		'AMP_Video_Sanitizer'                => 'includes/sanitizers/class-amp-video-sanitizer',
 		'AMP_Core_Theme_Sanitizer'           => 'includes/sanitizers/class-amp-core-theme-sanitizer',
@@ -104,6 +105,7 @@ class AMP_Autoloader {
 		'AMP_Validation_Callback_Wrapper'    => 'includes/validation/class-amp-validation-callback-wrapper',
 		'AMP_Validated_URL_Post_Type'        => 'includes/validation/class-amp-validated-url-post-type',
 		'AMP_Validation_Error_Taxonomy'      => 'includes/validation/class-amp-validation-error-taxonomy',
+		'AMP_CSS_Length'                     => 'includes/validation/class-amp-css-length',
 		'AMP_CLI_Namespace'                  => 'includes/cli/class-amp-cli-namespace',
 		'AMP_CLI_Validation_Command'         => 'includes/cli/class-amp-cli-validation-command',
 		'AMP_String_Utils'                   => 'includes/utils/class-amp-string-utils',

includes/embeds/class-amp-core-block-handler.php

@@ -21,6 +21,7 @@ class AMP_Core_Block_Handler extends AMP_Base_Embed_Handler {
 		'core/categories' => 'ampify_categories_block',
 		'core/archives'   => 'ampify_archives_block',
 		'core/video'      => 'ampify_video_block',
+		'core/cover'      => 'ampify_cover_block',
 	];
 
 	/**
@@ -155,4 +156,26 @@ public function ampify_video_block( $block_content, $block ) {
 
 		return $block_content;
 	}
+
+	/**
+	 * Ampify cover block.
+	 *
+	 * This specifically fixes the layout of the block when a background video is assigned.
+	 *
+	 * @see \AMP_Video_Sanitizer::filter_video_dimensions()
+	 *
+	 * @param string $block_content The block content about to be appended.
+	 * @param array  $block         The full block, including name and attributes.
+	 * @return string Filtered block content.
+	 */
+	public function ampify_cover_block( $block_content, $block ) {
+		if ( isset( $block['attrs']['backgroundType'] ) && 'video' === $block['attrs']['backgroundType'] ) {
+			$block_content = preg_replace(
+				'/(?<=<video\s)/',
+				'layout="fill" object-fit="cover" ',
+				$block_content
+			);
+		}
+		return $block_content;
+	}
 }

includes/embeds/class-amp-facebook-embed.php

@@ -191,14 +191,12 @@ private function get_embed_type( DOMElement $node ) {
 	private function create_amp_facebook_and_replace_node( Document $dom, DOMElement $node, $embed_type ) {
 
 		$attributes = [
-			'layout' => 'responsive',
-			'width'  => $node->hasAttribute( 'data-width' ) ? $node->getAttribute( 'data-width' ) : $this->DEFAULT_WIDTH,
-			'height' => $node->hasAttribute( 'data-height' ) ? $node->getAttribute( 'data-height' ) : $this->DEFAULT_HEIGHT,
+			// The layout sanitizer will convert this to `layout` when being sanitized.
+			// The data attribute needs to be used so that the layout sanitizer will process it.
+			'data-amp-layout' => 'responsive',
+			'width'           => $node->hasAttribute( 'data-width' ) ? $node->getAttribute( 'data-width' ) : $this->DEFAULT_WIDTH,
+			'height'          => $node->hasAttribute( 'data-height' ) ? $node->getAttribute( 'data-height' ) : $this->DEFAULT_HEIGHT,
 		];
-		if ( '100%' === $attributes['width'] ) {
-			$attributes['layout'] = 'fixed-height';
-			$attributes['width']  = 'auto';
-		}
 
 		$node->removeAttribute( 'data-width' );
 		$node->removeAttribute( 'data-height' );

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

@@ -248,17 +248,31 @@ public function sanitize_dimension( $value, $dimension ) {
 		return '';
 	}
 
+	/**
+	 * Determine if an attribute value is empty.
+	 *
+	 * @param string|null $value Attribute value.
+	 * @return bool True if empty, false if not.
+	 */
+	public function is_empty_attribute_value( $value ) {
+		return ! isset( $value ) || '' === $value;
+	}
+
 	/**
 	 * Sets the layout, and possibly the 'height' and 'width' attributes.
 	 *
 	 * @param array $attributes {
 	 *      Attributes.
 	 *
+	 *      @type string     $bottom
 	 *      @type int|string $height
-	 *      @type int|string $width
-	 *      @type string     $sizes
-	 *      @type string     $class
 	 *      @type string     $layout
+	 *      @type string     $left
+	 *      @type string     $position
+	 *      @type string     $right
+	 *      @type string     $style
+	 *      @type string     $top
+	 *      @type int|string $width
 	 * }
 	 * @return array Attributes.
 	 */
@@ -312,21 +326,27 @@ public function set_layout( $attributes ) {
 				&& '100%' === $attributes['width']
 				&& '100%' === $attributes['height']
 			) {
-				unset( $attributes['style'], $attributes['width'], $attributes['height'] );
+				unset( $attributes['style'], $styles['position'], $attributes['width'], $attributes['height'] );
+				if ( ! empty( $styles ) ) {
+					$attributes['style'] = $this->reassemble_style_string( $styles );
+				}
 				$attributes['layout'] = 'fill';
-				unset( $attributes['height'], $attributes['width'] );
 				return $attributes;
 			}
 		}
 
-		if ( empty( $attributes['height'] ) ) {
-			unset( $attributes['width'] );
-			$attributes['height'] = self::FALLBACK_HEIGHT;
-		}
-
-		if ( empty( $attributes['width'] ) || '100%' === $attributes['width'] ) {
-			$attributes['layout'] = 'fixed-height';
-			$attributes['width']  = 'auto';
+		if ( isset( $attributes['width'], $attributes['height'] ) && '100%' === $attributes['width'] && '100%' === $attributes['height'] ) {
+			unset( $attributes['width'], $attributes['height'] );
+			$attributes['layout'] = 'fill';
+		} else {
+			if ( empty( $attributes['height'] ) ) {
+				unset( $attributes['width'] );
+				$attributes['height'] = self::FALLBACK_HEIGHT;
+			}
+			if ( empty( $attributes['width'] ) || '100%' === $attributes['width'] ) {
+				$attributes['layout'] = 'fixed-height';
+				$attributes['width']  = 'auto';
+			}
 		}
 
 		return $attributes;

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

@@ -0,0 +1,118 @@
+<?php
+/**
+ * Class AMP_Layout_Sanitizer
+ *
+ * @since 1.5.0
+ * @package AMP
+ */
+
+/**
+ * Class AMP_Layout_Sanitizer
+ *
+ * @since 1.5.0
+ */
+class AMP_Layout_Sanitizer extends AMP_Base_Sanitizer {
+	use AMP_Noscript_Fallback;
+
+	/**
+	 * Sanitize any element that has the `layout` or `data-amp-layout` attribute.
+	 */
+	public function sanitize() {
+		$xpath = new DOMXPath( $this->dom );
+
+		// Elements with the `layout` attribute will be validated by `AMP_Tag_And_Attribute_Sanitizer`.
+		$nodes = $xpath->query( '//*[ not( @layout ) and ( @data-amp-layout or @width or @height or @style ) ]' );
+
+		foreach ( $nodes as $node ) {
+			/**
+			 * Element.
+			 *
+			 * @var DOMElement $node
+			 */
+
+			// Layout does not apply inside of noscript.
+			if ( $this->is_inside_amp_noscript( $node ) ) {
+				continue;
+			}
+
+			$width  = $node->getAttribute( 'width' );
+			$height = $node->getAttribute( 'height' );
+			$style  = $node->getAttribute( 'style' );
+
+			// The `layout` attribute can also be defined through the `data-amp-layout` attribute.
+			if ( $node->hasAttribute( 'data-amp-layout' ) ) {
+				$layout = $node->getAttribute( 'data-amp-layout' );
+				$node->setAttribute( 'layout', $layout );
+				$node->removeAttribute( 'data-amp-layout' );
+			}
+
+			if ( ! $this->is_empty_attribute_value( $style ) ) {
+				$styles = $this->parse_style_string( $style );
+
+				/*
+				 * If both height & width descriptors are 100%, or
+				 *    width attribute is 100% and height style descriptor is 100%, or
+				 *    height attribute is 100% and width style descriptor is 100%
+				 * then apply fill layout.
+				 */
+				if (
+					(
+						isset( $styles['width'], $styles['height'] ) &&
+						( '100%' === $styles['width'] && '100%' === $styles['height'] )
+					) ||
+					(
+						( ! $this->is_empty_attribute_value( $width ) && '100%' === $width ) &&
+						( isset( $styles['height'] ) && '100%' === $styles['height'] )
+					) ||
+					(
+						( ! $this->is_empty_attribute_value( $height ) && '100%' === $height ) &&
+						( isset( $styles['width'] ) && '100%' === $styles['width'] )
+					)
+				) {
+					unset( $styles['width'], $styles['height'] );
+					$node->removeAttribute( 'width' );
+					$node->removeAttribute( 'height' );
+
+					if ( empty( $styles ) ) {
+						$node->removeAttribute( 'style' );
+					} else {
+						$node->setAttribute( 'style', $this->reassemble_style_string( $styles ) );
+					}
+
+					$this->set_layout_fill( $node );
+					continue;
+				}
+			}
+
+			// If the width & height are `100%` then apply fill layout.
+			if ( '100%' === $width && '100%' === $height ) {
+				$this->set_layout_fill( $node );
+				continue;
+			}
+
+			// If the width is `100%`, convert the layout to `fixed-height` and width to `auto`.
+			if ( '100%' === $width ) {
+				$node->setAttribute( 'width', 'auto' );
+				$node->setAttribute( 'layout', AMP_Rule_Spec::LAYOUT_FIXED_HEIGHT );
+			}
+		}
+
+		$this->did_convert_elements = true;
+	}
+
+	/**
+	 * Apply the `fill` layout.
+	 *
+	 * @param DOMElement $node Node.
+	 */
+	private function set_layout_fill( DOMElement $node ) {
+		if ( $node->hasAttribute( 'width' ) && $node->hasAttribute( 'height' ) ) {
+			$node->removeAttribute( 'width' );
+			$node->removeAttribute( 'height' );
+		}
+
+		if ( AMP_Rule_Spec::LAYOUT_FILL !== $node->getAttribute( 'layout' ) ) {
+			$node->setAttribute( 'layout', AMP_Rule_Spec::LAYOUT_FILL );
+		}
+	}
+}

includes/sanitizers/class-amp-rule-spec.php

@@ -11,15 +11,14 @@
  * Set of constants used throughout the sanitizer.
  */
 abstract class AMP_Rule_Spec {
-
-	/**
+	/*
 	 * AMP rule_spec types
 	 */
 	const ATTR_SPEC_LIST = 'attr_spec_list';
 	const TAG_SPEC       = 'tag_spec';
 	const CDATA          = 'cdata';
 
-	/**
+	/*
 	 * AMP attr_spec value check results.
 	 *
 	 * In 0.7 these changed from strings to integers to speed up comparisons.
@@ -28,7 +27,7 @@ abstract class AMP_Rule_Spec {
 	const FAIL           = 0;
 	const NOT_APPLICABLE = -1;
 
-	/**
+	/*
 	 * HTML Element Tag rule names
 	 */
 	const DISALLOWED_ANCESTOR = 'disallowed_ancestor';
@@ -37,7 +36,7 @@ abstract class AMP_Rule_Spec {
 	const DESCENDANT_TAG_LIST = 'descendant_tag_list';
 	const CHILD_TAGS          = 'child_tags';
 
-	/**
+	/*
 	 * HTML Element Attribute rule names
 	 */
 	const ALLOW_EMPTY             = 'allow_empty';
@@ -53,6 +52,19 @@ abstract class AMP_Rule_Spec {
 	const VALUE_PROPERTIES        = 'value_properties';
 	const VALUE_URL               = 'value_url';
 
+	/*
+	 * AMP layout types
+	 */
+	const LAYOUT_NODISPLAY    = 'nodisplay';
+	const LAYOUT_FIXED        = 'fixed';
+	const LAYOUT_FIXED_HEIGHT = 'fixed-height';
+	const LAYOUT_RESPONSIVE   = 'responsive';
+	const LAYOUT_CONTAINER    = 'container';
+	const LAYOUT_FILL         = 'fill';
+	const LAYOUT_FLEX_ITEM    = 'flex-item';
+	const LAYOUT_FLUID        = 'fluid';
+	const LAYOUT_INTRINSIC    = 'intrinsic';
+
 	/**
 	 * Attribute name for AMP dev mode.
 	 *