From e530045a6e8daae4406213b6e51436f549b0a7cf Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Fri, 25 May 2018 15:49:23 +0300 Subject: [PATCH 01/12] Initial take on CSS selector conversions according to AMP. --- .../sanitizers/class-amp-style-sanitizer.php | 132 +++++++++++++----- 1 file changed, 97 insertions(+), 35 deletions(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index f99ad2bceb2..bd2300abfd7 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -1408,6 +1408,88 @@ private function finalize_styles() { } } + /** + * Check if CSS selector should be included to style. + * + * @param string $dynamic_selector_pattern Selector pattern. + * @param array $parsed_selector Array of tag and classes. + * @param string $selector CSS selector. + * @return bool If should include CSS selector. + */ + private function should_include_selector( $dynamic_selector_pattern, $parsed_selector, $selector ) { + $dom = $this->dom; + return ( + ( $dynamic_selector_pattern && preg_match( $dynamic_selector_pattern, $selector ) ) + || + ( + // If all class names are used in the doc. + ( + empty( $parsed_selector['classes'] ) + || + 0 === count( array_diff( $parsed_selector['classes'], $this->get_used_class_names() ) ) + ) + && + // If all IDs are used in the doc. + ( + empty( $parsed_selector['ids'] ) + || + 0 === count( array_filter( $parsed_selector['ids'], function( $id ) use ( $dom ) { + return ! $dom->getElementById( $id ); + } ) ) + ) + && + // If tag names are present in the doc. + ( + empty( $parsed_selector['tags'] ) + || + 0 === count( array_diff( $parsed_selector['tags'], $this->get_used_tag_names() ) ) + ) + ) + ); + } + + /** + * Ampify CSS selectors. + * + * @param array $parsed_selector Array of tag and classes. + * @param string $selector CSS selector. + * @return array|bool False if not ampification needed, ampified selector otherwise. + */ + private function get_ampified_selectors_parsed( $parsed_selector, $selector ) { + + // @todo Add mappings. + $mappings = array( + 'img' => 'amp-img', + ); + $ampified_selector = $selector; + $tags = array(); + if ( ! empty( $parsed_selector['tags'] ) ) { + foreach ( $parsed_selector['tags'] as $tag ) { + if ( ! array_key_exists( $tag, $mappings ) ) { + $tags[] = $tag; + continue; + } else { + $tags[] = $mappings[ $tag ]; + $replace_from = '/(^|>|~|\s)' . $tag . '\b/'; + $replace_to = '\1' . $mappings[ $tag ]; + $ampified_selector = preg_replace( $replace_from, $replace_to, $ampified_selector ); + } + } + } + if ( $selector === $ampified_selector ) { + return false; + } + + return array( + 'selector' => $ampified_selector, + 'parsed_selector' => array( + 'classes' => isset( $parsed_selector['classes'] ) ? $parsed_selector['classes'] : array(), + 'tags' => $tags, + 'ids' => isset( $parsed_selector['ids'] ) ? $parsed_selector['ids'] : array(), + ), + ); + } + /** * Finalize a stylesheet set (amp-custom or amp-keyframes). * @@ -1446,7 +1528,6 @@ function( $selector ) { $stylesheet_set['processed_nodes'] = array(); $final_size = 0; - $dom = $this->dom; foreach ( $stylesheet_set['pending_stylesheets'] as &$pending_stylesheet ) { $stylesheet = ''; foreach ( $pending_stylesheet['stylesheet'] as $stylesheet_part ) { @@ -1454,43 +1535,24 @@ function( $selector ) { $stylesheet .= $stylesheet_part; } else { list( $selectors_parsed, $declaration_block ) = $stylesheet_part; - if ( $should_tree_shake ) { - $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. - ( - empty( $parsed_selector['classes'] ) - || - 0 === count( array_diff( $parsed_selector['classes'], $this->get_used_class_names() ) ) - ) - && - // If all IDs are used in the doc. - ( - empty( $parsed_selector['ids'] ) - || - 0 === count( array_filter( $parsed_selector['ids'], function( $id ) use ( $dom ) { - return ! $dom->getElementById( $id ); - } ) ) - ) - && - // If tag names are present in the doc. - ( - empty( $parsed_selector['tags'] ) - || - 0 === count( array_diff( $parsed_selector['tags'], $this->get_used_tag_names() ) ) - ) - ) - ); - if ( $should_include ) { + $selectors = array(); + foreach ( $selectors_parsed as $selector => $parsed_selector ) { + if ( $should_tree_shake ) { + if ( $this->should_include_selector( $dynamic_selector_pattern, $parsed_selector, $selector ) ) { $selectors[] = $selector; } + } else { + + // Lets map the amp elements and replace the correct one and then check if we should include the replacement. + $amp_selectors_parsed = $this->get_ampified_selectors_parsed( $parsed_selector, $selector ); + if ( is_array( $amp_selectors_parsed ) && $this->should_include_selector( $dynamic_selector_pattern, $amp_selectors_parsed['parsed_selector'], $amp_selectors_parsed['selector'] ) ) { + + // @todo Check if there could be selector's array? + $selectors[] = $amp_selectors_parsed['selector']; + } else { + $selectors = array_keys( $selectors_parsed ); + } } - } else { - $selectors = array_keys( $selectors_parsed ); } if ( ! empty( $selectors ) ) { $stylesheet .= implode( ',', $selectors ) . $declaration_block; From f2395ee0ffc109b3cb826b35c4badad184c3ba08 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Fri, 25 May 2018 15:53:01 +0300 Subject: [PATCH 02/12] Add @todo note. --- includes/sanitizers/class-amp-style-sanitizer.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index bd2300abfd7..8e8a660f775 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -1545,6 +1545,8 @@ function( $selector ) { // Lets map the amp elements and replace the correct one and then check if we should include the replacement. $amp_selectors_parsed = $this->get_ampified_selectors_parsed( $parsed_selector, $selector ); + + // @todo Maybe we should also make sure here that the original selector is not needed? if ( is_array( $amp_selectors_parsed ) && $this->should_include_selector( $dynamic_selector_pattern, $amp_selectors_parsed['parsed_selector'], $amp_selectors_parsed['selector'] ) ) { // @todo Check if there could be selector's array? From 958fd48196f95e2b60ec34b0ba8a92d136993ee8 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Fri, 25 May 2018 16:00:22 +0300 Subject: [PATCH 03/12] Add @todo note. --- includes/sanitizers/class-amp-style-sanitizer.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 8e8a660f775..c3e74ace5af 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -1537,6 +1537,8 @@ function( $selector ) { list( $selectors_parsed, $declaration_block ) = $stylesheet_part; $selectors = array(); foreach ( $selectors_parsed as $selector => $parsed_selector ) { + + // @todo Replacing with AMP component selectors should happen if $should_tree_shake as well. if ( $should_tree_shake ) { if ( $this->should_include_selector( $dynamic_selector_pattern, $parsed_selector, $selector ) ) { $selectors[] = $selector; From 077a0945f90a40da16411d18cb01bbfc3f11131e Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Mon, 28 May 2018 14:05:35 +0300 Subject: [PATCH 04/12] Add additional mappings. --- assets/css/amp-default.css | 2 +- .../sanitizers/class-amp-style-sanitizer.php | 151 +++++++----------- 2 files changed, 61 insertions(+), 92 deletions(-) diff --git a/assets/css/amp-default.css b/assets/css/amp-default.css index c06e0895dbe..15766aa182e 100644 --- a/assets/css/amp-default.css +++ b/assets/css/amp-default.css @@ -1,4 +1,4 @@ -.amp-wp-unknown-size img { +.amp-wp-unknown-size .i-amphtml-replaced-content { /** 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; diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index c6500dc4b31..64a243647e7 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -942,6 +942,12 @@ private function real_path_urls( $urls, $stylesheet_url ) { private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_list, $options ) { $validation_errors = array(); + if ( method_exists( $ruleset, 'getSelectors' ) ) { + foreach ( $ruleset->getSelectors() as $selector ) { + $this->ampify_selector( $selector ); + } + } + // Remove disallowed properties. if ( ! empty( $options['property_whitelist'] ) ) { $properties = $ruleset->getRules(); @@ -1435,86 +1441,33 @@ private function finalize_styles() { } } - /** - * Check if CSS selector should be included to style. - * - * @param string $dynamic_selector_pattern Selector pattern. - * @param array $parsed_selector Array of tag and classes. - * @param string $selector CSS selector. - * @return bool If should include CSS selector. - */ - private function should_include_selector( $dynamic_selector_pattern, $parsed_selector, $selector ) { - $dom = $this->dom; - return ( - ( $dynamic_selector_pattern && preg_match( $dynamic_selector_pattern, $selector ) ) - || - ( - // If all class names are used in the doc. - ( - empty( $parsed_selector['classes'] ) - || - 0 === count( array_diff( $parsed_selector['classes'], $this->get_used_class_names() ) ) - ) - && - // If all IDs are used in the doc. - ( - empty( $parsed_selector['ids'] ) - || - 0 === count( array_filter( $parsed_selector['ids'], function( $id ) use ( $dom ) { - return ! $dom->getElementById( $id ); - } ) ) - ) - && - // If tag names are present in the doc. - ( - empty( $parsed_selector['tags'] ) - || - 0 === count( array_diff( $parsed_selector['tags'], $this->get_used_tag_names() ) ) - ) - ) - ); - } - /** * Ampify CSS selectors. * - * @param array $parsed_selector Array of tag and classes. - * @param string $selector CSS selector. - * @return array|bool False if not ampification needed, ampified selector otherwise. + * @param Selector $selector CSS Selector. */ - private function get_ampified_selectors_parsed( $parsed_selector, $selector ) { - - // @todo Add mappings. - $mappings = array( - 'img' => 'amp-img', + private function ampify_selector( $selector ) { + // @todo What about amp-anim, amp-playbuzz? + $mappings = array( + 'audio' => 'amp-audio', + 'iframe' => 'amp-iframe', + 'img' => 'amp-img', + 'video' => 'amp-video', ); - $ampified_selector = $selector; - $tags = array(); - if ( ! empty( $parsed_selector['tags'] ) ) { - foreach ( $parsed_selector['tags'] as $tag ) { - if ( ! array_key_exists( $tag, $mappings ) ) { - $tags[] = $tag; - continue; - } else { - $tags[] = $mappings[ $tag ]; - $replace_from = '/(^|>|~|\s)' . $tag . '\b/'; - $replace_to = '\1' . $mappings[ $tag ]; - $ampified_selector = preg_replace( $replace_from, $replace_to, $ampified_selector ); - } + + $new_selector = $selector->getSelector(); + foreach ( $mappings as $tag => $amp_tag ) { + if ( 0 === strpos( $selector->getSelector(), $tag ) ) { + $new_selector = substr_replace( $new_selector, $amp_tag, 0, strlen( $tag ) ); + } + if ( false !== strpos( $selector->getSelector(), ' ' . $tag ) ) { + $new_selector = str_replace( ' ' . $tag, ' ' . $amp_tag, $new_selector ); } - } - if ( $selector === $ampified_selector ) { - return false; } - return array( - 'selector' => $ampified_selector, - 'parsed_selector' => array( - 'classes' => isset( $parsed_selector['classes'] ) ? $parsed_selector['classes'] : array(), - 'tags' => $tags, - 'ids' => isset( $parsed_selector['ids'] ) ? $parsed_selector['ids'] : array(), - ), - ); + if ( $selector->getSelector() !== $new_selector ) { + $selector->setSelector( $new_selector ); + } } /** @@ -1555,6 +1508,7 @@ function( $selector ) { $stylesheet_set['processed_nodes'] = array(); $final_size = 0; + $dom = $this->dom; foreach ( $stylesheet_set['pending_stylesheets'] as &$pending_stylesheet ) { $stylesheet = ''; foreach ( $pending_stylesheet['stylesheet'] as $stylesheet_part ) { @@ -1562,28 +1516,43 @@ function( $selector ) { $stylesheet .= $stylesheet_part; } else { list( $selectors_parsed, $declaration_block ) = $stylesheet_part; - $selectors = array(); - foreach ( $selectors_parsed as $selector => $parsed_selector ) { - - // @todo Replacing with AMP component selectors should happen if $should_tree_shake as well. - if ( $should_tree_shake ) { - if ( $this->should_include_selector( $dynamic_selector_pattern, $parsed_selector, $selector ) ) { + if ( $should_tree_shake ) { + $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. + ( + empty( $parsed_selector['classes'] ) + || + 0 === count( array_diff( $parsed_selector['classes'], $this->get_used_class_names() ) ) + ) + && + // If all IDs are used in the doc. + ( + empty( $parsed_selector['ids'] ) + || + 0 === count( array_filter( $parsed_selector['ids'], function( $id ) use ( $dom ) { + return ! $dom->getElementById( $id ); + } ) ) + ) + && + // If tag names are present in the doc. + ( + empty( $parsed_selector['tags'] ) + || + 0 === count( array_diff( $parsed_selector['tags'], $this->get_used_tag_names() ) ) + ) + ) + ); + if ( $should_include ) { $selectors[] = $selector; } - } else { - - // Lets map the amp elements and replace the correct one and then check if we should include the replacement. - $amp_selectors_parsed = $this->get_ampified_selectors_parsed( $parsed_selector, $selector ); - - // @todo Maybe we should also make sure here that the original selector is not needed? - if ( is_array( $amp_selectors_parsed ) && $this->should_include_selector( $dynamic_selector_pattern, $amp_selectors_parsed['parsed_selector'], $amp_selectors_parsed['selector'] ) ) { - - // @todo Check if there could be selector's array? - $selectors[] = $amp_selectors_parsed['selector']; - } else { - $selectors = array_keys( $selectors_parsed ); - } } + } else { + $selectors = array_keys( $selectors_parsed ); } if ( ! empty( $selectors ) ) { $stylesheet .= implode( ',', $selectors ) . $declaration_block; From 7a218aa0bae195988dfbe7a9d00f37cc0fcb3e49 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 30 May 2018 23:54:47 -0700 Subject: [PATCH 05/12] Add get_selector_conversion_mapping method to all sanitizers and utilize for selector conversion --- assets/js/amp-editor-blocks.js | 2 +- .../sanitizers/class-amp-audio-sanitizer.php | 11 +++ .../sanitizers/class-amp-base-sanitizer.php | 36 ++++++++ .../sanitizers/class-amp-iframe-sanitizer.php | 15 +++- .../sanitizers/class-amp-img-sanitizer.php | 14 ++++ .../class-amp-playbuzz-sanitizer.php | 12 +++ .../sanitizers/class-amp-style-sanitizer.php | 84 +++++++++++++------ .../sanitizers/class-amp-video-sanitizer.php | 11 +++ .../templates/class-amp-content-sanitizer.php | 27 +++++- includes/utils/class-amp-dom-utils.php | 2 +- tests/test-amp-style-sanitizer.php | 48 +++++++++++ 11 files changed, 233 insertions(+), 29 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 7b9128f5d3f..2f40feb1a96 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -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; diff --git a/includes/sanitizers/class-amp-audio-sanitizer.php b/includes/sanitizers/class-amp-audio-sanitizer.php index 4c593a2e124..3622ccb2208 100644 --- a/includes/sanitizers/class-amp-audio-sanitizer.php +++ b/includes/sanitizers/class-amp-audio-sanitizer.php @@ -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