From 11405b3c40346605fe6ba08b72f67f46d02be261 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Tue, 13 Feb 2018 22:19:23 -0600 Subject: [PATCH] Issue #864: Remove layout="responsive" from . This could lead to unexpected results. If the intended width or height is less than the container, This will increase it to fill the container. This fixed the issue of overflowing in widgets. But it could create an issue in other places. --- .../sanitizers/class-amp-base-sanitizer.php | 3 -- .../sanitizers/class-amp-iframe-sanitizer.php | 3 +- .../sanitizers/class-amp-video-sanitizer.php | 2 +- tests/test-amp-iframe-sanitizer.php | 34 +++++++++---------- tests/test-amp-video-sanitizer.php | 22 ++++++------ 5 files changed, 31 insertions(+), 33 deletions(-) diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index fcf784d0678..43cbcc2b5aa 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -211,9 +211,6 @@ public function set_layout( $attributes ) { if ( empty( $attributes['width'] ) ) { $attributes['layout'] = 'fixed-height'; } - if ( ! empty( $attributes['width'] ) && ! empty( $attributes['height'] ) ) { - $attributes['layout'] = 'responsive'; - } return $attributes; } diff --git a/includes/sanitizers/class-amp-iframe-sanitizer.php b/includes/sanitizers/class-amp-iframe-sanitizer.php index a2acdd0de76..c3bd35dea30 100644 --- a/includes/sanitizers/class-amp-iframe-sanitizer.php +++ b/includes/sanitizers/class-amp-iframe-sanitizer.php @@ -80,7 +80,8 @@ public function sanitize() { $this->did_convert_elements = true; - $new_attributes = $this->set_layout( $new_attributes ); + $new_attributes = $this->set_layout( $new_attributes ); + $new_attributes['style'] = 'max-width:100%'; if ( isset( $new_attributes['width'] ) && isset( $new_attributes['height'] ) ) { $this->add_or_append_attribute( $new_attributes, 'class', 'amp-wp-enforced-sizes' ); } diff --git a/includes/sanitizers/class-amp-video-sanitizer.php b/includes/sanitizers/class-amp-video-sanitizer.php index a4a7c2b56cc..08e7762839f 100644 --- a/includes/sanitizers/class-amp-video-sanitizer.php +++ b/includes/sanitizers/class-amp-video-sanitizer.php @@ -52,7 +52,7 @@ public function sanitize() { $new_attributes = $this->set_layout( $new_attributes ); if ( isset( $new_attributes['width'] ) && isset( $new_attributes['height'] ) ) { - $this->add_or_append_attribute( $new_attributes, 'class', 'amp-wp-enforced-sizes' ); + $new_attributes['layout'] = 'responsive'; } $new_node = AMP_DOM_Utils::create_node( $this->dom, 'amp-video', $new_attributes ); diff --git a/tests/test-amp-iframe-sanitizer.php b/tests/test-amp-iframe-sanitizer.php index 7f02bafc3da..b3fdd23e5d6 100644 --- a/tests/test-amp-iframe-sanitizer.php +++ b/tests/test-amp-iframe-sanitizer.php @@ -10,57 +10,57 @@ public function get_data() { 'simple_iframe' => array( '', - '', + '', ), 'force_https' => array( '', - '', + '', ), 'iframe_without_dimensions' => array( '', - '', + '', ), 'iframe_with_height_only' => array( '', - '', + '', ), 'iframe_with_width_only' => array( '', - '', + '', ), 'iframe_with_invalid_frameborder' => array( '', - '', + '', ), 'iframe_with_1_frameborder' => array( '', - '', + '', ), 'simple_iframe_with_sandbox' => array( '', - '', + '', ), 'iframe_with_blacklisted_attribute' => array( '', - '', + '', ), 'iframe_with_sizes_attribute_is_overridden' => array( '', - '', + '', ), 'iframe_with_protocol_relative_url' => array( '', - '', + '', ), 'multiple_same_iframe' => array( @@ -69,7 +69,7 @@ public function get_data() { ', - '', + '', ), 'multiple_different_iframes' => array( @@ -78,19 +78,19 @@ public function get_data() { ', - '', + '', ), 'iframe_in_p_tag' => array( '

', - '', + '', ), 'multiple_iframes_in_p_tag' => array( '

', - '', + '', ), 'multiple_iframes_and_contents_in_p_tag' => array( '

contents

', - '

contents

', + '

contents

', ), ); } @@ -160,7 +160,7 @@ public function test_get_scripts__did_convert() { public function test__args__placeholder() { $source = ''; - $expected = '
'; + $expected = '
'; $dom = AMP_DOM_Utils::get_dom_from_content( $source ); $sanitizer = new AMP_Iframe_Sanitizer( $dom, array( diff --git a/tests/test-amp-video-sanitizer.php b/tests/test-amp-video-sanitizer.php index 917be213f5d..1a0ae62413d 100644 --- a/tests/test-amp-video-sanitizer.php +++ b/tests/test-amp-video-sanitizer.php @@ -10,7 +10,7 @@ public function get_data() { 'simple_video' => array( '', - '', + '', ), 'video_without_dimensions' => array( @@ -20,32 +20,32 @@ public function get_data() { 'autoplay_attribute' => array( '', - '', + '', ), 'autoplay_attribute__false' => array( '', - '', + '', ), 'video_with_whitelisted_attributes__enabled' => array( '', - '', + '', ), 'video_with_whitelisted_attributes__disabled' => array( '', - '', + '', ), 'video_with_blacklisted_attribute' => array( '', - '', + '', ), 'video_with_sizes_attribute_is_overridden' => array( '', - '', + '', ), 'video_with_children' => array( @@ -53,7 +53,7 @@ public function get_data() { ', - '', + '', ), 'multiple_same_video' => array( @@ -61,19 +61,19 @@ public function get_data() { ', - '', + '', ), 'multiple_different_videos' => array( ' ', - '', + '', ), 'https_not_required' => array( '', - '', + '', ), ); }