AMP4Email: Disallow "latest" extension scripts (ampproject#24854)

* Disallow -latest.js alias in AMP4EMAIL. * Add feature test. * Update validator_test.js. * Add spec_name for amp-timeago, update # of versions test. * Fix no_latest_extension.out.
mcmd · Oct 7, 2019 · 69efec0 · 69efec0
1 parent 3c8afd3
commit 69efec0
Show file tree

Hide file tree

Showing 14 changed files with 222 additions and 31 deletions.
diff --git a/extensions/amp-accordion/validator-amp-accordion.protoascii b/extensions/amp-accordion/validator-amp-accordion.protoascii
@@ -16,7 +16,6 @@
 tags: {  # amp-accordion
   html_format: AMP
   html_format: AMP4ADS
-  html_format: AMP4EMAIL
   html_format: ACTIONS
   tag_name: "SCRIPT"
   extension_spec: {
@@ -28,6 +27,19 @@ tags: {  # amp-accordion
   }
   attr_lists: "common-extension-attrs"
 }
+tags: {  # amp-accordion
+  html_format: AMP4EMAIL
+  tag_name: "SCRIPT"
+  spec_name: "SCRIPT[custom-element=amp-accordion] (AMP4EMAIL)"
+  extension_spec: {
+    name: "amp-accordion"
+    # AMP4EMAIL doesn't allow version: "latest".
+    version: "0.1"
+    requires_usage: GRANDFATHERED
+    deprecated_allow_duplicates: true
+  }
+  attr_lists: "common-extension-attrs"
+}
 tags: {  # <amp-accordion>
   html_format: AMP
   html_format: AMP4ADS

diff --git a/extensions/amp-anim/validator-amp-anim.protoascii b/extensions/amp-anim/validator-amp-anim.protoascii
@@ -34,8 +34,8 @@ tags: {  # amp-anim
   spec_name: "amp-anim extension .js script (AMP4EMAIL)"
   extension_spec: {
     name: "amp-anim"
+    # AMP4EMAIL doesn't allow version: "latest".
     version: "0.1"
-    version: "latest"
   }
   attr_lists: "common-extension-attrs"
 }

diff --git a/extensions/amp-bind/validator-amp-bind.protoascii b/extensions/amp-bind/validator-amp-bind.protoascii
@@ -16,7 +16,6 @@
 tags: {  # amp-bind
   html_format: AMP
   html_format: AMP4ADS
-  html_format: AMP4EMAIL
   html_format: ACTIONS
   tag_name: "SCRIPT"
   extension_spec: {
@@ -30,6 +29,21 @@ tags: {  # amp-bind
   }
   attr_lists: "common-extension-attrs"
 }
+tags: {  # amp-bind
+  html_format: AMP4EMAIL
+  tag_name: "SCRIPT"
+  spec_name: "SCRIPT[custom-element=amp-bind] (AMP4EMAIL)"
+  extension_spec: {
+    name: "amp-bind"
+    # AMP4EMAIL doesn't allow version: "latest"
+    version: "0.1"
+    # amp-bind has no associated tag which indicates usage of the extension.
+    # TODO(gregable): Implement a mechanism to associate attributes with
+    # extension usage and then set this to GRANDFATHERED or ERROR.
+    requires_usage: NONE
+  }
+  attr_lists: "common-extension-attrs"
+}
 tags: {  # <amp-state> (json)
   html_format: AMP
   html_format: AMP4ADS

diff --git a/extensions/amp-carousel/validator-amp-carousel.protoascii b/extensions/amp-carousel/validator-amp-carousel.protoascii
@@ -35,6 +35,7 @@ tags: {  # amp-carousel for AMP4EMAIL
   spec_name: "SCRIPT[custom-element=amp-carousel] (AMP4EMAIL)"
   extension_spec: {
     name: "amp-carousel"
+    # AMP4EMAIL doesn't allow version: "latest".
     version: "0.1"
   }
   attr_lists: "common-extension-attrs"

diff --git a/extensions/amp-fit-text/validator-amp-fit-text.protoascii b/extensions/amp-fit-text/validator-amp-fit-text.protoascii
@@ -29,6 +29,22 @@ tags: {  # amp-fit-text
   }
   attr_lists: "common-extension-attrs"
 }
+tags: {  # amp-fit-text
+  html_format: AMP
+  html_format: AMP4ADS
+  html_format: AMP4EMAIL
+  html_format: ACTIONS
+  tag_name: "SCRIPT"
+  spec_name: "SCRIPT[custom-element=amp-fit-text] (AMP4EMAIL)"
+  extension_spec: {
+    name: "amp-fit-text"
+    # AMP4EMAIL doesn't allow version: "latest".
+    version: "0.1"
+    requires_usage: GRANDFATHERED
+    deprecated_allow_duplicates: true
+  }
+  attr_lists: "common-extension-attrs"
+}
 tags: {  # <amp-fit-text>
   html_format: AMP
   html_format: AMP4ADS

diff --git a/extensions/amp-form/validator-amp-form.protoascii b/extensions/amp-form/validator-amp-form.protoascii
@@ -18,7 +18,6 @@ tags: {  # amp-form
   # under section "4.10 Forms"
   html_format: AMP
   html_format: AMP4ADS
-  html_format: AMP4EMAIL
   html_format: ACTIONS
   tag_name: "SCRIPT"
   extension_spec: {
@@ -30,3 +29,16 @@ tags: {  # amp-form
   }
   attr_lists: "common-extension-attrs"
 }
+tags: {  # amp-form
+  html_format: AMP4EMAIL
+  tag_name: "SCRIPT"
+  spec_name: "SCRIPT[custom-element=amp-form] (AMP4EMAIL)"
+  extension_spec: {
+    name: "amp-form"
+    # AMP4EMAIL doesn't allow version: "latest".
+    version: "0.1"
+    deprecated_allow_duplicates: true
+    requires_usage: GRANDFATHERED
+  }
+  attr_lists: "common-extension-attrs"
+}
diff --git a/extensions/amp-list/validator-amp-list.protoascii b/extensions/amp-list/validator-amp-list.protoascii
@@ -31,8 +31,8 @@ tags: {  # amp-list
   spec_name: "SCRIPT[custom-element=amp-list] (AMP4EMAIL)"
   extension_spec: {
     name: "amp-list"
+    # AMP4EMAIL doesn't allow version: "latest".
     version: "0.1"
-    version: "latest"
   }
   attr_lists: "common-extension-attrs"
 }

diff --git a/extensions/amp-mustache/validator-amp-mustache.protoascii b/extensions/amp-mustache/validator-amp-mustache.protoascii
@@ -36,10 +36,9 @@ tags: {  # amp-mustache
 }
 tags: {  # amp-mustache
   html_format: AMP4ADS
-  html_format: AMP4EMAIL
   html_format: ACTIONS
   tag_name: "SCRIPT"
-  spec_name: "SCRIPT[custom-element=amp-mustache] (AMP4ADS/AMP4EMAIL)"
+  spec_name: "SCRIPT[custom-template=amp-mustache] (AMP4ADS/ACTIONS)"
   extension_spec: {
     name: "amp-mustache"
     extension_type: CUSTOM_TEMPLATE
@@ -50,7 +49,21 @@ tags: {  # amp-mustache
   }
   attr_lists: "common-extension-attrs"
 }
-# The script element.
+tags: {  # amp-mustache
+  html_format: AMP4EMAIL
+  tag_name: "SCRIPT"
+  spec_name: "SCRIPT[custom-template=amp-mustache] (AMP4EMAIL)"
+  extension_spec: {
+    name: "amp-mustache"
+    extension_type: CUSTOM_TEMPLATE
+    # AMP4EMAIL doesn't allow version: "latest".
+    version: "0.1"
+    version: "0.2"
+    deprecated_version: "0.1"
+  }
+  attr_lists: "common-extension-attrs"
+}
+# Templates using script[type=text/plain].
 tags: {
   html_format: AMP
   html_format: AMP4ADS

diff --git a/extensions/amp-selector/validator-amp-selector.protoascii b/extensions/amp-selector/validator-amp-selector.protoascii
@@ -32,8 +32,8 @@ tags: {
   spec_name: "SCRIPT[custom-element=amp-selector] (AMP4EMAIL)"
   extension_spec: {
     name: "amp-selector"
+    # AMP4EMAIL doesn't allow version: "latest".
     version: "0.1"
-    version: "latest"
     requires_usage: GRANDFATHERED
   }
   attr_lists: "common-extension-attrs"

diff --git a/extensions/amp-sidebar/validator-amp-sidebar.protoascii b/extensions/amp-sidebar/validator-amp-sidebar.protoascii
@@ -32,8 +32,8 @@ tags: {  # amp-sidebar
   tag_name: "SCRIPT"
   extension_spec: {
     name: "amp-sidebar"
+    # AMP4EMAIL doesn't allow version: "latest".
     version: "0.1"
-    version: "latest"
   }
   attr_lists: "common-extension-attrs"
 }

diff --git a/extensions/amp-timeago/validator-amp-timeago.protoascii b/extensions/amp-timeago/validator-amp-timeago.protoascii
@@ -16,7 +16,6 @@
 
 tags: {  # amp-timeago
   html_format: AMP
-  html_format: AMP4EMAIL
   html_format: ACTIONS
   tag_name: "SCRIPT"
   extension_spec: {
@@ -26,6 +25,17 @@ tags: {  # amp-timeago
   }
   attr_lists: "common-extension-attrs"
 }
+tags: {  # amp-timeago
+  html_format: AMP4EMAIL
+  tag_name: "SCRIPT"
+  spec_name: "SCRIPT[custom-element=amp-timeago] (AMP4EMAIL)"
+  extension_spec: {
+    name: "amp-timeago"
+    # AMP4EMAIL doesn't allow version: "latest".
+    version: "0.1"
+  }
+  attr_lists: "common-extension-attrs"
+}
 tags: {  # <amp-timeago>
   html_format: AMP
   html_format: AMP4EMAIL

diff --git a/validator/engine/validator_test.js b/validator/engine/validator_test.js
@@ -1332,20 +1332,20 @@ describe('ValidatorRulesMakeSense', () => {
       // Changes to the following map must be approved by the AMP4Email
       // Working Group, @wg-amp4email.
       const approvedAmp4EmailExtensions = {
-        'AMP-ACCORDION': ['0.1', 'latest'],
-        'AMP-ANIM': ['0.1', 'latest'],
-        'AMP-BIND-MACRO': ['0.1', 'latest'],
+        'AMP-ACCORDION': ['0.1'],
+        'AMP-ANIM': ['0.1'],
+        'AMP-BIND-MACRO': ['0.1'],
         'AMP-CAROUSEL': ['0.1'],
-        'AMP-FIT-TEXT': ['0.1', 'latest'],
-        'AMP-IMG': ['0.1', 'latest'],
-        'AMP-IMAGE-LIGHTBOX': ['0.1', 'latest'],
-        'AMP-LAYOUT': ['0.1', 'latest'],
-        'AMP-LIGHTBOX': ['0.1', 'latest'],
-        'AMP-LIST': ['0.1', 'latest'],
-        'AMP-SELECTOR': ['0.1', 'latest'],
-        'AMP-SIDEBAR': ['0.1', 'latest'],
-        'AMP-STATE': ['0.1', 'latest'],
-        'AMP-TIMEAGO': ['0.1', 'latest'],
+        'AMP-FIT-TEXT': ['0.1'],
+        'AMP-IMG': ['0.1'],
+        'AMP-IMAGE-LIGHTBOX': ['0.1'],
+        'AMP-LAYOUT': ['0.1'],
+        'AMP-LIGHTBOX': ['0.1'],
+        'AMP-LIST': ['0.1'],
+        'AMP-SELECTOR': ['0.1'],
+        'AMP-SIDEBAR': ['0.1'],
+        'AMP-STATE': ['0.1'],
+        'AMP-TIMEAGO': ['0.1'],
       };
       // Verify extension and it's usage is approved.
       it(tagSpec.tagName + ' has html_format either explicitly or implicitly' +
@@ -1463,15 +1463,13 @@ describe('ValidatorRulesMakeSense', () => {
       it('extension must have a name field value', () => {
         expect(extensionSpec.name).toBeDefined();
       });
-      // TODO(b/139732703): remove the guard when AMP4EMAIL supports
-      // amp-carousel 0.2.
-      if (extensionSpec.name !== 'amp-carousel' ||
-          !tagSpec.htmlFormat.includes(
+      // AMP4EMAIL extensions must support at least one version.
+      if (tagSpec.htmlFormat.includes(
               amp.validator.HtmlFormat.Code.AMP4EMAIL)) {
-        it('extension ' + extensionSpec.name + ' must have at least two ' +
-               'versions, latest and a numeric version, e.g `1.0`',
+        it('extension ' + extensionSpec.name + ' must have at least one ' +
+               'version',
            () => {
-             expect(extensionSpec.version.length).toBeGreaterThan(1);
+             expect(extensionSpec.version.length).toBeGreaterThan(0);
            });
       }
       it('extension ' + extensionSpec.name + ' versions must be `latest` ' +

diff --git a/validator/testdata/amp4email_feature_tests/no_latest_extensions.html b/validator/testdata/amp4email_feature_tests/no_latest_extensions.html
@@ -0,0 +1,41 @@
+<!--
+  Copyright 2015 The AMP HTML Authors. All Rights Reserved.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS-IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the license.
+-->
+<!--
+  Test Description:
+  This checks that "latest" versions of extensions are not valid.
+-->
+<!doctype html>
+<html ⚡4email>
+<head>
+  <meta charset="utf-8">
+  <style amp4email-boilerplate>body{visibility:hidden}</style>
+  <script async src="https://cdn.ampproject.org/v0.js"></script>
+  <script async custom-element="amp-accordion" src="https://cdn.ampproject.org/v0/amp-accordion-latest.js"></script>
+  <script async custom-element="amp-anim" src="https://cdn.ampproject.org/v0/amp-anim-latest.js"></script>
+  <script async custom-element="amp-bind" src="https://cdn.ampproject.org/v0/amp-bind-latest.js"></script>
+  <script async custom-element="amp-carousel" src="https://cdn.ampproject.org/v0/amp-carousel-latest.js"></script>
+  <script async custom-element="amp-fit-text" src="https://cdn.ampproject.org/v0/amp-fit-text-latest.js"></script>
+  <script async custom-element="amp-form" src="https://cdn.ampproject.org/v0/amp-form-latest.js"></script>
+  <script async custom-element="amp-list" src="https://cdn.ampproject.org/v0/amp-list-latest.js"></script>
+  <script async custom-element="amp-selector" src="https://cdn.ampproject.org/v0/amp-selector-latest.js"></script>
+  <script async custom-element="amp-sidebar" src="https://cdn.ampproject.org/v0/amp-sidebar-latest.js"></script>
+  <script async custom-element="amp-timeago" src="https://cdn.ampproject.org/v0/amp-timeago-latest.js"></script>
+  <script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-latest.js"></script>
+</head>
+<body>
+Hello, world.
+</body>
+</html>
diff --git a/validator/testdata/amp4email_feature_tests/no_latest_extensions.out b/validator/testdata/amp4email_feature_tests/no_latest_extensions.out
@@ -0,0 +1,74 @@
+FAIL
+|  <!--
+|    Copyright 2015 The AMP HTML Authors. All Rights Reserved.
+|
+|    Licensed under the Apache License, Version 2.0 (the "License");
+|    you may not use this file except in compliance with the License.
+|    You may obtain a copy of the License at
+|
+|        http://www.apache.org/licenses/LICENSE-2.0
+|
+|    Unless required by applicable law or agreed to in writing, software
+|    distributed under the License is distributed on an "AS-IS" BASIS,
+|    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+|    See the License for the specific language governing permissions and
+|    limitations under the license.
+|  -->
+|  <!--
+|    Test Description:
+|    This checks that "latest" versions of extensions are not valid.
+|  -->
+|  <!doctype html>
+|  <html ⚡4email>
+|  <head>
+|    <meta charset="utf-8">
+|    <style amp4email-boilerplate>body{visibility:hidden}</style>
+|    <script async src="https://cdn.ampproject.org/v0.js"></script>
+|    <script async custom-element="amp-accordion" src="https://cdn.ampproject.org/v0/amp-accordion-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:26:2 The attribute 'src' in tag 'SCRIPT[custom-element=amp-accordion] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-accordion-latest.js'. (see https://amp.dev/documentation/components/amp-accordion) [DISALLOWED_HTML]
+|    <script async custom-element="amp-anim" src="https://cdn.ampproject.org/v0/amp-anim-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:27:2 The attribute 'src' in tag 'amp-anim extension .js script (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-anim-latest.js'. (see https://amp.dev/documentation/components/amp-anim) [DISALLOWED_HTML]
+|    <script async custom-element="amp-bind" src="https://cdn.ampproject.org/v0/amp-bind-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:28:2 The attribute 'src' in tag 'SCRIPT[custom-element=amp-bind] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-bind-latest.js'. (see https://amp.dev/documentation/components/amp-bind) [DISALLOWED_HTML]
+|    <script async custom-element="amp-carousel" src="https://cdn.ampproject.org/v0/amp-carousel-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:29:2 The attribute 'src' in tag 'SCRIPT[custom-element=amp-carousel] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-carousel-latest.js'. (see https://amp.dev/documentation/components/amp-carousel) [DISALLOWED_HTML]
+|    <script async custom-element="amp-fit-text" src="https://cdn.ampproject.org/v0/amp-fit-text-latest.js"></script>
+|    <script async custom-element="amp-form" src="https://cdn.ampproject.org/v0/amp-form-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:31:2 The attribute 'src' in tag 'SCRIPT[custom-element=amp-form] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-form-latest.js'. (see https://amp.dev/documentation/components/amp-form) [DISALLOWED_HTML]
+|    <script async custom-element="amp-list" src="https://cdn.ampproject.org/v0/amp-list-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:32:2 The attribute 'src' in tag 'SCRIPT[custom-element=amp-list] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-list-latest.js'. (see https://amp.dev/documentation/components/amp-list) [DISALLOWED_HTML]
+|    <script async custom-element="amp-selector" src="https://cdn.ampproject.org/v0/amp-selector-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:33:2 The attribute 'src' in tag 'SCRIPT[custom-element=amp-selector] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-selector-latest.js'. (see https://amp.dev/documentation/components/amp-selector) [DISALLOWED_HTML]
+|    <script async custom-element="amp-sidebar" src="https://cdn.ampproject.org/v0/amp-sidebar-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:34:2 The attribute 'src' in tag 'SCRIPT[custom-element=amp-sidebar] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-sidebar-latest.js'. (see https://amp.dev/documentation/components/amp-sidebar) [DISALLOWED_HTML]
+|    <script async custom-element="amp-timeago" src="https://cdn.ampproject.org/v0/amp-timeago-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:35:2 The attribute 'src' in tag 'SCRIPT[custom-element=amp-timeago] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-timeago-latest.js'. (see https://amp.dev/documentation/components/amp-timeago) [DISALLOWED_HTML]
+|    <script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-latest.js"></script>
+>>   ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:36:2 The attribute 'src' in tag 'SCRIPT[custom-template=amp-mustache] (AMP4EMAIL)' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-mustache-latest.js'. (see https://amp.dev/documentation/components/amp-mustache) [DISALLOWED_HTML]
+|  </head>
+|  <body>
+|  Hello, world.
+|  </body>
+|  </html>
+>>       ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:41:6 The extension 'amp-anim' was found on this page, but is unused. Please remove this extension. [AMP_TAG_PROBLEM]
+>>       ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:41:6 The extension 'amp-carousel' was found on this page, but is unused. Please remove this extension. [AMP_TAG_PROBLEM]
+>>       ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:41:6 The extension 'amp-list' was found on this page, but is unused. Please remove this extension. [AMP_TAG_PROBLEM]
+>>       ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:41:6 The extension 'amp-mustache' was found on this page, but is unused. Please remove this extension. [AMP_TAG_PROBLEM]
+>>       ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:41:6 The extension 'amp-sidebar' was found on this page, but is unused. Please remove this extension. [AMP_TAG_PROBLEM]
+>>       ^~~~~~~~~
+amp4email_feature_tests/no_latest_extensions.html:41:6 The extension 'amp-timeago' was found on this page, but is unused. Please remove this extension. [AMP_TAG_PROBLEM]