Merge pull request #3941 from ampproject/fix/3939-prevent-lazy-loaded…

…-iframe Remove `loading=lazy` attribute from iframes
ampproject · Dec 17, 2019 · 0ffb583 · 0ffb583
2 parents e4152e2 + a764a90
commit 0ffb583
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 6 deletions.
diff --git a/includes/sanitizers/class-amp-iframe-sanitizer.php b/includes/sanitizers/class-amp-iframe-sanitizer.php
@@ -219,7 +219,7 @@ private function normalize_attributes( $attributes ) {
 
 				case 'allowfullscreen':
 				case 'allowtransparency':
-					if ( 'false' !== $value ) {
+					if ( 'false' !== strtolower( $value ) ) {
 						$out[ $name ] = '';
 					}
 					break;
@@ -229,6 +229,16 @@ private function normalize_attributes( $attributes ) {
 					// Omit these since amp-iframe will add them if needed if the `allowfullscreen` attribute is present.
 					break;
 
+				case 'loading':
+					/*
+					 * The `amp-iframe` component already does lazy-loading by default; trigger a validation error only
+					 * if the value is not `lazy`.
+					 */
+					if ( 'lazy' !== strtolower( $value ) ) {
+						$out[ $name ] = $value;
+					}
+					break;
+
 				default:
 					$out[ $name ] = $value;
 					break;

diff --git a/tests/php/test-amp-iframe-sanitizer.php b/tests/php/test-amp-iframe-sanitizer.php
@@ -43,7 +43,7 @@ public function get_data() {
 			],
 
 			'simple_iframe_without_noscript_or_placeholder' => [
-				'<iframe src="https://example.com/embed/132886713" width="500" height="281" frameborder="0" class="iframe-class" allowtransparency="false" allowfullscreen></iframe>',
+				'<iframe src="https://example.com/embed/132886713" width="500" height="281" frameborder="0" class="iframe-class" allowtransparency="FALSE" allowfullscreen></iframe>',
 				'<amp-iframe src="https://example.com/embed/132886713" width="500" height="281" frameborder="0" class="iframe-class amp-wp-enforced-sizes" allowfullscreen="" sandbox="allow-scripts allow-same-origin" layout="intrinsic"></amp-iframe>',
 				[
 					'add_noscript_fallback' => false,
@@ -288,7 +288,7 @@ public function get_data() {
 			'attributes_removed_from_noscript_iframe'   => [
 				'<iframe src="https://example.com/embed/132886713" width="500" height="281" onclick="foo()" data-foo="bar"></iframe>',
 				'
-					<amp-iframe src="https://example.com/embed/132886713" width="500" height="281" data-foo="bar" sandbox="allow-scripts allow-same-origin" layout="intrinsic" class="amp-wp-enforced-sizes">
+					<amp-iframe src="https://example.com/embed/132886713" width="500" height="281" onclick="foo()" data-foo="bar" sandbox="allow-scripts allow-same-origin" layout="intrinsic" class="amp-wp-enforced-sizes">
 						<span placeholder="" class="amp-wp-iframe-placeholder"></span>
 						<noscript>
 							<iframe src="https://example.com/embed/132886713" width="500" height="281"></iframe>
@@ -410,6 +410,26 @@ public function get_data() {
 						</noscript>
 					</amp-iframe>',
 			],
+
+			'iframe_with_loading_lazy_attr' => [
+				'<iframe loading="lazy" src="https://example.com" width="320" height="640"></iframe>',
+				'
+					<amp-iframe src="https://example.com" width="320" height="640" sandbox="allow-scripts allow-same-origin" layout="intrinsic" class="amp-wp-enforced-sizes">
+						<noscript>
+							<iframe src="https://example.com" width="320" height="640"></iframe>
+						</noscript>
+					</amp-iframe>',
+			],
+
+			'iframe_with_loading_eager_attr' => [
+				'<iframe loading="eager" src="https://example.com" width="320" height="640"></iframe>',
+				'
+					<amp-iframe loading="eager" src="https://example.com" width="320" height="640" sandbox="allow-scripts allow-same-origin" layout="intrinsic" class="amp-wp-enforced-sizes">
+						<noscript>
+							<iframe src="https://example.com" width="320" height="640"></iframe>
+						</noscript>
+					</amp-iframe>',
+			],
 		];
 	}
 
@@ -434,9 +454,6 @@ public function test_converter( $source, $expected = null, $args = [] ) {
 		$sanitizer = new AMP_Iframe_Sanitizer( $dom, $args );
 		$sanitizer->sanitize();
 
-		$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom );
-		$whitelist_sanitizer->sanitize();
-
 		$content = AMP_DOM_Utils::get_content_from_dom( $dom );
 		$this->assertEqualMarkup( $expected, $content );
 	}