From 9d0e2adaa0f2c4193a42c8c62aa7a6c8f66c73db Mon Sep 17 00:00:00 2001 From: Pierre Gordon Date: Wed, 8 Apr 2020 16:27:31 -0400 Subject: [PATCH 1/9] Prevent styles from being removed when in Customizer preview with Standard mode enabled --- includes/class-amp-theme-support.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 47170285c1b..d3fdea6fcb2 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -2197,6 +2197,11 @@ private static function get_optimizer_configuration( $args ) { ); } + // Prevent styles from being removed when in Customizer preview and Standard mode is enabled. + if ( ! empty( $args['allow_dirty_styles'] ) && amp_is_canonical() ) { + $transformers = array_diff( $transformers, [ Optimizer\Transformer\AmpBoilerplate::class ] ); + } + array_unshift( $transformers, Transformer\AmpSchemaOrgMetadata::class ); /** From b7d273a421a8bf4d6df9b388f4fee84c9e2fe3af Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Apr 2020 21:11:07 -0700 Subject: [PATCH 2/9] Revert "Prevent styles from being removed when in Customizer preview with Standard mode enabled" This reverts commit 9d0e2adaa0f2c4193a42c8c62aa7a6c8f66c73db. --- includes/class-amp-theme-support.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index d3fdea6fcb2..47170285c1b 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -2197,11 +2197,6 @@ private static function get_optimizer_configuration( $args ) { ); } - // Prevent styles from being removed when in Customizer preview and Standard mode is enabled. - if ( ! empty( $args['allow_dirty_styles'] ) && amp_is_canonical() ) { - $transformers = array_diff( $transformers, [ Optimizer\Transformer\AmpBoilerplate::class ] ); - } - array_unshift( $transformers, Transformer\AmpSchemaOrgMetadata::class ); /** From 00dc3ce8aa2ab47bd5c8a6acdf92aac35f82f3f2 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Apr 2020 22:08:05 -0700 Subject: [PATCH 3/9] Re-work AmpBoilerplate to only remove boilerplate-specific styles --- .../src/Transformer/AmpBoilerplate.php | 25 +++-- .../tests/Transformer/AmpBoilerplateTest.php | 102 +++++++++--------- 2 files changed, 65 insertions(+), 62 deletions(-) diff --git a/lib/optimizer/src/Transformer/AmpBoilerplate.php b/lib/optimizer/src/Transformer/AmpBoilerplate.php index e49d6ddfc2d..b3a60584344 100644 --- a/lib/optimizer/src/Transformer/AmpBoilerplate.php +++ b/lib/optimizer/src/Transformer/AmpBoilerplate.php @@ -12,8 +12,8 @@ use DOMElement; /** - * Transformer that removes ' . @@ -57,65 +46,76 @@ public function dataTransform() $amp4EmailBoilerplate = ''; return [ - 'keeps boilerplate' => [ - $inputWithBoilerplate('', $ampBoilerplate), - $expected('', $ampBoilerplate), - ], + 'keeps boilerplate' => $repeatTwice( + $htmlDocument('', $ampBoilerplate), + ), - 'keeps boilerplate for amp4ads' => [ - $inputWithBoilerplate('', $amp4AdsBoilerplate), - $expected('', $amp4AdsBoilerplate), - ], + 'keeps boilerplate again' => $repeatTwice( + $htmlDocument('', $ampBoilerplate), + ), - 'keeps boilerplate for ⚡4ads' => [ - $inputWithBoilerplate('', $amp4AdsBoilerplate), - $expected('', $amp4AdsBoilerplate), - ], + 'leaves out boilerplate' => $repeatTwice( + $htmlDocument(''), + ), - 'keeps boilerplate for amp4email' => [ - $inputWithBoilerplate('', $amp4EmailBoilerplate), - $expected('', $amp4EmailBoilerplate), + 'removes boilerplate' => [ + $htmlDocument('', $ampBoilerplate), + $htmlDocument(''), ], - 'keeps boilerplate for ⚡4email' => [ - $inputWithBoilerplate('', $amp4EmailBoilerplate), - $expected('', $amp4EmailBoilerplate), - ], + 'keeps boilerplate for amp4ads' => $repeatTwice( + $htmlDocument('', $amp4AdsBoilerplate) + ), + + 'keeps boilerplate for ⚡4ads' => $repeatTwice( + $htmlDocument('', $amp4AdsBoilerplate) + ), + + 'keeps boilerplate for amp4email' => $repeatTwice( + $htmlDocument('', $amp4EmailBoilerplate) + ), + + 'keeps boilerplate for ⚡4email' => $repeatTwice( + $htmlDocument('', $amp4EmailBoilerplate) + ), 'adds boilerplate if missing' => [ - $inputWithoutBoilerplate(''), - $expected('', $ampBoilerplate), + $htmlDocument(''), + $htmlDocument('', $ampBoilerplate), ], 'adds boilerplate if missing for amp4ads' => [ - $inputWithoutBoilerplate(''), - $expected('', $amp4AdsBoilerplate), + $htmlDocument(''), + $htmlDocument('', $amp4AdsBoilerplate), ], 'adds boilerplate if missing for ⚡4ads' => [ - $inputWithoutBoilerplate(''), - $expected('', $amp4AdsBoilerplate), + $htmlDocument(''), + $htmlDocument('', $amp4AdsBoilerplate), ], 'adds boilerplate if missing for amp4email' => [ - $inputWithoutBoilerplate(''), - $expected('', $amp4EmailBoilerplate), + $htmlDocument(''), + $htmlDocument('', $amp4EmailBoilerplate), ], 'adds boilerplate if missing for ⚡4email' => [ - $inputWithoutBoilerplate(''), - $expected('', $amp4EmailBoilerplate), + $htmlDocument(''), + $htmlDocument('', $amp4EmailBoilerplate), ], - 'keeps devmode nodes when in devmode' => [ - $inputWithBoilerplate('', '' . $ampBoilerplate), - $expected('', '' . $ampBoilerplate), - ], + 'leaves styles that lack boilerplate attribute' => $repeatTwice( + $htmlDocument('', '' . $ampBoilerplate), + ), - 'removes devmode nodes when not in devmode' => [ - $inputWithBoilerplate('', '' . $ampBoilerplate), - $expected('', $ampBoilerplate), + 'leaves styles that lack boilerplate attribute and adds boilerplate' => [ + $htmlDocument('', ''), + $htmlDocument('', '' . $ampBoilerplate), ], + + 'leaves styles that lack boilerplate attribute and leaves out boilerplate' => $repeatTwice( + $htmlDocument('', ''), + ), ]; } From b3fb07890ef4fef753942ff5c260c9c4f73e229a Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Apr 2020 22:29:04 -0700 Subject: [PATCH 4/9] Remove now-unused AmpProject\DevMode --- lib/optimizer/src/Transformer/AmpBoilerplate.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/optimizer/src/Transformer/AmpBoilerplate.php b/lib/optimizer/src/Transformer/AmpBoilerplate.php index b3a60584344..5db3eb6bd22 100644 --- a/lib/optimizer/src/Transformer/AmpBoilerplate.php +++ b/lib/optimizer/src/Transformer/AmpBoilerplate.php @@ -4,7 +4,6 @@ use AmpProject\Amp; use AmpProject\Attribute; -use AmpProject\DevMode; use AmpProject\Dom\Document; use AmpProject\Optimizer\ErrorCollection; use AmpProject\Optimizer\Transformer; From d8fc048be3d0d526603b0a4859e4d5168fdacf55 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Apr 2020 22:31:06 -0700 Subject: [PATCH 5/9] Improve phpdoc explaining refined behavior --- lib/optimizer/src/Transformer/AmpBoilerplate.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/optimizer/src/Transformer/AmpBoilerplate.php b/lib/optimizer/src/Transformer/AmpBoilerplate.php index 5db3eb6bd22..03ade0c986a 100644 --- a/lib/optimizer/src/Transformer/AmpBoilerplate.php +++ b/lib/optimizer/src/Transformer/AmpBoilerplate.php @@ -12,7 +12,7 @@ /** * Transformer that removes AMP boilerplate ' . $ampBoilerplate), + $htmlDocument('', '' . $ampBoilerplate) ), 'leaves styles that lack boilerplate attribute and adds boilerplate' => [ @@ -114,7 +114,7 @@ public function dataTransform() ], 'leaves styles that lack boilerplate attribute and leaves out boilerplate' => $repeatTwice( - $htmlDocument('', ''), + $htmlDocument('', '') ), ]; } From 4f81f561ad975e0dee8aaefc350127164172c0b1 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Apr 2020 23:29:17 -0700 Subject: [PATCH 7/9] Simplify logic in removeStyleAndNoscriptTags --- .../src/Transformer/AmpBoilerplate.php | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/lib/optimizer/src/Transformer/AmpBoilerplate.php b/lib/optimizer/src/Transformer/AmpBoilerplate.php index 03ade0c986a..d9b5fc0d5a4 100644 --- a/lib/optimizer/src/Transformer/AmpBoilerplate.php +++ b/lib/optimizer/src/Transformer/AmpBoilerplate.php @@ -74,26 +74,20 @@ public function transform(Document $document, ErrorCollection $errors) */ private function removeStyleAndNoscriptTags(Document $document, $boilerplate) { - $headNode = $document->head->firstChild; - while ($headNode) { - $nextSibling = $headNode->nextSibling; - if ($headNode instanceof DOMElement) { - switch ($headNode->tagName) { - case Tag::STYLE: - if ($headNode->hasAttribute($boilerplate)) { - $document->head->removeChild($headNode); - } - break; - case Tag::NOSCRIPT: - $style = $headNode->getElementsByTagName('style')->item(0); - if ($style instanceof DOMElement && $style->hasAttribute($boilerplate)) { - $document->head->removeChild($headNode); - } - break; - } + /** + * Style element. + * + * @var DOMElement $style + */ + foreach (iterator_to_array($document->head->getElementsByTagName('style')) as $style) { + if (! $style->hasAttribute($boilerplate)) { + continue; + } + if (Tag::NOSCRIPT === $style->parentNode->nodeName) { + $style->parentNode->parentNode->removeChild($style->parentNode); + } else { + $style->parentNode->removeChild($style); } - - $headNode = $nextSibling; } } From 67620543870fa8fb576dfc221de13ca7e1d17beb Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 9 Apr 2020 08:22:30 -0700 Subject: [PATCH 8/9] Remove all boilerplates not just the identified one --- lib/common/src/Attribute.php | 2 ++ .../src/Transformer/AmpBoilerplate.php | 30 ++++++++++++++----- .../tests/Transformer/AmpBoilerplateTest.php | 5 ++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/lib/common/src/Attribute.php b/lib/common/src/Attribute.php index 8321e408596..a7e4df5b05a 100644 --- a/lib/common/src/Attribute.php +++ b/lib/common/src/Attribute.php @@ -59,6 +59,8 @@ interface Attribute const ALL_AMP4ADS = [self::AMP4ADS, self::AMP4ADS_EMOJI, self::AMP4ADS_EMOJI_ALT]; const ALL_AMP4EMAIL = [self::AMP4EMAIL, self::AMP4EMAIL_EMOJI, self::AMP4EMAIL_EMOJI_ALT]; + const ALL_BOILERPLATES = [self::AMP_BOILERPLATE, self::AMP4ADS_BOILERPLATE, self::AMP4EMAIL_BOILERPLATE]; + const TYPE_HTML = 'text/html'; const TYPE_JSON = 'application/json'; const TYPE_LD_JSON = 'application/ld+json'; diff --git a/lib/optimizer/src/Transformer/AmpBoilerplate.php b/lib/optimizer/src/Transformer/AmpBoilerplate.php index d9b5fc0d5a4..107cb9320de 100644 --- a/lib/optimizer/src/Transformer/AmpBoilerplate.php +++ b/lib/optimizer/src/Transformer/AmpBoilerplate.php @@ -35,14 +35,14 @@ final class AmpBoilerplate implements Transformer */ public function transform(Document $document, ErrorCollection $errors) { - list($boilerplate, $css) = $this->determineBoilerplateAndCss($document->html); - - $this->removeStyleAndNoscriptTags($document, $boilerplate); + $this->removeStyleAndNoscriptTags($document); if ($this->hasNoBoilerplateAttribute($document)) { return; } + list($boilerplate, $css) = $this->determineBoilerplateAndCss($document->html); + $styleNode = $document->createElement(Tag::STYLE); $styleNode->setAttribute($boilerplate, ''); $document->head->appendChild($styleNode); @@ -69,10 +69,9 @@ public function transform(Document $document, ErrorCollection $errors) /** * Remove all