Make checking for an empty attribute DRY

ampproject · Dec 5, 2019 · a651cc4 · a651cc4
1 parent 2978b34
commit a651cc4
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 11 deletions.
diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php
@@ -254,6 +254,16 @@ public function sanitize_dimension( $value, $dimension ) {
 		return '';
 	}
 
+	/**
+	 * Determine if an attribute is empty.
+	 *
+	 * @param string $attribute Attribute value.
+	 * @return bool True if empty, false if not.
+	 */
+	public function attr_empty( $attribute ) {
+		return ! isset( $attribute ) || '' === $attribute;
+	}
+
 	/**
 	 * Sets the layout, and possibly the 'height' and 'width' attributes.
 	 *

diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
@@ -1205,7 +1205,7 @@ private function is_valid_layout( $tag_spec, $node ) {
 		}
 
 		// Heights attribute is only allowed for RESPONSIVE layout.
-		if ( ( isset( $heights_attr ) && '' !== $heights_attr ) && AMP_Rule_Spec::LAYOUT_RESPONSIVE !== $layout ) {
+		if ( ! $this->attr_empty( $heights_attr ) && AMP_Rule_Spec::LAYOUT_RESPONSIVE !== $layout ) {
 			return false;
 		}
 
@@ -1232,10 +1232,7 @@ private function is_valid_layout( $tag_spec, $node ) {
 	 */
 	private function calculate_width( $amp_layout_spec, $input_layout, AMP_CSS_Length $input_width ) {
 		if (
-			(
-				( isset( $input_layout ) && '' !== $input_layout ) ||
-				AMP_Rule_Spec::LAYOUT_FIXED === $input_layout
-			) &&
+			( ! $this->attr_empty( $input_layout ) || AMP_Rule_Spec::LAYOUT_FIXED === $input_layout ) &&
 			! $input_width->is_set() &&
 			isset( $amp_layout_spec['defines_default_width'] )
 		) {
@@ -1265,7 +1262,7 @@ private function calculate_width( $amp_layout_spec, $input_layout, AMP_CSS_Lengt
 	private function calculate_height( $amp_layout_spec, $input_layout, AMP_CSS_Length $input_height ) {
 		if (
 			(
-				( isset( $input_layout ) && '' !== $input_layout ) ||
+				! $this->attr_empty( $input_layout ) ||
 				AMP_Rule_Spec::LAYOUT_FIXED === $input_layout ||
 				AMP_Rule_Spec::LAYOUT_FIXED_HEIGHT === $input_layout
 			) &&
@@ -1300,7 +1297,7 @@ private function calculate_height( $amp_layout_spec, $input_layout, AMP_CSS_Leng
 	 * @return string Layout type.
 	 */
 	private function calculate_layout( $layout_attr, AMP_CSS_Length $width, AMP_CSS_Length $height, $sizes_attr, $heights_attr ) {
-		if ( isset( $layout_attr ) && '' !== $layout_attr ) {
+		if ( ! $this->attr_empty( $layout_attr ) ) {
 			return $layout_attr;
 		} elseif ( ! $width->is_set() && ! $height->is_set() ) {
 			return AMP_Rule_Spec::LAYOUT_CONTAINER;
@@ -1312,10 +1309,7 @@ private function calculate_layout( $layout_attr, AMP_CSS_Length $width, AMP_CSS_
 			return AMP_Rule_Spec::LAYOUT_FIXED_HEIGHT;
 		} elseif (
 			$height->is_set() && $width->is_set() &&
-			(
-				( isset( $sizes_attr ) && '' !== $sizes_attr ) ||
-				( isset( $heights_attr ) && '' !== $heights_attr )
-			)
+			( ! $this->attr_empty( $sizes_attr ) || ! $this->attr_empty( $heights_attr ) )
 		) {
 			return AMP_Rule_Spec::LAYOUT_RESPONSIVE;
 		} else {