Adapt heights, sizes and media attributes for SSR

lib/common/composer.json

@@ -8,11 +8,11 @@
     "ext-dom": "*",
     "ext-iconv": "*",
     "ext-json": "*",
-    "ext-libxml": "*"
+    "ext-libxml": "*",
+    "php-parallel-lint/php-parallel-lint": "^1.2"
   },
   "require-dev": {
     "civicrm/composer-downloads-plugin": "^2.1",
-    "jakub-onderka/php-parallel-lint": "^1.0",
     "roave/security-advisories": "dev-master",
     "squizlabs/php_codesniffer": "^3"
   },

lib/optimizer/composer.json

@@ -8,12 +8,12 @@
     "ext-dom": "*",
     "ext-iconv": "*",
     "ext-libxml": "*",
-    "ampproject/common": "^1"
+    "ampproject/common": "^1",
+    "php-parallel-lint/php-parallel-lint": "^1.2"
   },
   "require-dev": {
     "ext-zip": "*",
     "civicrm/composer-downloads-plugin": "^2.1",
-    "jakub-onderka/php-parallel-lint": "^1.0",
     "roave/security-advisories": "dev-master",
     "squizlabs/php_codesniffer": "^3"
   },

lib/optimizer/resources/local_fallback/rtv/metadata

@@ -1 +1 @@
-{"ampRuntimeVersion":"012004252135000","ampCssUrl":"https://cdn.ampproject.org/rtv/012004252135000/v0.css","canaryPercentage":"0.005","diversions":["002004252135000","002005050322000","022004252135000","032004252135000","032005050322000","042005051629000","052004252135000","102004252135000"],"ltsRuntimeVersion":"012004030010070","ltsCssUrl":"https://cdn.ampproject.org/rtv/012004030010070/v0.css"}
+{"ampRuntimeVersion":"012004252135000","ampCssUrl":"https://cdn.ampproject.org/rtv/012004252135000/v0.css","canaryPercentage":"0.005","diversions":["002005050322000","022004252135000","032005050322000","042005062312000","052004252135000","102005050322000"],"ltsRuntimeVersion":"012004030010070","ltsCssUrl":"https://cdn.ampproject.org/rtv/012004030010070/v0.css"}

lib/optimizer/src/Transformer/ServerSideRendering.php

@@ -122,6 +122,13 @@ final class ServerSideRendering implements Transformer
      */
     private $ampCustomCssByteCount = 0;
 
+    /**
+     * Associative array of custom sizer styles where the key is the ID of the associated element.
+     *
+     * @var string[]
+     */
+    private $customSizerStyles = [];
+
     /**
      * Apply transformations to the provided DOM document.
      *
@@ -557,7 +564,12 @@ private function maybeAddSizerInto(
         $sizer = null;
 
         if ($layout === Layout::RESPONSIVE) {
-            $sizer = $this->createResponsiveSizer($document, $width, $height);
+            $elementId = $element->getAttribute(Attribute::ID);
+            if (!empty($elementId) && array_key_exists($elementId, $this->customSizerStyles)) {
+                $sizer = $this->createResponsiveSizer($document, $width, $height, $this->customSizerStyles[$elementId]);
+            } else {
+                $sizer = $this->createResponsiveSizer($document, $width, $height);
+            }
         } elseif ($layout === Layout::INTRINSIC) {
             $sizer = $this->createIntrinsicSizer($document, $width, $height);
         }
@@ -573,13 +585,15 @@ private function maybeAddSizerInto(
      * @param Document  $document DOM document to create the sizer for.
      * @param CssLength $width    Calculated width of the element.
      * @param CssLength $height   Calculated height of the element.
+     * @param string    $style    Style to use for the sizer. Defaults to padding-top in percentage.
      * @return DOMElement
      */
-    private function createResponsiveSizer(Document $document, CssLength $width, CssLength $height)
+    private function createResponsiveSizer(Document $document, CssLength $width, CssLength $height, $style = 'padding-top:%1.4F%%;')
     {
         $padding = $height->getNumeral() / $width->getNumeral() * 100;
         $sizer   = $document->createElement(Amp::SIZER_ELEMENT);
-        $sizer->setAttribute(Tag::STYLE, sprintf('display:block;padding-top:%1.4F%%;', $padding));
+        $style   = empty($style) ? 'display:block' : "display:block;{$style}";
+        $sizer->setAttribute(Tag::STYLE, sprintf($style, $padding));
 
         return $sizer;
     }
@@ -743,11 +757,10 @@ private function adaptBlockingAttributes(Document $document, DOMElement $ampElem
                 $method = self::EXTRACTION_METHOD_MAP[$normalizedAttributeName];
 
                 try {
-                    $customCss            .= $this->$method($document, $ampElement, $attribute->value);
+                    $customCss           .= $this->$method($document, $ampElement, $attribute->name, $attribute->value);
                     $attributesToRemove[] = $attribute->name;
-                    $ampElement->removeAttribute($attribute->name);
                 } catch (Exception $exception) {
-                    $errors->add(Error\CannotRemoveBoilerplate::fromAttributesRequiringBoilerplate($ampElement));
+                    $errors->add(Error\CannotRemoveBoilerplate::fromAttributeThrowingException($exception));
                     return false;
                 }
             }
@@ -814,10 +827,11 @@ private function addCustomCss(Document $document, $customCss)
      *
      * @param Document   $document       Document containing the element to adapt.
      * @param DOMElement $element        Element to adapt.
+     * @param string     $attributeName  Name of the attribute to be extracted.
      * @param string     $attributeValue Value of the attribute to extract the CSS styling from.
      * @return string Extract custom CSS styling.
      */
-    private function extractSizesAttributeCss(Document $document, DOMElement $element, $attributeValue)
+    private function extractSizesAttributeCss(Document $document, DOMElement $element, $attributeName, $attributeValue)
     {
         if (!$element->hasAttribute(Attribute::SRCSET) || empty($element->getAttribute(Attribute::SRCSET))) {
             // According to the Mozilla docs, a sizes attribute without a valid srcset attribute should have no effect.
@@ -829,9 +843,10 @@ private function extractSizesAttributeCss(Document $document, DOMElement $elemen
         return $this->extractAttributeCss(
             $document,
             $element,
+            $attributeName,
             $attributeValue,
-            '#__ID__{width:%s};',
-            '@media %s{#__ID__{width:%s;}}'
+            '#__ID__{width:%s}',
+            '@media %s{#__ID__{width:%s}}'
         );
     }
 
@@ -849,17 +864,21 @@ private function extractSizesAttributeCss(Document $document, DOMElement $elemen
      *
      * @param Document   $document       Document containing the element to adapt.
      * @param DOMElement $element        Element to adapt.
+     * @param string     $attributeName  Name of the attribute to be extracted.
      * @param string     $attributeValue Value of the attribute to extract the CSS styling from.
      * @return string Extract custom CSS styling.
      */
-    private function extractHeightsAttributeCss(Document $document, DOMElement $element, $attributeValue)
+    private function extractHeightsAttributeCss(Document $document, DOMElement $element, $attributeName, $attributeValue)
     {
+        $this->customSizerStyles[$document->getElementId($element)] = '';
+
         return $this->extractAttributeCss(
             $document,
             $element,
+            $attributeName,
             $attributeValue,
-            '#__ID__>i-amphtml-sizer{height:%s};',
-            '@media %s{#__ID__>i-amphtml-sizer{height:%s;}}'
+            '#__ID__>:first-child{padding-top:%s}',
+            '@media %s{#__ID__>:first-child{padding-top:%s}}'
         );
     }
 
@@ -868,12 +887,13 @@ private function extractHeightsAttributeCss(Document $document, DOMElement $elem
      *
      * @param Document   $document        Document containing the element to adapt.
      * @param DOMElement $element         Element to adapt.
+     * @param string     $attributeName   Name of the attribute to be extracted.
      * @param string     $attributeValue  Value of the attribute to extract the CSS styling from.
      * @param string     $mainStyle       CSS template for the main style.
      * @param string     $mediaQueryStyle CSS template for a media query style.
      * @return string Extract custom CSS styling.
      */
-    private function extractAttributeCss(Document $document, DOMElement $element, $attributeValue, $mainStyle, $mediaQueryStyle)
+    private function extractAttributeCss(Document $document, DOMElement $element, $attributeName, $attributeValue, $mainStyle, $mediaQueryStyle)
     {
         if (empty($attributeValue)) {
             return '';
@@ -883,7 +903,7 @@ private function extractAttributeCss(Document $document, DOMElement $element, $a
         $lastItem    = trim(array_pop($sourceSizes), self::CSS_TRIM_CHARACTERS);
 
         if (empty($lastItem)) {
-            throw InvalidHtmlAttribute::fromAttribute(Attribute::HEIGHTS, $element);
+            throw InvalidHtmlAttribute::fromAttribute($attributeName, $element);
         }
 
         $customCss = sprintf($mainStyle, $lastItem);
@@ -894,13 +914,13 @@ private function extractAttributeCss(Document $document, DOMElement $element, $a
                 $mediaCondition = trim($matches['media_condition'], self::CSS_TRIM_CHARACTERS);
 
                 if (empty($mediaCondition)) {
-                    throw InvalidHtmlAttribute::fromAttribute(Attribute::HEIGHTS, $element);
+                    throw InvalidHtmlAttribute::fromAttribute($attributeName, $element);
                 }
 
                 $dimension = trim($matches['dimension'], self::CSS_TRIM_CHARACTERS);
 
                 if (empty($dimension)) {
-                    throw InvalidHtmlAttribute::fromAttribute(Attribute::HEIGHTS, $element);
+                    throw InvalidHtmlAttribute::fromAttribute($attributeName, $element);
                 }
 
                 $customCss .= sprintf($mediaQueryStyle, $mediaCondition, $dimension);
@@ -923,10 +943,11 @@ private function extractAttributeCss(Document $document, DOMElement $element, $a
      *
      * @param Document   $document       Document containing the element to adapt.
      * @param DOMElement $element        Element to adapt.
+     * @param string     $attributeName  Name of the attribute to be extracted.
      * @param string     $attributeValue Value of the attribute to extract the CSS styling from.
      * @return string Extract custom CSS styling.
      */
-    private function extractMediaAttributeCss(Document $document, DOMElement $element, $attributeValue)
+    private function extractMediaAttributeCss(Document $document, DOMElement $element, $attributeName, $attributeValue)
     {
         $attributeValue = trim($attributeValue, self::CSS_TRIM_CHARACTERS);
 
@@ -939,7 +960,7 @@ private function extractMediaAttributeCss(Document $document, DOMElement $elemen
         }
 
         return sprintf(
-            '@media not %s{#%s{display:none;}}',
+            '@media not %s{#%s{display:none}}',
             $attributeValue,
             $document->getElementId($element)
         );

lib/optimizer/tests/SpecTest.php

@@ -33,9 +33,6 @@ final class SpecTest extends TestCase
 
         'ServerSideRendering - noscript_then_boilerplate_not_removed_due_to_attribute' => 'see https://github.com/ampproject/amp-wp/issues/4439',
         'ServerSideRendering - boilerplate_then_noscript_not_removed_due_to_attribute' => 'see https://github.com/ampproject/amp-wp/issues/4439',
-
-        'AmpRuntimeCss - always_inlines_v0css'                                 => 'see https://github.com/ampproject/amp-wp/issues/4654',
-        'AmpRuntimeCss - does_not_add_v0.css_if_style_amp-runtime_not_present' => 'see https://github.com/ampproject/amp-wp/issues/4654',
     ];
 
     const CLASS_SKIP_TEST = '__SKIP__';