Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1094: Transform CSS selectors according to sanitizer HTML element to AMP component conversions #1175

Merged
merged 17 commits into from
May 31, 2018
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion assets/css/amp-default.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.amp-wp-unknown-size img {
.amp-wp-unknown-size [src] {
/** Worst case scenario when we can't figure out dimensions for an image. **/
/** Force the image into a box of fixed dimensions and use object-fit to scale. **/
object-fit: contain;
Expand Down
2 changes: 1 addition & 1 deletion assets/js/amp-editor-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
return element;
}
} else if ( ! component.hasGalleryShortcodeCarouselAttribute( attributes.text || '' ) ) {
// Add amp-carousel=false attribut to the shortcode.
// Add amp-carousel=false attribute to the shortcode.
text = attributes.text.replace( '[gallery', '[gallery amp-carousel=false' );
} else {
text = attributes.text;
Expand Down
11 changes: 11 additions & 0 deletions includes/sanitizers/class-amp-audio-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ class AMP_Audio_Sanitizer extends AMP_Base_Sanitizer {
*/
public static $tag = 'audio';

/**
* Get mapping of HTML selectors to the AMP component selectors which they may be converted into.
*
* @return array Mapping.
*/
public function get_selector_conversion_mapping() {
return array(
'audio' => array( 'amp-audio' ),
);
}

/**
* Sanitize the <audio> elements from the HTML contained in this instance's DOMDocument.
*
Expand Down
30 changes: 21 additions & 9 deletions includes/sanitizers/class-amp-base-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,25 @@ public function __construct( $dom, $args = array() ) {
}
}

/**
* Get mapping of HTML selectors to the AMP component selectors which they may be converted into.
*
* @return array Mapping.
*/
public function get_selector_conversion_mapping() {
return array();
}

/**
* Run logic before any sanitizers are run.
*
* After the sanitizers are instantiated but before calling sanitize on each of them, this
* method is called with list of all the instantiated sanitizers.
*
* @param AMP_Base_Sanitizer[] $sanitizers Sanitizers.
*/
public function init( $sanitizers ) {}

/**
* Sanitize the HTML contained in the DOMDocument received by the constructor
*/
Expand Down Expand Up @@ -444,9 +463,6 @@ public function get_data_amp_attributes( $node ) {
if ( isset( $parent_attributes['data-amp-noloading'] ) && true === filter_var( $parent_attributes['data-amp-noloading'], FILTER_VALIDATE_BOOLEAN ) ) {
$attributes['noloading'] = $parent_attributes['data-amp-noloading'];
}
if ( isset( $parent_attributes['data-amp-lightbox'] ) && true === filter_var( $parent_attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN ) ) {
$attributes['lightbox'] = true;
}
}

return $attributes;
Expand All @@ -466,12 +482,6 @@ public function filter_data_amp_attributes( $attributes, $amp_data ) {
if ( isset( $amp_data['noloading'] ) ) {
$attributes['data-amp-noloading'] = '';
}
if ( isset( $amp_data['lightbox'] ) ) {
$attributes['data-amp-lightbox'] = '';
$attributes['on'] = 'tap:' . self::AMP_IMAGE_LIGHTBOX_ID;
$attributes['role'] = 'button';
$attributes['tabindex'] = 0;
}
return $attributes;
}

Expand Down Expand Up @@ -514,6 +524,8 @@ public function filter_attachment_layout_attributes( $node, $new_attributes, $la

/**
* Add <amp-image-lightbox> element to body tag if it doesn't exist yet.
*
* @todo Move this to AMP_Img_Sanitizer?
*/
public function maybe_add_amp_image_lightbox_node() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is also used by AMP_Gallery_Block_Sanitizer.


Expand Down
15 changes: 14 additions & 1 deletion includes/sanitizers/class-amp-iframe-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ class AMP_Iframe_Sanitizer extends AMP_Base_Sanitizer {
'add_placeholder' => false,
);

/**
* Get mapping of HTML selectors to the AMP component selectors which they may be converted into.
*
* @return array Mapping.
*/
public function get_selector_conversion_mapping() {
return array(
'iframe' => array(
'amp-iframe',
),
);
}

/**
* Sanitize the <iframe> elements from the HTML contained in this instance's DOMDocument.
*
Expand Down Expand Up @@ -189,7 +202,7 @@ private function filter_attributes( $attributes ) {
private function build_placeholder( $parent_attributes ) {
$placeholder_node = AMP_DOM_Utils::create_node( $this->dom, 'div', array(
'placeholder' => '',
'class' => 'amp-wp-iframe-placeholder',
'class' => 'amp-wp-iframe-placeholder',
) );

return $placeholder_node;
Expand Down
46 changes: 42 additions & 4 deletions includes/sanitizers/class-amp-img-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,20 @@ class AMP_Img_Sanitizer extends AMP_Base_Sanitizer {
*/
private static $anim_extension = '.gif';

/**
* Get mapping of HTML selectors to the AMP component selectors which they may be converted into.
*
* @return array Mapping.
*/
public function get_selector_conversion_mapping() {
return array(
'img' => array(
'amp-img',
'amp-anim',
),
);
}

/**
* Sanitize the <img> elements from the HTML contained in this instance's DOMDocument.
*
Expand Down Expand Up @@ -222,15 +236,12 @@ private function adjust_and_replace_node( $node ) {
$amp_data = $this->get_data_amp_attributes( $node );
$old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node );
$old_attributes = $this->filter_data_amp_attributes( $old_attributes, $amp_data );
$old_attributes = $this->maybe_add_lightbox_attributes( $old_attributes, $node );

$new_attributes = $this->filter_attributes( $old_attributes );
$layout = isset( $amp_data['layout'] ) ? $amp_data['layout'] : false;
$new_attributes = $this->filter_attachment_layout_attributes( $node, $new_attributes, $layout );

if ( isset( $old_attributes['data-amp-lightbox'] ) ) {
$this->maybe_add_amp_image_lightbox_node();
}

$this->add_or_append_attribute( $new_attributes, 'class', 'amp-wp-enforced-sizes' );
if ( empty( $new_attributes['layout'] ) && ! empty( $new_attributes['height'] ) && ! empty( $new_attributes['width'] ) ) {
$new_attributes['layout'] = 'intrinsic';
Expand All @@ -248,6 +259,33 @@ private function adjust_and_replace_node( $node ) {
$node->parentNode->replaceChild( $new_node, $node );
}

/**
* Set lightbox attributes.
*
* @param array $attributes Array of attributes.
* @param DomNode $node Array of AMP attributes.
* @return array Updated attributes.
*/
private function maybe_add_lightbox_attributes( $attributes, $node ) {
$parent_node = $node->parentNode;
if ( 'figure' !== $parent_node->tagName ) {
return $attributes;
}

$parent_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $parent_node );

if ( isset( $parent_attributes['data-amp-lightbox'] ) && true === filter_var( $parent_attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN ) ) {
$attributes['data-amp-lightbox'] = '';
$attributes['on'] = 'tap:' . self::AMP_IMAGE_LIGHTBOX_ID;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that self:AMP_IMAGE_LIGHTBOX_ID is also used by AMP_Gallery_Block_Sanitizer, that's why it to AMP_Base_Sanitizer.

$attributes['role'] = 'button';
$attributes['tabindex'] = 0;

$this->maybe_add_amp_image_lightbox_node();
}

return $attributes;
}

/**
* Determines is a URL is considered a GIF URL
*
Expand Down
12 changes: 12 additions & 0 deletions includes/sanitizers/class-amp-playbuzz-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ class AMP_Playbuzz_Sanitizer extends AMP_Base_Sanitizer {
*/
private static $height = '500';

/**
* Get mapping of HTML selectors to the AMP component selectors which they may be converted into.
*
* @return array Mapping.
*/
public function get_selector_conversion_mapping() {
return array(
'div.pb_feed' => array( 'amp-playbuzz' ),
'.pb_feed' => array( 'amp-playbuzz' ),
);
}

/**
* Sanitize the Playbuzz elements from the HTML contained in this instance's DOMDocument.
*
Expand Down
71 changes: 70 additions & 1 deletion includes/sanitizers/class-amp-style-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,13 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
*/
private $processed_imported_stylesheet_urls = array();

/**
* Mapping of HTML element selectors to AMP selector elements.
*
* @var array
*/
private $selector_mappings = array();

/**
* Get error codes that can be raised during parsing of CSS.
*
Expand Down Expand Up @@ -308,6 +315,30 @@ private function get_used_tag_names() {
return $this->used_tag_names;
}

/**
* Run logic before any sanitizers are run.
*
* After the sanitizers are instantiated but before calling sanitize on each of them, this
* method is called with list of all the instantiated sanitizers.
*
* @param AMP_Base_Sanitizer[] $sanitizers Sanitizers.
*/
public function init( $sanitizers ) {
parent::init( $sanitizers );

foreach ( $sanitizers as $sanitizer ) {
foreach ( $sanitizer->get_selector_conversion_mapping() as $html_selectors => $amp_selectors ) {
if ( ! isset( $this->selector_mappings[ $html_selectors ] ) ) {
$this->selector_mappings[ $html_selectors ] = $amp_selectors;
} else {
$this->selector_mappings[ $html_selectors ] = array_unique(
array_merge( $this->selector_mappings[ $html_selectors ], $amp_selectors )
);
}
}
}
}

/**
* Sanitize CSS styles within the HTML contained in this instance's DOMDocument.
*
Expand Down Expand Up @@ -647,7 +678,7 @@ private function fetch_external_stylesheet( $url ) {
private function process_stylesheet( $stylesheet, $options = array() ) {
$parsed = null;
$cache_key = null;
$cache_group = 'amp-parsed-stylesheet-v6';
$cache_group = 'amp-parsed-stylesheet-v7';

$cache_impacting_options = array_merge(
wp_array_slice_assoc(
Expand Down Expand Up @@ -1236,6 +1267,10 @@ private function real_path_urls( $urls, $stylesheet_url ) {
private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_list, $options ) {
$results = array();

if ( $ruleset instanceof DeclarationBlock ) {
$this->ampify_ruleset_selectors( $ruleset );
}

// Remove disallowed properties.
if ( ! empty( $options['property_whitelist'] ) ) {
$properties = $ruleset->getRules();
Expand Down Expand Up @@ -1754,6 +1789,40 @@ private function finalize_styles() {
}
}

/**
* Convert CSS selectors.
*
* @param DeclarationBlock $ruleset Ruleset.
*/
private function ampify_ruleset_selectors( $ruleset ) {
$selectors = array();
$replacements = 0;
foreach ( $ruleset->getSelectors() as $old_selector ) {
$edited_selectors = array( $old_selector->getSelector() );
foreach ( $this->selector_mappings as $html_selector => $amp_selectors ) { // Note: The $selector_mappings array contains ~6 items.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These notes are here because 4 level of loop nesting is normally something that is a big red flag for performance.

$html_pattern = '/(?<=^|[^a-z0-9_-])' . preg_quote( $html_selector ) . '(?=$|[^a-z0-9_-])/i';
foreach ( $edited_selectors as &$edited_selector ) { // Note: The $edited_selectors array contains only item in the normal case.
$original_selector = $edited_selector;
$amp_selector = array_shift( $amp_selectors );
$edited_selector = preg_replace( $html_pattern, $amp_selector, $edited_selector, -1, $count );
if ( ! $count ) {
continue;
}
$replacements += $count;
while ( ! empty( $amp_selectors ) ) { // Note: This array contains only a couple items.
$amp_selector = array_shift( $amp_selectors );
$edited_selectors[] = preg_replace( $html_pattern, $amp_selector, $original_selector, -1, $count );
}
}
}
$selectors = array_merge( $selectors, $edited_selectors );
}

if ( $replacements > 0 ) {
$ruleset->setSelectors( $selectors );
}
}

/**
* Finalize a stylesheet set (amp-custom or amp-keyframes).
*
Expand Down
11 changes: 11 additions & 0 deletions includes/sanitizers/class-amp-video-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ class AMP_Video_Sanitizer extends AMP_Base_Sanitizer {
*/
public static $tag = 'video';

/**
* Get mapping of HTML selectors to the AMP component selectors which they may be converted into.
*
* @return array Mapping.
*/
public function get_selector_conversion_mapping() {
return array(
'video' => array( 'amp-video' ),
);
}

/**
* Sanitize the <video> elements from the HTML contained in this instance's DOMDocument.
*
Expand Down
22 changes: 21 additions & 1 deletion includes/templates/class-amp-content-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,16 @@ public static function sanitize_document( &$dom, $sanitizer_classes, $args ) {

$return_styles = ! empty( $args['return_styles'] );
unset( $args['return_styles'] );

/**
* Sanitizers.
*
* @var AMP_Base_Sanitizer[] $sanitizers
*/
$sanitizers = array();

// Instantiate the sanitizers.
foreach ( $sanitizer_classes as $sanitizer_class => $sanitizer_args ) {
$sanitize_class_start = microtime( true );
if ( ! class_exists( $sanitizer_class ) ) {
/* translators: %s is sanitizer class */
_doing_it_wrong( __METHOD__, sprintf( esc_html__( 'Sanitizer (%s) class does not exist', 'amp' ), esc_html( $sanitizer_class ) ), '0.4.1' );
Expand All @@ -84,6 +92,18 @@ public static function sanitize_document( &$dom, $sanitizer_classes, $args ) {
continue;
}

$sanitizers[ $sanitizer_class ] = $sanitizer;
}

// Let the sanitizers know about each other prior to sanitizing.
foreach ( $sanitizers as $sanitizer ) {
$sanitizer->init( $sanitizers );
}

// Sanitize.
foreach ( $sanitizers as $sanitizer ) {
$sanitize_class_start = microtime( true );

$sanitizer->sanitize();

$scripts = array_merge( $scripts, $sanitizer->get_scripts() );
Expand Down
2 changes: 1 addition & 1 deletion includes/utils/class-amp-dom-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ public static function create_node( $dom, $tag, $attributes ) {
*
* @since 0.2
*
* @param DOMNode $node Represents an HTML element for which to extract attributes.
* @param DOMElement $node Represents an HTML element for which to extract attributes.
*
* @return string[] The attributes for the passed node, or an
* empty array if it has no attributes.
Expand Down
4 changes: 2 additions & 2 deletions tests/test-amp-img-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ public function get_data() {
),

'image_with_custom_lightbox_attrs' => array(
'<img src="http://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="true" />',
'<amp-img src="http://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="true" class="amp-wp-enforced-sizes" layout="intrinsic"></amp-img><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
'<figure data-amp-lightbox="true"><img src="http://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" /></figure>',
'<figure data-amp-lightbox="true"><amp-img src="http://placehold.it/100x100" width="100" height="100" data-foo="bar" role="button" tabindex="0" data-amp-lightbox="" on="tap:amp-image-lightbox" class="amp-wp-enforced-sizes" layout="intrinsic"></amp-img></figure><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
),
);
}
Expand Down
Loading