Skip to content

Commit

Permalink
Use native img by default
Browse files Browse the repository at this point in the history
  • Loading branch information
@westonruter
westonruter committed Dec 22, 2021
1 parent 3492579 commit 24f0658
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 22 deletions.
2 changes: 1 addition & 1 deletion includes/amp-helper-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1462,7 +1462,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 );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion includes/sanitizers/class-amp-img-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class AMP_Img_Sanitizer extends AMP_Base_Sanitizer {
*/
protected $DEFAULT_ARGS = [
'add_noscript_fallback' => true,
'native_img_used' => false,
'native_img_used' => true,
];

/**
Expand Down
1 change: 1 addition & 0 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]',
Expand Down
10 changes: 5 additions & 5 deletions tests/php/test-amp-helper-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1790,13 +1790,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() );
}

/**
Expand Down
13 changes: 9 additions & 4 deletions tests/php/test-amp-img-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,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()
);

Expand Down Expand Up @@ -521,6 +521,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,
Expand Down Expand Up @@ -584,7 +589,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 );
Expand All @@ -609,7 +614,7 @@ public function test_image_block_link_to_media_file_with_lightbox() {
$expected = '<figure class="wp-block-image" data-amp-lightbox="true"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" lightbox="" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure>';

$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 );
Expand All @@ -630,7 +635,7 @@ public function test_image_block_link_to_media_file_and_alignment_with_lightbox(
$expected = '<div data-amp-lightbox="true" class="wp-block-image"><figure class="alignright size-large"><amp-img src="https://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" lightbox="" class="amp-wp-enforced-sizes" layout="intrinsic"><noscript><img src="https://placehold.it/100x100" width="100" height="100" role="button" tabindex="0"></noscript></amp-img></figure></div>';

$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 );
Expand Down
23 changes: 21 additions & 2 deletions tests/php/test-amp-style-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1298,6 +1298,9 @@ public function get_amp_selector_data() {
sprintf( '<div><img class="logo" src="%s" width="200" height="100"></div>', 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( '<div><img class="logo" src="%s" width="200" height="100"></div>', admin_url( 'images/wordpress-logo.png' ) ),
Expand All @@ -1308,16 +1311,25 @@ public function get_amp_selector_data() {
sprintf( '<div><img class="logo" src="%s" width="200" height="100"><img class="spinner" src="%s" width="200" height="100"></div>', 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( '<amp-img class="logo amp-wp-enforced-sizes" src="%s" width="200" height="100" layout="intrinsic"></amp-img><amp-anim class="spinner amp-wp-enforced-sizes" src="%s" width="200" height="100" layout="intrinsic"></amp-anim>', 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' => [
'<div id="wpadminbar"><a href="https://example.com/"><amp-img src="https://example.com/foo.png" width="100" height="100"></amp-img><amp-anim src="https://example.com/foo.gif" width="100" height="100"></amp-anim></a></div>',
'#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' => [
'<amp-img></amp-img>',
Expand All @@ -1328,6 +1340,9 @@ public function get_amp_selector_data() {
sprintf( '<div><amp-img class="logo" src="%s" width="200" height="100"></amp-img></div>', 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( '<article><img class="logo" src="%s" width="200" height="100"></article>', admin_url( 'images/wordpress-logo.png' ) ),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -3392,7 +3409,9 @@ function() {
?>
<figure class="wp-block-audio"><figcaption></figcaption></figure>
<div class="wp-block-foo"><figcaption></figcaption></div>
<img src="https://example.com/example.jpg" width="100" height="200">
<div class="amp-wp-default-form-message">
<p>Hi</p>
</div>
<?php
}
);
Expand All @@ -3415,7 +3434,7 @@ function( $original_dom, $original_source, $amphtml_dom, $amphtml_source ) {

$this->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' ) );
Expand Down
18 changes: 9 additions & 9 deletions tests/php/test-class-amp-theme-support.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1614,7 +1614,7 @@ public function test_finish_output_buffering() {
$this->assertTrue( AMP_Theme_Support::is_output_buffering() );
$this->assertSame( 3, ob_get_level() );

echo '<html><head></head><body><img src="test.png"><script data-test>document.write(\'Illegal\');</script>';
echo '<html><head></head><body><video src="test.mp4" width="100" height="100"></video><script data-test>document.write(\'Illegal\');</script>';
wp_footer();

// Additional nested output bufferings which aren't getting closed.
Expand All @@ -1638,7 +1638,7 @@ static function( $response ) {
$this->assertStringContainsString( '<html amp', $output );
$this->assertStringContainsString( 'foo', $output );
$this->assertStringContainsString( 'BAR', $output );
$this->assertStringContainsString( '<amp-img src="test.png"', $output );
$this->assertStringContainsString( '<amp-video src="test.mp4"', $output );
$this->assertStringNotContainsString( '<script data-test', $output );

}
Expand All @@ -1655,11 +1655,11 @@ public function test_filter_customize_partial_render() {
AMP_Theme_Support::init();
AMP_Theme_Support::finish_init();

$partial = '<img src="test.png" style="border:solid 1px red;"><script data-head>document.write(\'Illegal\');</script><style>img { background:blue }</style>';
$partial = '<video src="test.mp4" width="100" height="200" style="border:solid 1px red;"></video><script data-head>document.write(\'Illegal\');</script><style>video { background:blue }</style>';
$output = AMP_Theme_Support::filter_customize_partial_render( $partial );
$this->assertStringContainsString( '<amp-img src="test.png"', $output );
$this->assertStringContainsString( '<amp-video src="test.mp4"', $output );
$this->assertStringContainsString( '<style amp-custom-partial="">', $output );
$this->assertStringContainsString( 'amp-img{background:blue}', $output );
$this->assertStringContainsString( 'amp-video{background:blue}', $output );
$this->assertStringContainsString( ':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-b123f72{border:solid 1px red}', $output );
$this->assertStringEndsWith( '/*# sourceURL=amp-custom-partial.css */</style>', $output );
$this->assertStringNotContainsString( '<script', $output );
Expand Down Expand Up @@ -1770,8 +1770,9 @@ function ( $dom, $effective_sandboxing_level ) {
$prev_ordered_contain = $ordered_contain;
}

$this->assertStringContainsString( '<noscript><img', $sanitized_html );
$this->assertStringContainsString( '<amp-img', $sanitized_html );
$this->assertStringNotContainsString( '<noscript><img', $sanitized_html );
$this->assertStringContainsString( '<img width="100" height="100" src="https://example.com/hero.png" decoding="async" class="amp-wp-enforced-sizes">', $sanitized_html );
$this->assertStringContainsString( '<img width="100" height="100" src="https://example.com/test.png" loading="lazy" decoding="async" class="amp-wp-enforced-sizes">', $sanitized_html );

$this->assertStringContainsString( '<noscript><audio', $sanitized_html );
$this->assertStringContainsString( '<amp-audio', $sanitized_html );
Expand Down Expand Up @@ -2163,9 +2164,8 @@ static function() {
</head>
<body><!-- </body></html> -->
<div id="dynamic-id-0"></div>
<!-- 2nd image is needed for testing <noscript> as first is SSR'ed -->
<img width="100" height="100" src="https://example.com/hero.png">
<img width="100" height="100" src="https://example.com/test.png">
<img width="100" height="100" src="https://example.com/test.png" loading="lazy">
<audio src="https://example.com/audios/myaudio.mp3"></audio>
<amp-ad type="a9"
width="300"
Expand Down

0 comments on commit 24f0658

Please sign in to comment.