Skip to content

Commit

Permalink
Issue #864: Remove the 'sizes' attribute from <amp-img>.
Browse files Browse the repository at this point in the history 
On Paul Baukus and Weston Ruter's suggestion.
The sizes attribute isn't relevant to this WP plugin,
as Paul Baukus mentioned.
  • Loading branch information
Ryan Kienstra committed Feb 13, 2018
1 parent 936f302 commit 6848541
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 156 deletions.
36 changes: 0 additions & 36 deletions includes/sanitizers/class-amp-base-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,42 +218,6 @@ public function set_layout( $attributes ) {
return $attributes;
}

/**
* This is our workaround to enforce max sizing with layout=responsive.
*
* We want elements to not grow beyond their width and shrink to fill the screen on viewports smaller than their width.
*
* See https://github.com/ampproject/amphtml/issues/1280#issuecomment-171533526
* See https://github.com/Automattic/amp-wp/issues/101
*
* @param string[] $attributes {
* Attributes.
*
* @type int $height
* @type int $width
* @type string $sizes
* @type string $class
* @type string $layout
* }
* @return string[]
*/
public function enforce_sizes_attribute( $attributes ) {
if ( ! isset( $attributes['width'], $attributes['height'] ) ) {
return $attributes;
}

$max_width = $attributes['width'];
if ( isset( $this->args['content_max_width'] ) && $max_width >= $this->args['content_max_width'] ) {
$max_width = $this->args['content_max_width'];
}

$attributes['sizes'] = sprintf( '(min-width: %1$dpx) %1$dpx, 100vw', absint( $max_width ) );

$this->add_or_append_attribute( $attributes, 'class', 'amp-wp-enforced-sizes' );

return $attributes;
}

/**
* Adds or appends key and value to list of attributes
*
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 @@ -192,7 +192,7 @@ private function adjust_and_replace_nodes_in_array_map( $node_lists ) {
private function adjust_and_replace_node( $node ) {
$old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node );
$new_attributes = $this->filter_attributes( $old_attributes );
$new_attributes = $this->enforce_sizes_attribute( $new_attributes );
$this->add_or_append_attribute( $new_attributes, 'class', 'amp-wp-enforced-sizes' );
if ( $this->is_gif_url( $new_attributes['src'] ) ) {
$this->did_convert_elements = true;

Expand Down
24 changes: 12 additions & 12 deletions tests/test-amp-img-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,47 +29,47 @@ public function get_data() {

'image_with_self_closing_tag' => array(
'<img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!" />',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img>',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!" class="amp-wp-enforced-sizes"></amp-img>',
),

'image_with_no_end_tag' => array(
'<img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!">',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img>',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!" class="amp-wp-enforced-sizes"></amp-img>',
),

'image_with_end_tag' => array(
'<img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!"></img>',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img>',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" alt="Placeholder!" class="amp-wp-enforced-sizes"></amp-img>',
),

'image_with_on_attribute' => array(
'<img src="http://placehold.it/350x150" on="tap:my-lightbox" width="350" height="150" />',
'<amp-img src="http://placehold.it/350x150" on="tap:my-lightbox" width="350" height="150" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img>',
'<amp-img src="http://placehold.it/350x150" on="tap:my-lightbox" width="350" height="150" class="amp-wp-enforced-sizes"></amp-img>',
),

'image_with_blacklisted_attribute' => array(
'<img src="http://placehold.it/350x150" width="350" height="150" style="border: 1px solid red;" />',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img>',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" class="amp-wp-enforced-sizes"></amp-img>',
),

'image_with_no_dimensions_is_forced_dimensions' => array(
'<img src="http://placehold.it/350x150" />',
'<amp-img src="http://placehold.it/350x150" width="600" height="400" class="amp-wp-unknown-size amp-wp-enforced-sizes" sizes="(min-width: 600px) 600px, 100vw"></amp-img>',
'<amp-img src="http://placehold.it/350x150" width="600" height="400" class="amp-wp-unknown-size amp-wp-enforced-sizes"></amp-img>',
),

'image_with_sizes_attribute_is_overridden' => array(
'<img src="http://placehold.it/350x150" width="350" height="150" sizes="(min-width: 100px) 300px, 90vw" />',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img>',
'<img src="http://placehold.it/350x150" width="350" height="150" />',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" class="amp-wp-enforced-sizes"></amp-img>',
),

'gif_image_conversion' => array(
'<img src="http://placehold.it/350x150.gif" width="350" height="150" alt="Placeholder!" />',
'<amp-anim src="http://placehold.it/350x150.gif" width="350" height="150" alt="Placeholder!" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-anim>',
'<amp-anim src="http://placehold.it/350x150.gif" width="350" height="150" alt="Placeholder!" class="amp-wp-enforced-sizes"></amp-anim>',
),

'gif_image_url_with_querystring' => array(
'<img src="http://placehold.it/350x150.gif?foo=bar" width="350" height="150" alt="Placeholder!" />',
'<amp-anim src="http://placehold.it/350x150.gif?foo=bar" width="350" height="150" alt="Placeholder!" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-anim>',
'<amp-anim src="http://placehold.it/350x150.gif?foo=bar" width="350" height="150" alt="Placeholder!" class="amp-wp-enforced-sizes"></amp-anim>',
),

'multiple_same_image' => array(
Expand All @@ -78,15 +78,15 @@ public function get_data() {
<img src="http://placehold.it/350x150" width="350" height="150" />
<img src="http://placehold.it/350x150" width="350" height="150" />
',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/350x150" width="350" height="150" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/350x150" width="350" height="150" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/350x150" width="350" height="150" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img>',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/350x150" width="350" height="150" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/350x150" width="350" height="150" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/350x150" width="350" height="150" class="amp-wp-enforced-sizes"></amp-img>',
),

'multiple_different_images' => array(
'<img src="http://placehold.it/350x150" width="350" height="150" />
<img src="http://placehold.it/360x160" width="360" height="160" />
<img src="http://placehold.it/370x170" width="370" height="170" />
<img src="http://placehold.it/380x180" width="380" height="180" />',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" sizes="(min-width: 350px) 350px, 100vw" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/360x160" width="360" height="160" sizes="(min-width: 360px) 360px, 100vw" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/370x170" width="370" height="170" sizes="(min-width: 370px) 370px, 100vw" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/380x180" width="380" height="180" sizes="(min-width: 380px) 380px, 100vw" class="amp-wp-enforced-sizes"></amp-img>',
'<amp-img src="http://placehold.it/350x150" width="350" height="150" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/360x160" width="360" height="160" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/370x170" width="370" height="170" class="amp-wp-enforced-sizes"></amp-img><amp-img src="http://placehold.it/380x180" width="380" height="180" class="amp-wp-enforced-sizes"></amp-img>',
),
);
}
Expand Down
107 changes: 0 additions & 107 deletions tests/test-class-amp-base-sanitizer.php
View file
Original file line number Diff line number Diff line change
@@ -1,112 +1,5 @@
<?php

class AMP_Base_Sanitizer__Enforce_Sizes_Attribute__Test extends WP_UnitTestCase {
public function get_data() {
return array(
'already_has_sizes' => array(
array(
'sizes' => 'blah',
),
array(
'sizes' => 'blah',
),
),

'empty' => array(
array(),
array(),
),

'no_width' => array(
array(
'height' => 100,
),
array(
'height' => 100,
),
),

'no_height' => array(
array(
'width' => 200,
),
array(
'width' => 200,
),
),

'enforce_sizes_no_class' => array(
array(
'width' => 200,
'height' => 100,
),
array(
'width' => 200,
'height' => 100,
'sizes' => '(min-width: 200px) 200px, 100vw',
'class' => 'amp-wp-enforced-sizes',
),
),

'enforce_sizes_has_class' => array(
array(
'width' => 200,
'height' => 100,
'class' => 'my-class',
),
array(
'width' => 200,
'height' => 100,
'sizes' => '(min-width: 200px) 200px, 100vw',
'class' => 'my-class amp-wp-enforced-sizes',
),
),

'enforce_sizes_with_bigger_content_max_width' => array(
array(
'width' => 250,
'height' => 100,
),
array(
'width' => 250,
'height' => 100,
'sizes' => '(min-width: 250px) 250px, 100vw',
'class' => 'amp-wp-enforced-sizes',
),
array(
'content_max_width' => 500,
),
),

'enforce_sizes_with_smaller_content_max_width' => array(
array(
'width' => 800,
'height' => 350,
),
array(
'width' => 800,
'height' => 350,
'sizes' => '(min-width: 675px) 675px, 100vw',
'class' => 'amp-wp-enforced-sizes',
),
array(
'content_max_width' => 675,
),
),
);
}

/**
* @dataProvider get_data
*/
public function test_enforce_sizes_attribute( $source_attributes, $expected_attributes, $args = array() ) {
$sanitizer = new AMP_Test_Stub_Sanitizer( new DOMDocument, $args );
$returned_attributes = $sanitizer->enforce_sizes_attribute( $source_attributes );

$this->assertEquals( $expected_attributes, $returned_attributes );
}
}

class AMP_Base_Sanitizer__Enforce_Fixed_Height__Test extends WP_UnitTestCase {
public function get_data() {
return array(
Expand Down

0 comments on commit 6848541

Please sign in to comment.