✨ Validator support for transformed intrinsic layout (ampproject#24119)

Validator support for transformed intrinsic layout Intrinsic layout SSR adds the following sizer element: ```html <amp-img height="100" width="300" layout="intrinsic" class="i-amphtml-layout-intrinsic i-amphtml-layout-size-defined" i-amphtml-layout="intrinsic"> <i-amphtml-sizer class="i-amphtml-sizer"> <img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;charset=utf-8,<svg height="100" width="300" xmlns="http://www.w3.org/2000/svg" version="1.1"/>"> </i-amphtml-sizer> </amp-img> ``` This commit extends the `i-amphtml-sizer` validation rules to alternatively support the intrinsic sizer element. The SVG sizer image is validated via a regex checking if the `src` value is an SVG.
micajuine-ho · Dec 27, 2019 · db3e74a · db3e74a
1 parent d90b3c6
commit db3e74a
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 13 deletions.
diff --git a/validator/engine/validator.js b/validator/engine/validator.js
@@ -4404,8 +4404,8 @@ function validateAttributes(
   const attrsByName = parsedTagSpec.getAttrsByName();
   for (const attr of encounteredTag.attrs()) {
     // For transformed AMP, attributes `class` and `i-amphtml-layout` are
-    // handled within validateSsrLayout.
-    if (context.isTransformed() &&
+    // handled within validateSsrLayout for non-sizer elements.
+    if (context.isTransformed() && encounteredTag.lowerName() !== 'i-amphtml-sizer' &&
         (attr.name === 'class' || attr.name === 'i-amphtml-layout')) {
       continue;
     }

diff --git a/validator/testdata/transformed_feature_tests/server_side_rendering.html b/validator/testdata/transformed_feature_tests/server_side_rendering.html
@@ -17,7 +17,8 @@
   Test Description:
   Test for server side rendering transformer:
    - html tag attributes i-amphtml-layout and i-amphtml-no-boilerplate present
-   - i-amphtml-sizer tag with style attribute
+   - responsive layout: i-amphtml-sizer tag with style attribute
+   - intrinsic layout: i-amphtml-sizer tag with img tag
 -->
 <!doctype html>
 <html ⚡ transformed="google;v=1" i-amphtml-layout i-amphtml-no-boilerplate>
@@ -35,6 +36,20 @@
     <i-amphtml-sizer style=display:block;padding-top:171.4370%;></i-amphtml-sizer>
   </amp-img>
   <!-- Valid -->
+  <amp-img src="image.png" height="100" width="300" layout="intrinsic" class="i-amphtml-layout-intrinsic i-amphtml-layout-size-defined" i-amphtml-layout="intrinsic">
+    <i-amphtml-sizer class="i-amphtml-sizer"><img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;charset=utf-8,<svg height=&quot;100&quot; width=&quot;300&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>"></i-amphtml-sizer>
+  </amp-img>
+  <!-- Invalid i-amphtml-sizer > img does not specify an svg -->
+  <amp-img src="image.png" height="100" width="300" layout="intrinsic" class="i-amphtml-layout-intrinsic i-amphtml-layout-size-defined" i-amphtml-layout="intrinsic">
+    <i-amphtml-sizer class="i-amphtml-sizer"><img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="image.png"></i-amphtml-sizer>
+  </amp-img>
+  <!-- Invalid intrinsic i-amphtml-sizer inside responsive sizer  -->
+  <amp-img src="image.png" height="100" width="300" layout="intrinsic" class="i-amphtml-layout-intrinsic i-amphtml-layout-size-defined" i-amphtml-layout="intrinsic">
+    <i-amphtml-sizer style=display:block;padding-top:171.4370%;>
+      <img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;charset=utf-8,<svg height=&quot;100&quot; width=&quot;300&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>">
+    </i-amphtml-sizer>
+  </amp-img>
+  <!-- Valid -->
   <amp-social-share class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" i-amphtml-layout=fixed style=width:60px;height:44px; type=test></amp-social-share>
   <!-- Invalid i-amphtml-layout attribute value does not match layout value -->
   <amp-img class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" height=2911 i-amphtml-layout=nodisplay layout=responsive src=https://example-com.cdn.ampproject.org/i/s/example.com/lemur-narrow.jpg srcset="https://example-com.cdn.ampproject.org/i/s/example.com/lemur-wide.jpg 640w, https://example-com.cdn.ampproject.org/i/s/example.com/lemur-narrow.jpg 320w" width=1698>

diff --git a/validator/testdata/transformed_feature_tests/server_side_rendering.out b/validator/testdata/transformed_feature_tests/server_side_rendering.out
@@ -18,7 +18,8 @@ FAIL
 |    Test Description:
 |    Test for server side rendering transformer:
 |     - html tag attributes i-amphtml-layout and i-amphtml-no-boilerplate present
-|     - i-amphtml-sizer tag with style attribute
+|     - responsive layout: i-amphtml-sizer tag with style attribute
+|     - intrinsic layout: i-amphtml-sizer tag with img tag
 |  -->
 |  <!doctype html>
 |  <html ⚡ transformed="google;v=1" i-amphtml-layout i-amphtml-no-boilerplate>
@@ -36,35 +37,53 @@ FAIL
 |      <i-amphtml-sizer style=display:block;padding-top:171.4370%;></i-amphtml-sizer>
 |    </amp-img>
 |    <!-- Valid -->
+|    <amp-img src="image.png" height="100" width="300" layout="intrinsic" class="i-amphtml-layout-intrinsic i-amphtml-layout-size-defined" i-amphtml-layout="intrinsic">
+|      <i-amphtml-sizer class="i-amphtml-sizer"><img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;charset=utf-8,<svg height=&quot;100&quot; width=&quot;300&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>"></i-amphtml-sizer>
+|    </amp-img>
+|    <!-- Invalid i-amphtml-sizer > img does not specify an svg -->
+|    <amp-img src="image.png" height="100" width="300" layout="intrinsic" class="i-amphtml-layout-intrinsic i-amphtml-layout-size-defined" i-amphtml-layout="intrinsic">
+|      <i-amphtml-sizer class="i-amphtml-sizer"><img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="image.png"></i-amphtml-sizer>
+>>                                              ^~~~~~~~~
+transformed_feature_tests/server_side_rendering.html:44:45 The attribute 'src' in tag 'IMG-I-AMPHTML-INTRINSIC-SIZER' is set to the invalid value 'image.png'.
+|    </amp-img>
+|    <!-- Invalid intrinsic i-amphtml-sizer inside responsive sizer  -->
+|    <amp-img src="image.png" height="100" width="300" layout="intrinsic" class="i-amphtml-layout-intrinsic i-amphtml-layout-size-defined" i-amphtml-layout="intrinsic">
+|      <i-amphtml-sizer style=display:block;padding-top:171.4370%;>
+|        <img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;charset=utf-8,<svg height=&quot;100&quot; width=&quot;300&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>">
+>>       ^~~~~~~~~
+transformed_feature_tests/server_side_rendering.html:49:6 The parent tag of tag 'IMG-I-AMPHTML-INTRINSIC-SIZER' is 'i-amphtml-sizer', but it can only be 'i-amphtml-sizer-intrinsic'.
+|      </i-amphtml-sizer>
+|    </amp-img>
+|    <!-- Valid -->
 |    <amp-social-share class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" i-amphtml-layout=fixed style=width:60px;height:44px; type=test></amp-social-share>
 |    <!-- Invalid i-amphtml-layout attribute value does not match layout value -->
 |    <amp-img class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" height=2911 i-amphtml-layout=nodisplay layout=responsive src=https://example-com.cdn.ampproject.org/i/s/example.com/lemur-narrow.jpg srcset="https://example-com.cdn.ampproject.org/i/s/example.com/lemur-wide.jpg 640w, https://example-com.cdn.ampproject.org/i/s/example.com/lemur-narrow.jpg 320w" width=1698>
 >>   ^~~~~~~~~
-transformed_feature_tests/server_side_rendering.html:40:2 Invalid value 'nodisplay' for attribute 'i-amphtml-layout' in tag 'amp-img' - for layout 'RESPONSIVE', set the attribute 'i-amphtml-layout' to value 'responsive'. (see https://amp.dev/documentation/components/amp-img)
+transformed_feature_tests/server_side_rendering.html:55:2 Invalid value 'nodisplay' for attribute 'i-amphtml-layout' in tag 'amp-img' - for layout 'RESPONSIVE', set the attribute 'i-amphtml-layout' to value 'responsive'. (see https://amp.dev/documentation/components/amp-img)
 |      <i-amphtml-sizer style=display:block;padding-top:171.4370%;></i-amphtml-sizer>
 |    </amp-img>
 |    <!-- Invalid class attribute value due to not matching layout value -->
 |    <amp-img class="i-amphtml-layout-nodisplay i-amphtml-layout-size-defined" height=2911 i-amphtml-layout=responsive layout=responsive src=https://example-com.cdn.ampproject.org/i/s/example.com/lemur-narrow.jpg srcset="https://example-com.cdn.ampproject.org/i/s/example.com/lemur-wide.jpg 640w, https://example-com.cdn.ampproject.org/i/s/example.com/lemur-narrow.jpg 320w" width=1698>
 >>   ^~~~~~~~~
-transformed_feature_tests/server_side_rendering.html:44:2 The attribute 'class' in tag 'amp-img' is set to the invalid value 'i-amphtml-layout-nodisplay i-amphtml-layout-size-defined'. (see https://amp.dev/documentation/components/amp-img)
+transformed_feature_tests/server_side_rendering.html:59:2 The attribute 'class' in tag 'amp-img' is set to the invalid value 'i-amphtml-layout-nodisplay i-amphtml-layout-size-defined'. (see https://amp.dev/documentation/components/amp-img)
 |      <i-amphtml-sizer style=display:block;padding-top:171.4370%;></i-amphtml-sizer>
 |    </amp-img>
 |    <!-- Invalid class attribute value due to layout not being size defined (spaces) -->
 |    <amp-img class="i-amphtml-layout-nodisplay i-amphtml-layout-size-defined" i-amphtml-layout=nodisplay layout=nodisplay></amp-img>
 >>   ^~~~~~~~~
-transformed_feature_tests/server_side_rendering.html:48:2 The attribute 'class' in tag 'amp-img' is set to the invalid value 'i-amphtml-layout-nodisplay i-amphtml-layout-size-defined'. (see https://amp.dev/documentation/components/amp-img)
+transformed_feature_tests/server_side_rendering.html:63:2 The attribute 'class' in tag 'amp-img' is set to the invalid value 'i-amphtml-layout-nodisplay i-amphtml-layout-size-defined'. (see https://amp.dev/documentation/components/amp-img)
 |    <!-- Invalid class attribute value due to layout not being size defined (tabs) -->
 |    <amp-img class="i-amphtml-layout-nodisplay  i-amphtml-layout-size-defined" i-amphtml-layout=nodisplay layout=nodisplay></amp-img>
 >>   ^~~~~~~~~
-transformed_feature_tests/server_side_rendering.html:50:2 The attribute 'class' in tag 'amp-img' is set to the invalid value 'i-amphtml-layout-nodisplay i-amphtml-layout-size-defined'. (see https://amp.dev/documentation/components/amp-img)
+transformed_feature_tests/server_side_rendering.html:65:2 The attribute 'class' in tag 'amp-img' is set to the invalid value 'i-amphtml-layout-nodisplay i-amphtml-layout-size-defined'. (see https://amp.dev/documentation/components/amp-img)
 |    <!-- Invalid i-amphtml-sizer due to css declarations -->
 |    <amp-img class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" height=2911 i-amphtml-layout=responsive layout=responsive src=https://example-com.cdn.ampproject.org/i/s/example.com/lemur-narrow.jpg srcset="https://example-com.cdn.ampproject.org/i/s/example.com/lemur-wide.jpg 640w, https://example-com.cdn.ampproject.org/i/s/example.com/lemur-narrow.jpg 320w" width=1698>
 |      <i-amphtml-sizer style=display:none;padding-bottom:171.4370%;></i-amphtml-sizer>
 >>     ^~~~~~~~~
-transformed_feature_tests/server_side_rendering.html:53:4 CSS syntax error in tag 'i-amphtml-sizer' - the property 'display' is set to the disallowed value 'none'. (see https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/style_pages)
+transformed_feature_tests/server_side_rendering.html:68:4 CSS syntax error in tag 'I-AMPHTML-SIZER-RESPONSIVE' - the property 'display' is set to the disallowed value 'none'. (see https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/style_pages)
 >>     ^~~~~~~~~
-transformed_feature_tests/server_side_rendering.html:53:4 The property 'padding-bottom' in attribute 'style' in tag 'i-amphtml-sizer' is disallowed. (see https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/style_pages)
+transformed_feature_tests/server_side_rendering.html:68:4 The property 'padding-bottom' in attribute 'style' in tag 'I-AMPHTML-SIZER-RESPONSIVE' is disallowed. (see https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/style_pages)
 |    </amp-img>
 |
 |  </body>
-|  </html>
+|  </html>
diff --git a/validator/testdata/transformed_feature_tests/transformed_but_not_identified_transformed.out b/validator/testdata/transformed_feature_tests/transformed_but_not_identified_transformed.out
@@ -44,7 +44,7 @@ transformed_feature_tests/transformed_but_not_identified_transformed.html:31:2 T
 transformed_feature_tests/transformed_but_not_identified_transformed.html:31:2 The attribute 'i-amphtml-layout' may not appear in tag 'amp-img'. (see https://amp.dev/documentation/components/amp-img)
 |      <i-amphtml-sizer style=display:block;padding-top:171.4370%;></i-amphtml-sizer>
 >>     ^~~~~~~~~
-transformed_feature_tests/transformed_but_not_identified_transformed.html:32:4 The tag 'i-amphtml-sizer' is disallowed.
+transformed_feature_tests/transformed_but_not_identified_transformed.html:32:4 The tag 'i-amphtml-sizer' is disallowed except in specific forms.
 |    </amp-img>
 |  </body>
 |  </html>
@@ -53,4 +53,4 @@ transformed_feature_tests/transformed_but_not_identified_transformed.html:35:6 T
 >>       ^~~~~~~~~
 transformed_feature_tests/transformed_but_not_identified_transformed.html:35:6 The mandatory tag 'noscript > style[amp-boilerplate]' is missing or incorrect. (see https://amp.dev/documentation/guides-and-tutorials/learn/spec/amp-boilerplate?format=websites)
 >>       ^~~~~~~~~
-transformed_feature_tests/transformed_but_not_identified_transformed.html:35:6 The mandatory tag 'noscript enclosure for boilerplate' is missing or incorrect. (see https://amp.dev/documentation/guides-and-tutorials/learn/spec/amp-boilerplate?format=websites)
+transformed_feature_tests/transformed_but_not_identified_transformed.html:35:6 The mandatory tag 'noscript enclosure for boilerplate' is missing or incorrect. (see https://amp.dev/documentation/guides-and-tutorials/learn/spec/amp-boilerplate?format=websites)
diff --git a/validator/validator-main.protoascii b/validator/validator-main.protoascii
@@ -5320,9 +5320,11 @@ tags: {
   html_format: ACTIONS
   enabled_by: "transformed"
   tag_name: "I-AMPHTML-SIZER"
+  spec_name: "I-AMPHTML-SIZER-RESPONSIVE"
   explicit_attrs_only: true
   attrs: {
     name: "style"
+    dispatch_key: NAME_DISPATCH
     mandatory: true
     blacklisted_value_regex: "!\\s*important"
     css_declaration: {
@@ -5333,6 +5335,49 @@ tags: {
   }
 }
 
+tags: {
+  html_format: AMP
+  html_format: ACTIONS
+  enabled_by: "transformed"
+  tag_name: "I-AMPHTML-SIZER"
+  spec_name: "I-AMPHTML-SIZER-INTRINSIC"
+  explicit_attrs_only: true
+  attrs: {
+    name: "class"
+    value: "i-amphtml-sizer"
+    mandatory: true
+    dispatch_key: NAME_DISPATCH
+  }
+}
+
+tags: {
+  html_format: AMP
+  enabled_by: "transformed"
+  tag_name: "IMG"
+  spec_name: "IMG-I-AMPHTML-INTRINSIC-SIZER"
+  mandatory_parent: "I-AMPHTML-SIZER-INTRINSIC"
+  attrs: {
+    name: "alt"
+    value: ""
+    mandatory: true
+  }
+  attrs: {
+    name: "aria-hidden"
+    value: "true"
+    mandatory: true
+  }
+  attrs: {
+    name: "role"
+    value: "presentation"
+    mandatory: true
+  }
+  attrs: {
+    name: "src"
+    value_regex: "data:image\/svg\+xml;charset=utf-8,<svg height=\"\d+\" width=\"\d+\" xmlns=\"http:\/\/www\.w3\.org\/2000\/svg\" version=\"1\.1\"\/>"
+    mandatory: true
+  }
+}
+
 # AMP Layout attributes: implied for TagSpecs with amp_layout field
 # set. Note that while these attributes are defined as optional here,
 # depending on amp layout the validator will examine these fields and