Apply fill layout when the width & height are set to 100% in style …

…attribute
ampproject · Dec 6, 2019 · a85af72 · a85af72
1 parent a651cc4
commit a85af72
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 6 deletions.
diff --git a/includes/sanitizers/class-amp-layout-sanitizer.php b/includes/sanitizers/class-amp-layout-sanitizer.php
@@ -19,11 +19,12 @@ class AMP_Layout_Sanitizer extends AMP_Base_Sanitizer {
 	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 ) ]' );
+		$nodes = $xpath->query( '//*[ not( @layout ) and ( @data-amp-layout or @width or @height or @style ) ]' );
 
 		foreach ( $nodes as $node ) {
 			$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' ) ) {
@@ -32,12 +33,31 @@ public function sanitize() {
 				$node->removeAttribute( 'data-amp-layout' );
 			}
 
-			// If the width & height are `100%` the layout must be `fill`.
+			if ( ! $this->attr_empty( $style ) ) {
+				$styles = $this->parse_style_string( $style );
+
+				// If both height & width descriptors are 100%, apply fill layout.
+				if (
+					isset( $styles['width'], $styles['height'] ) &&
+					( '100%' === $styles['width'] && '100%' === $styles['height'] )
+				) {
+					unset( $styles['width'], $styles['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 ) {
-				$node->removeAttribute( 'width' );
-				$node->removeAttribute( 'height' );
-				$node->setAttribute( 'layout', AMP_Rule_Spec::LAYOUT_FILL );
-				return;
+				$this->set_layout_fill( $node );
+				continue;
 			}
 
 			// If the width is `100%`, convert the layout to `fixed-height` and width to `auto`.
@@ -49,4 +69,20 @@ public function sanitize() {
 
 		$this->did_convert_elements = true;
 	}
+
+	/**
+	 * Apply the `fill` layout.
+	 *
+	 * @param DOMElement $node Node.
+	 */
+	private function set_layout_fill( $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 );
+		}
+	}
 }
diff --git a/tests/php/test-amp-layout-sanitizer.php b/tests/php/test-amp-layout-sanitizer.php
@@ -54,6 +54,16 @@ public function get_body_data() {
 			'100%_width_and_height_with_layout_attr'      => [
 				'<amp-img src="foo.jpg" width="100%" height="100%" layout="fill"></amp-img>',
 			],
+
+			'100%_width_and_height_style_descriptors'      => [
+				'<amp-img src="foo.jpg" style="width:100%; height:100%" ></amp-img>',
+				'<amp-img src="foo.jpg" layout="fill"></amp-img>',
+			],
+
+			'1002%_width_and_height_style_descriptors'      => [
+				'<amp-img src="foo.jpg" style="width:100%; height:100%; color:#000" ></amp-img>',
+				'<amp-img src="foo.jpg" style="color:#000" layout="fill"></amp-img>',
+			],
 		];
 	}