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 all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion assets/css/amp-default.css
@@ -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
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
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
28 changes: 19 additions & 9 deletions includes/sanitizers/class-amp-base-sanitizer.php
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
15 changes: 14 additions & 1 deletion includes/sanitizers/class-amp-iframe-sanitizer.php
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
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
19 changes: 14 additions & 5 deletions includes/sanitizers/class-amp-playbuzz-sanitizer.php
Expand Up @@ -40,6 +40,17 @@ 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' ),
);
}

/**
* Sanitize the Playbuzz elements from the HTML contained in this instance's DOMDocument.
*
Expand Down Expand Up @@ -113,16 +124,14 @@ private function filter_attributes( $attributes ) {
}
break;

case 'data-game-info':
$out['data-item-info'] = $value;
break;

case 'data-shares':
$out['data-share-buttons'] = $value;
break;

case 'data-game-info':
case 'data-comments':
$out['data-comments'] = $value;
case 'class':
$out[ $name ] = $value;
break;

default:
Expand Down
109 changes: 96 additions & 13 deletions includes/sanitizers/class-amp-style-sanitizer.php
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,36 @@ 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 )
);
}

// Prevent selectors like `amp-img img` getting deleted since `img` does not occur in the DOM.
$this->args['dynamic_element_selectors'] = array_merge(
$this->args['dynamic_element_selectors'],
$this->selector_mappings[ $html_selectors ]
);
}
}
}

/**
* Sanitize CSS styles within the HTML contained in this instance's DOMDocument.
*
Expand Down Expand Up @@ -647,7 +684,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 @@ -912,6 +949,16 @@ private function prepare_stylesheet( $stylesheet_string, $options = array() ) {
$pattern .= preg_quote( $after_declaration_block, '#' );
$pattern .= '#s';

$dynamic_selector_pattern = null;
if ( ! empty( $this->args['dynamic_element_selectors'] ) ) {
$dynamic_selector_pattern = implode( '|', array_map(
function( $selector ) {
return preg_quote( $selector, '#' );
},
$this->args['dynamic_element_selectors']
) );
}

$split_stylesheet = preg_split( $pattern, $stylesheet_string, -1, PREG_SPLIT_DELIM_CAPTURE );
$length = count( $split_stylesheet );
for ( $i = 0; $i < $length; $i++ ) {
Expand All @@ -929,6 +976,11 @@ private function prepare_stylesheet( $stylesheet_string, $options = array() ) {
// Remove attribute selectors to eliminate false negative, such as with `.social-navigation a[href*="example.com"]:before`.
$reduced_selector = preg_replace( '/\[\w.*?\]/', '', $reduced_selector );

// Ignore any selector terms that occur under a dynamic selector.
if ( $dynamic_selector_pattern ) {
$reduced_selector = preg_replace( '#((?:' . $dynamic_selector_pattern . ')(?:\.[a-z0-9_-]+)*)[^a-z0-9_-].*#si', '$1', $reduced_selector . ' ' );
}

$reduced_selector = preg_replace_callback(
'/\.([a-zA-Z0-9_-]+)/',
function( $matches ) use ( $selector, &$selectors_parsed ) {
Expand Down Expand Up @@ -1236,6 +1288,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 +1810,45 @@ 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 );
$amp_tag_pattern = '/(?<=^|[^a-z0-9_-])' . preg_quote( $amp_selector ) . '(?=$|[^a-z0-9_-])/i';
preg_match( $amp_tag_pattern, $edited_selector, $matches );
if ( ! empty( $matches ) && $amp_selector === $matches[0] ) {
continue;
}
$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 All @@ -1778,16 +1873,6 @@ private function finalize_stylesheet_set( $stylesheet_set ) {
) );
}

$dynamic_selector_pattern = null;
if ( $should_tree_shake && ! empty( $this->args['dynamic_element_selectors'] ) ) {
$dynamic_selector_pattern = '#' . implode( '|', array_map(
function( $selector ) {
return preg_quote( $selector, '#' );
},
$this->args['dynamic_element_selectors']
) ) . '#';
}

$stylesheet_set['processed_nodes'] = array();

$final_size = 0;
Expand All @@ -1804,8 +1889,6 @@ function( $selector ) {
$selectors = array();
foreach ( $selectors_parsed as $selector => $parsed_selector ) {
$should_include = (
( $dynamic_selector_pattern && preg_match( $dynamic_selector_pattern, $selector ) )
||
(
// If all class names are used in the doc.
(
Expand Down