From e9714baaab78fdaf2e15129c2df9852fed87f095 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 11 Mar 2022 09:32:28 -0800 Subject: [PATCH 1/2] Use native img by default --- includes/amp-helper-functions.php | 2 +- .../sanitizers/class-amp-img-sanitizer.php | 2 +- .../sanitizers/class-amp-style-sanitizer.php | 1 + tests/php/test-amp-helper-functions.php | 10 ++++---- tests/php/test-amp-img-sanitizer.php | 13 +++++++---- tests/php/test-amp-style-sanitizer.php | 23 +++++++++++++++++-- tests/php/test-class-amp-theme-support.php | 18 +++++++-------- 7 files changed, 47 insertions(+), 22 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 38d16a191a4..9ca96319b07 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1469,7 +1469,7 @@ function amp_is_native_img_used() { * * @param bool $use_native Whether to use `img`. */ - return (bool) apply_filters( 'amp_native_img_used', false ); + return (bool) apply_filters( 'amp_native_img_used', true ); } /** diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 9188ac2f05e..dd1fabe860b 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -57,7 +57,7 @@ class AMP_Img_Sanitizer extends AMP_Base_Sanitizer { */ protected $DEFAULT_ARGS = [ 'add_noscript_fallback' => true, - 'native_img_used' => false, + 'native_img_used' => true, 'allow_picture' => false, ]; diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 94e3a443afd..ceb67b50c46 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -155,6 +155,7 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer { protected $DEFAULT_ARGS = [ 'disable_style_processing' => false, 'dynamic_element_selectors' => [ + 'amp-img', 'amp-list', 'amp-live-list', '[submit-error]', diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 309c2f8ecd9..009f09c95b9 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -1834,13 +1834,13 @@ public function test_amp_is_dev_mode() { * @covers ::amp_is_native_img_used() */ public function test_amp_is_native_img_used() { - $this->assertFalse( amp_is_native_img_used(), 'Expected to be disabled by default for now.' ); + $this->assertTrue( amp_is_native_img_used(), 'Expected to be disabled by default for now.' ); - add_filter( 'amp_native_img_used', '__return_true' ); - $this->assertTrue( amp_is_native_img_used() ); - - add_filter( 'amp_native_img_used', '__return_false', 20 ); + add_filter( 'amp_native_img_used', '__return_false' ); $this->assertFalse( amp_is_native_img_used() ); + + add_filter( 'amp_native_img_used', '__return_true', 20 ); + $this->assertTrue( amp_is_native_img_used() ); } /** diff --git a/tests/php/test-amp-img-sanitizer.php b/tests/php/test-amp-img-sanitizer.php index 9742e32fbfb..283cb0e6bb2 100644 --- a/tests/php/test-amp-img-sanitizer.php +++ b/tests/php/test-amp-img-sanitizer.php @@ -48,7 +48,7 @@ public function test_get_selector_conversion_mapping() { $with_defaults = new AMP_Img_Sanitizer( $dom ); $this->assertEquals( - [ 'img' => [ 'amp-img', 'amp-anim' ] ], + [], $with_defaults->get_selector_conversion_mapping() ); @@ -615,6 +615,11 @@ public function test_converter( $source, $expected = null, $args = [], $expected $error_codes = []; + $args = array_merge( + [ 'native_img_used' => false ], + $args + ); + $args = array_merge( [ 'use_document_element' => true, @@ -678,7 +683,7 @@ public function test_no_gif_image_scripts() { $expected = [ 'amp-anim' => true ]; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); - $sanitizer = new AMP_Img_Sanitizer( $dom ); + $sanitizer = new AMP_Img_Sanitizer( $dom, [ 'native_img_used' => false ] ); $sanitizer->sanitize(); $validating_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom ); @@ -703,7 +708,7 @@ public function test_image_block_link_to_media_file_with_lightbox() { $expected = '
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); - $sanitizer = new AMP_Img_Sanitizer( $dom ); + $sanitizer = new AMP_Img_Sanitizer( $dom, [ 'native_img_used' => false ] ); $sanitizer->sanitize(); $sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom ); @@ -724,7 +729,7 @@ public function test_image_block_link_to_media_file_and_alignment_with_lightbox( $expected = '
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); - $sanitizer = new AMP_Img_Sanitizer( $dom ); + $sanitizer = new AMP_Img_Sanitizer( $dom, [ 'native_img_used' => false ] ); $sanitizer->sanitize(); $sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom ); diff --git a/tests/php/test-amp-style-sanitizer.php b/tests/php/test-amp-style-sanitizer.php index dc19ef59c1b..ab7497525c0 100644 --- a/tests/php/test-amp-style-sanitizer.php +++ b/tests/php/test-amp-style-sanitizer.php @@ -1298,6 +1298,9 @@ public function get_amp_selector_data() { sprintf( '
', admin_url( 'images/wordpress-logo.png' ) ), 'div img.logo{border:solid 1px red}', 'div amp-img.logo{border:solid 1px red}', // Note amp-anim is still tree-shaken because it doesn't occur in the DOM. + [ + AMP_Img_Sanitizer::class => [ 'native_img_used' => false ], + ], ], 'img-missing-class' => [ sprintf( '
', admin_url( 'images/wordpress-logo.png' ) ), @@ -1308,16 +1311,25 @@ public function get_amp_selector_data() { sprintf( '
', admin_url( 'images/wordpress-logo.png' ), admin_url( 'images/spinner-2x.gif' ) ), 'div img{border:solid 1px red}', 'div amp-img,div amp-anim{border:solid 1px red}', + [ + AMP_Img_Sanitizer::class => [ 'native_img_used' => false ], + ], ], 'amp-img-and-amp-anim' => [ sprintf( '', admin_url( 'images/wordpress-logo.png' ), admin_url( 'images/spinner-2x.gif' ) ), 'amp-img.amp-wp-enforced-sizes[layout="intrinsic"] > img,amp-anim.amp-wp-enforced-sizes[layout="intrinsic"] > img{object-fit:contain}', 'amp-img.amp-wp-enforced-sizes[layout="intrinsic"] > img,amp-anim.amp-wp-enforced-sizes[layout="intrinsic"] > img{object-fit:contain}', + [ + AMP_Img_Sanitizer::class => [ 'native_img_used' => false ], + ], ], 'admin-bar-style-selectors' => [ '
', '#wpadminbar a, #wpadminbar a:hover, #wpadminbar a img, #wpadminbar a img:hover { border: none; text-decoration: none; background: none;}', '#wpadminbar a,#wpadminbar a:hover,#wpadminbar a amp-img,#wpadminbar a amp-anim,#wpadminbar a amp-img:hover,#wpadminbar a amp-anim:hover{border:none;text-decoration:none;background:none}', + [ + AMP_Img_Sanitizer::class => [ 'native_img_used' => false ], + ], ], 'img_with_amp_img' => [ '', @@ -1328,6 +1340,9 @@ public function get_amp_selector_data() { sprintf( '
', admin_url( 'images/wordpress-logo.png' ) ), 'div amp-img.logo img{object-fit:cover}', 'div amp-img.logo img{object-fit:cover}', + [ + AMP_Img_Sanitizer::class => [ 'native_img_used' => false ], + ], ], 'img-tree-shaking' => [ sprintf( '
', admin_url( 'images/wordpress-logo.png' ) ), @@ -1649,6 +1664,8 @@ static function ( $selector ) { ) ); + add_filter( 'amp_native_img_used', '__return_false' ); + // The toggling of the 'add_noscript_fallback' arg is to catch a bizzare PHP DOM issue whereby if you replace // an element in a Document, and that replaced element had an ID, the element will still be returned by // getElementById even though it is no longer inside of the document. When add_noscript_fallback is false, @@ -3453,7 +3470,9 @@ function() { ?>
- +
+

Hi

+
assertStringContainsString( '.wp-block-audio figcaption', $amphtml_source, 'Expected block-library/style.css' ); $this->assertStringContainsString( '[class^="wp-block-"]:not(.wp-block-gallery) figcaption', $amphtml_source, 'Expected twentyten/blocks.css' ); - $this->assertStringContainsString( 'amp-img img', $amphtml_source, 'Expected amp-default.css' ); + $this->assertStringContainsString( '.amp-wp-default-form-message>p', $amphtml_source, 'Expected amp-default.css' ); $this->assertStringContainsString( 'ab-empty-item', $amphtml_source, 'Expected admin-bar.css to still be present.' ); $this->assertStringNotContainsString( 'earlyprintstyle', $amphtml_source, 'Expected early print style to not be present.' ); $this->assertStringContainsString( 'admin-bar', $amphtml_dom->body->getAttribute( 'class' ) ); diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 27679d30cc2..f47bfbef724 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -1613,7 +1613,7 @@ public function test_finish_output_buffering() { $this->assertTrue( AMP_Theme_Support::is_output_buffering() ); $this->assertSame( 3, ob_get_level() ); - echo ''; + echo ''; wp_footer(); // Additional nested output bufferings which aren't getting closed. @@ -1637,7 +1637,7 @@ static function( $response ) { $this->assertStringContainsString( 'assertStringContainsString( 'foo', $output ); $this->assertStringContainsString( 'BAR', $output ); - $this->assertStringContainsString( 'assertStringContainsString( 'assertStringNotContainsString( ''; $output = AMP_Theme_Support::filter_customize_partial_render( $partial ); - $this->assertStringContainsString( 'assertStringContainsString( 'assertStringContainsString( '', $output ); $this->assertStringNotContainsString( 'assertStringContainsString( '