From 83e4b59e64da201ecdd278666c3debb74c5e4cd3 Mon Sep 17 00:00:00 2001 From: Kolya Korobochkin Date: Tue, 11 Sep 2018 22:40:00 +0300 Subject: [PATCH 01/12] Added functionality to remove empty media queries in CSS --- .../sanitizers/class-amp-style-sanitizer.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index f6d2329aef7..6dceeec1c90 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -2070,6 +2070,7 @@ private function finalize_stylesheet_set( $stylesheet_set ) { $stylesheet .= implode( ',', $selectors ) . $declaration_block; } } + $stylesheet = $this->remove_empty_media_queries( $stylesheet ); $sheet_size = strlen( $stylesheet ); $pending_stylesheet['size'] = $sheet_size; @@ -2104,4 +2105,19 @@ private function finalize_stylesheet_set( $stylesheet_set ) { return $stylesheet_set; } + + /** + * Remove media queries without any styles. + * + * @param string $css CSS styles. + * + * @return string Styles without empty media queries. + */ + public function remove_empty_media_queries( $css ) { + $new = preg_replace( '/@media[^\{]+\{\}/', '', $css ); + if ( is_string( $new ) ) { + return $new; + } + return $css; + } } From faf0d37c96d9481b4f9f37da05f199a33d41367b Mon Sep 17 00:00:00 2001 From: Kolya Korobochkin Date: Wed, 12 Sep 2018 16:36:14 +0300 Subject: [PATCH 02/12] Added PHP unit tests for method which removes empty media queries --- tests/test-amp-style-sanitizer.php | 38 ++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/test-amp-style-sanitizer.php b/tests/test-amp-style-sanitizer.php index 1ad45d46174..167188b45f6 100644 --- a/tests/test-amp-style-sanitizer.php +++ b/tests/test-amp-style-sanitizer.php @@ -830,6 +830,7 @@ public function test_large_custom_css_and_rule_removal() { $html .= ''; $html .= ''; $html .= ''; + $html .= ''; $html .= '...'; $dom = AMP_DOM_Utils::get_dom( $html ); @@ -847,6 +848,7 @@ public function test_large_custom_css_and_rule_removal() { '.b{color:blue}', '#exists{color:white}', 'span{color:white}', + '.b { background: lightblue; }', ), array_values( $sanitizer->get_stylesheets() ) ); @@ -1362,4 +1364,40 @@ public function test_unicode_stylesheet() { $this->assertContains( ".dashicons-admin-customizer:before{content:\"\xEF\x95\x80\"}", $sanitized_html ); $this->assertContains( 'span::after{content:"⚡️"}', $sanitized_html ); } + + /** + * Test method which remove media queries without any styles. + * + * @dataProvider get_styles_with_media_queries + * + * @covers AMP_Style_Sanitizer::remove_empty_media_queries + * + * @param string $css Input CSS. + * @param string $expected_css CSS expected after removing media queries. + */ + public function test_remove_empty_media_queries( $css, $expected_css ) { + $dom = AMP_DOM_Utils::get_dom_from_content( '' ); + $style_sanitizer = new AMP_Style_Sanitizer( $dom ); + $result = $style_sanitizer->remove_empty_media_queries( $css ); + + $this->assertSame( $expected_css, $result ); + } + + /** + * Return CSS styles with empty media queries. + * + * @return array CSS Styles. + */ + public function get_styles_with_media_queries() { + return array( + 'not_empty_media_query' => array( + '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', + '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', + ), + 'two_media_queries' => array( + '.a{color:gray;}@media only screen and(max-width:940px){}@media only screen and(max-width:940px){.b{margin: 0 auto;}}.c{display:block;}', + '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', + ), + ); + } } From 8862c4c235fe3a71f6d7036d240404bf8462bc2b Mon Sep 17 00:00:00 2001 From: Kolya Korobochkin Date: Wed, 12 Sep 2018 17:49:02 +0300 Subject: [PATCH 03/12] Improved regular expression which detects media queries to prevent removing content property values --- .../sanitizers/class-amp-style-sanitizer.php | 2 +- tests/test-amp-style-sanitizer.php | 22 +++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 6dceeec1c90..71c860043f5 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -2114,7 +2114,7 @@ private function finalize_stylesheet_set( $stylesheet_set ) { * @return string Styles without empty media queries. */ public function remove_empty_media_queries( $css ) { - $new = preg_replace( '/@media[^\{]+\{\}/', '', $css ); + $new = preg_replace( '/(?#nonexists { color:black; } #exists { color:white; }'; $html .= ''; $html .= ''; + $html .= ''; $html .= '...'; $dom = AMP_DOM_Utils::get_dom( $html ); @@ -848,7 +865,8 @@ public function test_large_custom_css_and_rule_removal() { '.b{color:blue}', '#exists{color:white}', 'span{color:white}', - '.b { background: lightblue; }', + '.b{background:lightblue}', + '@media screen and (max-width: 1000px){@supports (display: grid){.b::before{content:"@media screen and (max-width: 1000px) {"}.b::after{content:"}"}}}@media screen and (min-width: 750px) and (max-width: 999px){.b::before{content:"@media screen and (max-width: 1000px) {}";content:"@media screen and (max-width: 1000px) {}"}}', ), array_values( $sanitizer->get_stylesheets() ) ); @@ -1395,7 +1413,7 @@ public function get_styles_with_media_queries() { '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', ), 'two_media_queries' => array( - '.a{color:gray;}@media only screen and(max-width:940px){}@media only screen and(max-width:940px){.b{margin: 0 auto;}}.c{display:block;}', + '.a{color:gray;}@media only screen and(max-width:940px){}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', ), ); From 7ce499c28c40adb5e0ea599abf6a2acddbc3886c Mon Sep 17 00:00:00 2001 From: Kolya Korobochkin Date: Wed, 12 Sep 2018 19:00:28 +0300 Subject: [PATCH 04/12] Added test case for remove_empty_media_queries method where exists multiple media queries including query inside content property --- tests/test-amp-style-sanitizer.php | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/test-amp-style-sanitizer.php b/tests/test-amp-style-sanitizer.php index b6132fd6af9..47befe385af 100644 --- a/tests/test-amp-style-sanitizer.php +++ b/tests/test-amp-style-sanitizer.php @@ -1416,6 +1416,43 @@ public function get_styles_with_media_queries() { '.a{color:gray;}@media only screen and(max-width:940px){}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', ), + 'media_query_in_content_property' => array( + '@media screen {} +@media screen and (max-width: 1000px) { + @supports (display: grid) { + .b::before { + content: "@media screen and (max-width: 1000px) {"; + } + .b::after { + content: "}"; + } + } +} +@media screen {} +@media screen and (min-width: 750px) and (max-width: 999px) { + .b::before { + content: "@media screen and (max-width: 1000px) {}"; + content: \'@media screen and (max-width: 1000px) {}\'; + } +} +@media screen {}', + '@media screen and (max-width: 1000px) { + @supports (display: grid) { + .b::before { + content: "@media screen and (max-width: 1000px) {"; + } + .b::after { + content: "}"; + } + } +} +@media screen and (min-width: 750px) and (max-width: 999px) { + .b::before { + content: "@media screen and (max-width: 1000px) {}"; + content: \'@media screen and (max-width: 1000px) {}\'; + } +}', + ), ); } } From c5783ba6ba27d5fed0434f1a598c8c250b1d0439 Mon Sep 17 00:00:00 2001 From: Kolya Korobochkin Date: Wed, 12 Sep 2018 19:13:01 +0300 Subject: [PATCH 05/12] Detect and remove any white space chars after media query --- includes/sanitizers/class-amp-style-sanitizer.php | 2 +- tests/test-amp-style-sanitizer.php | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 71c860043f5..0655fece39c 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -2114,7 +2114,7 @@ private function finalize_stylesheet_set( $stylesheet_set ) { * @return string Styles without empty media queries. */ public function remove_empty_media_queries( $css ) { - $new = preg_replace( '/(? Date: Thu, 13 Sep 2018 14:59:30 -0700 Subject: [PATCH 06/12] Update PHP-CSS-Parser to https://github.com/xwp/PHP-CSS-Parser/pull/1 --- composer.json | 2 +- composer.lock | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index c13e765d545..c8600aa246d 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,7 @@ ], "require": { "php": "^5.3.6 || ^7.0", - "sabberworm/php-css-parser": "dev-master" + "sabberworm/php-css-parser": "dev-add/at-rule-before-after" }, "require-dev": { "wp-coding-standards/wpcs": "^0.14.0", diff --git a/composer.lock b/composer.lock index 07abc13ab15..4885ab97ce1 100644 --- a/composer.lock +++ b/composer.lock @@ -4,20 +4,20 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "content-hash": "33d4c9d9bf48285e6557d79a6b4b7601", + "content-hash": "539e80946c38e5b05a8aef89a8c50a09", "packages": [ { "name": "sabberworm/php-css-parser", - "version": "dev-master", + "version": "dev-add/at-rule-before-after", "source": { "type": "git", "url": "https://github.com/xwp/PHP-CSS-Parser.git", - "reference": "bd0c481ba28ccd54bbbb1846a69ead237cd26cd8" + "reference": "53b0c2ba0895bd7c54e0e80b69d74a8237bad8a5" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/xwp/PHP-CSS-Parser/zipball/bd0c481ba28ccd54bbbb1846a69ead237cd26cd8", - "reference": "bd0c481ba28ccd54bbbb1846a69ead237cd26cd8", + "url": "https://api.github.com/repos/xwp/PHP-CSS-Parser/zipball/53b0c2ba0895bd7c54e0e80b69d74a8237bad8a5", + "reference": "53b0c2ba0895bd7c54e0e80b69d74a8237bad8a5", "shasum": "" }, "require": { @@ -48,9 +48,9 @@ "stylesheet" ], "support": { - "source": "https://github.com/xwp/PHP-CSS-Parser/tree/master" + "source": "https://github.com/xwp/PHP-CSS-Parser/tree/add/at-rule-before-after" }, - "time": "2018-09-06 16:11:27" + "time": "2018-09-13 21:49:15" } ], "packages-dev": [ From b38d891cf7dea0cb3d005317b6e92ca109f55da9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 13 Sep 2018 15:00:36 -0700 Subject: [PATCH 07/12] Split stylesheets before and after CSS at rules --- includes/sanitizers/class-amp-style-sanitizer.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 96fe6de4936..2e0c2a38058 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -789,7 +789,7 @@ private function fetch_external_stylesheet( $url ) { private function process_stylesheet( $stylesheet, $options = array() ) { $parsed = null; $cache_key = null; - $cache_group = 'amp-parsed-stylesheet-v11'; // This should be bumped whenever the PHP-CSS-Parser is updated. + $cache_group = 'amp-parsed-stylesheet-v12'; // This should be bumped whenever the PHP-CSS-Parser is updated. $cache_impacting_options = array_merge( wp_array_slice_assoc( @@ -1066,15 +1066,23 @@ private function prepare_stylesheet( $stylesheet_string, $options = array() ) { $between_selectors = '/*AMP_WP_BETWEEN_SELECTORS*/'; $after_declaration_block_selectors = '/*AMP_WP_BEFORE_DECLARATION_SELECTORS*/'; $after_declaration_block = '/*AMP_WP_AFTER_DECLARATION*/'; + $before_at_rule = '/*AMP_WP_BEFORE_AT_RULE*/'; + $after_at_rule = '/*AMP_WP_AFTER_AT_RULE*/'; $output_format->set( 'BeforeDeclarationBlock', $before_declaration_block ); $output_format->set( 'SpaceBeforeSelectorSeparator', $between_selectors ); $output_format->set( 'AfterDeclarationBlockSelectors', $after_declaration_block_selectors ); $output_format->set( 'AfterDeclarationBlock', $after_declaration_block ); + $output_format->set( 'BeforeAtRuleBlock', $before_at_rule ); + $output_format->set( 'AfterAtRuleBlock', $after_at_rule ); $stylesheet_string = $css_document->render( $output_format ); $pattern = '#'; + $pattern .= preg_quote( $before_at_rule, '#' ); + $pattern .= '|'; + $pattern .= preg_quote( $after_at_rule, '#' ); + $pattern .= '|'; $pattern .= '(' . preg_quote( $before_declaration_block, '#' ) . ')'; $pattern .= '(.+?)'; $pattern .= preg_quote( $after_declaration_block_selectors, '#' ); From 7b5530445c259a0ec607cda813f6771268e398ce Mon Sep 17 00:00:00 2001 From: Kolya Korobochkin Date: Mon, 17 Sep 2018 18:04:18 +0300 Subject: [PATCH 08/12] Remove empty media queries without regular expressions --- .../sanitizers/class-amp-style-sanitizer.php | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 2e0c2a38058..aa9292129c6 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -2041,7 +2041,17 @@ private function finalize_stylesheet_set( $stylesheet_set ) { $stylesheet = ''; foreach ( $pending_stylesheet['stylesheet'] as $stylesheet_part ) { if ( is_string( $stylesheet_part ) ) { - $stylesheet .= $stylesheet_part; + if ( '}' === $stylesheet_part && ! empty( $query ) ) { + if ( count( $query ) >= 2 ) { + $query[] = $stylesheet_part; + $stylesheet .= implode( '', $query ); + } + unset( $query ); + } elseif ( strpos( $stylesheet_part, '@media' ) !== false ) { + $query = array( $stylesheet_part ); + } else { + $stylesheet .= $stylesheet_part; + } continue; } list( $selectors_parsed, $declaration_block ) = $stylesheet_part; @@ -2082,10 +2092,13 @@ private function finalize_stylesheet_set( $stylesheet_set ) { $selectors = array_keys( $selectors_parsed ); } if ( ! empty( $selectors ) ) { - $stylesheet .= implode( ',', $selectors ) . $declaration_block; + if ( ! empty( $query ) ) { + $query[] = implode( ',', $selectors ) . $declaration_block; + } else { + $stylesheet .= implode( ',', $selectors ) . $declaration_block; + } } } - $stylesheet = $this->remove_empty_media_queries( $stylesheet ); $sheet_size = strlen( $stylesheet ); $pending_stylesheet['size'] = $sheet_size; From 56d83bdda9bcae5fd3a8a39864003b2f1757932f Mon Sep 17 00:00:00 2001 From: Kolya Korobochkin Date: Mon, 24 Sep 2018 12:47:27 +0300 Subject: [PATCH 09/12] Removed method and PHP unit test which used in the first implementation of removing empty media query mechanism. --- .../sanitizers/class-amp-style-sanitizer.php | 15 ---- tests/test-amp-style-sanitizer.php | 72 ------------------- 2 files changed, 87 deletions(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index aa9292129c6..4ce22e314d4 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -2133,19 +2133,4 @@ private function finalize_stylesheet_set( $stylesheet_set ) { return $stylesheet_set; } - - /** - * Remove media queries without any styles. - * - * @param string $css CSS styles. - * - * @return string Styles without empty media queries. - */ - public function remove_empty_media_queries( $css ) { - $new = preg_replace( '/(?assertContains( ".dashicons-admin-customizer:before{content:\"\xEF\x95\x80\"}", $sanitized_html ); $this->assertContains( 'span::after{content:"⚡️"}', $sanitized_html ); } - - /** - * Test method which remove media queries without any styles. - * - * @dataProvider get_styles_with_media_queries - * - * @covers AMP_Style_Sanitizer::remove_empty_media_queries - * - * @param string $css Input CSS. - * @param string $expected_css CSS expected after removing media queries. - */ - public function test_remove_empty_media_queries( $css, $expected_css ) { - $dom = AMP_DOM_Utils::get_dom_from_content( '' ); - $style_sanitizer = new AMP_Style_Sanitizer( $dom ); - $result = $style_sanitizer->remove_empty_media_queries( $css ); - - $this->assertSame( $expected_css, $result ); - } - - /** - * Return CSS styles with empty media queries. - * - * @return array CSS Styles. - */ - public function get_styles_with_media_queries() { - return array( - 'not_empty_media_query' => array( - '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', - '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', - ), - 'two_media_queries' => array( - '.a{color:gray;}@media only screen and(max-width:940px){}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', - '.a{color:gray;}@media only screen and(max-width:940px){.b{margin:0 auto;}}.c{display:block;}', - ), - 'media_query_in_content_property' => array( - '@media screen {} -@media screen and (max-width: 1000px) { - @supports (display: grid) { - .b::before { - content: "@media screen and (max-width: 1000px) {"; - } - .b::after { - content: "}"; - } - } -} -@media screen {} -@media screen and (min-width: 750px) and (max-width: 999px) { - .b::before { - content: "@media screen and (max-width: 1000px) {}"; - content: \'@media screen and (max-width: 1000px) {}\'; - } -} -@media screen {}', - '@media screen and (max-width: 1000px) { - @supports (display: grid) { - .b::before { - content: "@media screen and (max-width: 1000px) {"; - } - .b::after { - content: "}"; - } - } -}@media screen and (min-width: 750px) and (max-width: 999px) { - .b::before { - content: "@media screen and (max-width: 1000px) {}"; - content: \'@media screen and (max-width: 1000px) {}\'; - } -}', - ), - ); - } } From 32f6d8d48f21ac27399554a81cbf126f5b8292de Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 23 Nov 2018 22:45:43 -0800 Subject: [PATCH 10/12] Move empty rule removal to second pass --- .../sanitizers/class-amp-style-sanitizer.php | 63 ++++++++++++++----- tests/test-amp-style-sanitizer.php | 43 +++++++------ 2 files changed, 71 insertions(+), 35 deletions(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 4ce22e314d4..4c6820d847e 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -2038,20 +2038,10 @@ private function finalize_stylesheet_set( $stylesheet_set ) { $final_size = 0; $dom = $this->dom; foreach ( $stylesheet_set['pending_stylesheets'] as &$pending_stylesheet ) { - $stylesheet = ''; + $stylesheet_parts = array(); foreach ( $pending_stylesheet['stylesheet'] as $stylesheet_part ) { if ( is_string( $stylesheet_part ) ) { - if ( '}' === $stylesheet_part && ! empty( $query ) ) { - if ( count( $query ) >= 2 ) { - $query[] = $stylesheet_part; - $stylesheet .= implode( '', $query ); - } - unset( $query ); - } elseif ( strpos( $stylesheet_part, '@media' ) !== false ) { - $query = array( $stylesheet_part ); - } else { - $stylesheet .= $stylesheet_part; - } + $stylesheet_parts[] = $stylesheet_part; continue; } list( $selectors_parsed, $declaration_block ) = $stylesheet_part; @@ -2092,13 +2082,54 @@ private function finalize_stylesheet_set( $stylesheet_set ) { $selectors = array_keys( $selectors_parsed ); } if ( ! empty( $selectors ) ) { - if ( ! empty( $query ) ) { - $query[] = implode( ',', $selectors ) . $declaration_block; - } else { - $stylesheet .= implode( ',', $selectors ) . $declaration_block; + $stylesheet_parts[] = implode( ',', $selectors ) . $declaration_block; + } + } + + // Strip empty at-rules after tree shaking. + $stylesheet_part_count = count( $stylesheet_parts ); + for ( $i = 0; $i < $stylesheet_part_count; $i++ ) { + $stylesheet_part = $stylesheet_parts[ $i ]; + if ( '@' !== substr( $stylesheet_part, 0, 1 ) ) { + continue; + } + + // Delete empty at-rules. + if ( '{}' === substr( $stylesheet_part, -2 ) ) { + $stylesheet_part_count--; + array_splice( $stylesheet_parts, $i, 1 ); + $i--; + continue; + } + + // Delete at-rules that were emptied due to tree-shaking. + if ( '{' === substr( $stylesheet_part, -1 ) ) { + $open_braces = 1; + for ( $j = $i + 1; $j < $stylesheet_part_count; $j++ ) { + $stylesheet_part = $stylesheet_parts[ $j ]; + $is_at_rule = '@' === substr( $stylesheet_part, 0, 1 ); + if ( $is_at_rule && '{' === substr( $stylesheet_part, -1 ) ) { + $open_braces++; + + } elseif ( '}' === $stylesheet_part ) { + $open_braces--; + } else { + break; + } + + // Splice out the parts that are empty. + if ( 0 === $open_braces ) { + array_splice( $stylesheet_parts, $i, $j - $i + 1 ); + $stylesheet_part_count = count( $stylesheet_parts ); + $i--; + continue; + } } } } + $stylesheet = implode( '', $stylesheet_parts ); + + // @todo Rename $stylesheet to $stylesheet_parts above. $sheet_size = strlen( $stylesheet ); $pending_stylesheet['size'] = $sheet_size; diff --git a/tests/test-amp-style-sanitizer.php b/tests/test-amp-style-sanitizer.php index 16acc548440..6cdd86c806c 100644 --- a/tests/test-amp-style-sanitizer.php +++ b/tests/test-amp-style-sanitizer.php @@ -831,23 +831,28 @@ public function test_large_custom_css_and_rule_removal() { $html .= ''; $html .= ''; $html .= ''; - $html .= ''; + $html .= ' + + '; $html .= '...'; $dom = AMP_DOM_Utils::get_dom( $html ); @@ -988,8 +993,8 @@ public function get_keyframe_data() { ), 'style_amp_keyframes_last_child' => array( - 'before between as after', - 'before between as after', + 'before between as after', + 'before between as after', array(), ), From e7b7dbdfe6a666006712532e37872bfd8312f3a8 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 23 Nov 2018 23:21:02 -0800 Subject: [PATCH 11/12] Improve handling of empty rules alongside non-empty ones --- includes/sanitizers/class-amp-style-sanitizer.php | 11 +++++++---- tests/test-amp-style-sanitizer.php | 7 +++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index cd7a64c4fb4..a68a8e30617 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -777,7 +777,7 @@ private function fetch_external_stylesheet( $url ) { private function process_stylesheet( $stylesheet, $options = array() ) { $parsed = null; $cache_key = null; - $cache_group = 'amp-parsed-stylesheet-v12'; // This should be bumped whenever the PHP-CSS-Parser is updated. + $cache_group = 'amp-parsed-stylesheet-v13'; // This should be bumped whenever the PHP-CSS-Parser is updated. $cache_impacting_options = array_merge( wp_array_slice_assoc( @@ -2125,9 +2125,12 @@ private function finalize_stylesheet_set( $stylesheet_set ) { for ( $j = $i + 1; $j < $stylesheet_part_count; $j++ ) { $stylesheet_part = $stylesheet_parts[ $j ]; $is_at_rule = '@' === substr( $stylesheet_part, 0, 1 ); - if ( $is_at_rule && '{' === substr( $stylesheet_part, -1 ) ) { + if ( empty( $stylesheet_part ) ) { + continue; // There was a shaken rule. + } elseif ( $is_at_rule && '{}' === substr( $stylesheet_part, -2 ) ) { + continue; // The rule opens is empty from the start. + } elseif ( $is_at_rule && '{' === substr( $stylesheet_part, -1 ) ) { $open_braces++; - } elseif ( '}' === $stylesheet_part ) { $open_braces--; } else { @@ -2139,7 +2142,7 @@ private function finalize_stylesheet_set( $stylesheet_set ) { array_splice( $stylesheet_parts, $i, $j - $i + 1 ); $stylesheet_part_count = count( $stylesheet_parts ); $i--; - continue; + continue 2; } } } diff --git a/tests/test-amp-style-sanitizer.php b/tests/test-amp-style-sanitizer.php index ee0fcc34918..7d2681b94f4 100644 --- a/tests/test-amp-style-sanitizer.php +++ b/tests/test-amp-style-sanitizer.php @@ -843,6 +843,7 @@ public function test_large_custom_css_and_rule_removal() { $html .= ''; $html .= ''; $html .= ''; + $html .= ''; $html .= ''; $html .= '