From 613f16f6c164a0ac9688eafb99b1f0364e5dbf27 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 14 Feb 2018 23:06:16 -0600 Subject: [PATCH] Issue #864: Add layout="responsive" to . Iframes from WordPress embeds usually have width and height. In AMP, the inferred value layout for this is fixed. This means that even if you apply max-width:100%, The height won't adjust to the new ratio. Setting the 'layout' to 'responsive' seems to be the only way to ensure the same aspect ratio. Of course, this has a risk that iframes will expand to fill their container, where they didn't before. --- .../sanitizers/class-amp-base-sanitizer.php | 4 ++- .../sanitizers/class-amp-iframe-sanitizer.php | 4 +-- tests/test-amp-iframe-sanitizer.php | 34 +++++++++---------- tests/test-class-amp-base-sanitizer.php | 20 ++++++++--- 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index 43cbcc2b5aa..bf4a73fc85f 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -208,7 +208,9 @@ public function set_layout( $attributes ) { unset( $attributes['width'] ); $attributes['height'] = self::FALLBACK_HEIGHT; } - if ( empty( $attributes['width'] ) ) { + if ( ! empty( $attributes['width'] ) && ! empty( $attributes['height'] ) ) { + $attributes['layout'] = 'responsive'; + } elseif ( empty( $attributes['width'] ) ) { $attributes['layout'] = 'fixed-height'; } diff --git a/includes/sanitizers/class-amp-iframe-sanitizer.php b/includes/sanitizers/class-amp-iframe-sanitizer.php index 0f758bfb6d3..ed767018310 100644 --- a/includes/sanitizers/class-amp-iframe-sanitizer.php +++ b/includes/sanitizers/class-amp-iframe-sanitizer.php @@ -79,9 +79,7 @@ public function sanitize() { } $this->did_convert_elements = true; - - $new_attributes = $this->set_layout( $new_attributes ); - $new_attributes['style'] = 'max-width:100%'; // AMP_Style_Sanitizer will move this to the amp-custom style. + $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' ); } diff --git a/tests/test-amp-iframe-sanitizer.php b/tests/test-amp-iframe-sanitizer.php index b3fdd23e5d6..7f02bafc3da 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-class-amp-base-sanitizer.php b/tests/test-class-amp-base-sanitizer.php index e7fac24cf61..3ea82f6d92c 100644 --- a/tests/test-class-amp-base-sanitizer.php +++ b/tests/test-class-amp-base-sanitizer.php @@ -16,7 +16,7 @@ public function get_data() { ), ), - 'both_dimensions_missing' => array( + 'both_dimensions_missing' => array( array(), array( 'height' => 400, @@ -24,7 +24,7 @@ public function get_data() { ), ), - 'both_dimensions_empty' => array( + 'both_dimensions_empty' => array( array( 'width' => '', 'height' => '', @@ -35,7 +35,7 @@ public function get_data() { ), ), - 'no_width' => array( + 'no_width' => array( array( 'height' => 100, ), @@ -45,7 +45,7 @@ public function get_data() { ), ), - 'no_height' => array( + 'no_height' => array( array( 'width' => 200, ), @@ -54,6 +54,18 @@ public function get_data() { 'layout' => 'fixed-height', ), ), + + 'no_layout_specified' => array( + array( + 'width' => 100, + 'height' => 100, + ), + array( + 'width' => 100, + 'height' => 100, + 'layout' => 'responsive', + ), + ), ); }