Skip to content

Update amphtml validator spec to v1907301630320#3003

Merged
swissspidy merged 4 commits intodevelopfrom
update/amphtml-v1907301630320
Aug 12, 2019
Merged

Update amphtml validator spec to v1907301630320#3003
swissspidy merged 4 commits intodevelopfrom
update/amphtml-v1907301630320

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Aug 8, 2019 •
edited
Loading

Previously: #2816.

Todos specific to this update:

  • Use responsive layout for amp-soundcloud.
  • When AMP_Block_Sanitizer removes classes like wp-embed-aspect-16-9, have it push the width and height onto the child element if it has layout=responsive.

Changelog

SoundCloud Embed Layout

This is revisiting #2722 since amp-soundcloud now supports layout=responsive per ampproject/amphtml#23144.

Before

Only layout=fixed-height was supported, resulting in squished-looking embeds:

Screen Shot 2019-08-09 at 15 41 26

After

Now with layout=responsive, the embed looks identical to the non-AMP version, including the preservation of the aspect ratio defined by the block class name:

Screen Shot 2019-08-09 at 15 41 08

Details

(
    PREV_VERSION=1907022322580;
    THIS_VERSION=1907301630320; 
    git diff $PREV_VERSION...$THIS_VERSION -- $( git ls-files $THIS_VERSION -- . | grep '.protoascii' )
)
Compare 1907022322580...1907301630320 
diff --git a/extensions/amp-ad/validator-amp-ad.protoascii b/extensions/amp-ad/validator-amp-ad.protoascii
index cfc0e8018..67cce32bb 100644
--- a/extensions/amp-ad/validator-amp-ad.protoascii
+++ b/extensions/amp-ad/validator-amp-ad.protoascii
@@ -77,6 +77,42 @@ tags: {  # <amp-ad>
     supported_layouts: RESPONSIVE
   }
 }
+tags: {  # <amp-ad type="custom">
+  html_format: AMP  # Ads are not allowed inside ads
+  tag_name: "AMP-AD"
+  spec_name: "amp-ad with type=custom"
+  requires_extension: "amp-ad"  # See note at top of file
+  also_requires_tag_warning: "amp-ad extension .js script"
+  # If modifying disallowed_ancestors, then please also edit
+  # extensions/amp-auto-ads/*/placement.js
+  disallowed_ancestor: "AMP-APP-BANNER"
+  attrs: {
+    name: "data-url"
+    mandatory: true
+    value_url: {
+      protocol: "https"
+    }
+  }
+  attrs: { name: "template" }
+  attrs: {
+    name: "type"
+    mandatory: true
+    value: "custom"
+    dispatch_key: NAME_VALUE_DISPATCH
+  }
+  attr_lists: "extended-amp-global"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/ads/custom.md"
+  amp_layout: {
+    supported_layouts: FILL
+    supported_layouts: FIXED
+    supported_layouts: FIXED_HEIGHT
+    supported_layouts: FLEX_ITEM
+    supported_layouts: FLUID
+    supported_layouts: INTRINSIC
+    supported_layouts: NODISPLAY
+    supported_layouts: RESPONSIVE
+  }
+}
 tags: {  # <amp-ad data-multi-size>
   html_format: AMP  # Ads are not allowed inside ads
   tag_name: "AMP-AD"
@@ -185,7 +221,6 @@ tags: {  # <amp-embed>
     }
     blacklisted_value_regex: "__amp_source_origin"
   }
-  attrs: { name: "template" }
   attrs: { name: "type" mandatory: true }
   attr_lists: "extended-amp-global"
   spec_url: "https://amp.dev/documentation/components/amp-ad"
diff --git a/extensions/amp-carousel/validator-amp-carousel.protoascii b/extensions/amp-carousel/validator-amp-carousel.protoascii
index b40dab8a3..1b1f31e9c 100644
--- a/extensions/amp-carousel/validator-amp-carousel.protoascii
+++ b/extensions/amp-carousel/validator-amp-carousel.protoascii
@@ -37,13 +37,11 @@ attr_lists: {
     value: ""
   }
   attrs: {
-    disabled_by: "amp4email"
     name: "autoplay"
     value_regex: "(|[0-9]+)"
   }
   attrs: { name: "controls" }
   attrs: {
-    disabled_by: "amp4email"
     name: "delay"
     value_regex: "[0-9]+"
   }
@@ -59,6 +57,23 @@ attr_lists: {
   # <amp-bind>
   attrs: { name: "[slide]" }
 }
+# There are 4 variants here, due to two independent axis of differences:
+#  1) type=slides vs. type=carousel
+#     - type=slides is the implied default if type is unspecified. This is
+#       encoded in the rules by making type=carousel mandatory, and
+#       type=slides optional.
+#     - slides vs. carousel implies different sets up supported layouts.
+#  2) presence of `lightbox` attribute.
+#     - lightbox is only allowed in AMP html format.
+#     - lightbox implies additional reference_point requirements of the
+#       amp-lightbox children.
+# The attribute implied rules (different layouts or different children) is not
+# a feature of the validator rules engine. Thus the only supportable ruleset is
+# to implement 1 tagspec for each combination. This produces the desired rules
+# but has undesirable results for some of the error messages, as seen in some
+# of the test cases. The main issue is that since there are two tagspecs with
+# each combination, we cannot use the usual dispatch_key trick for either
+# attribute.
 tags: {  # <amp-carousel type=slides>
   html_format: AMP
   html_format: AMP4ADS
@@ -96,7 +111,6 @@ tags: {  # <amp-carousel type=carousel>
     name: "type"
     value: "carousel"
     mandatory: true
-    dispatch_key: NAME_VALUE_DISPATCH
   }
   attr_lists: "amp-carousel-common"
   attr_lists: "extended-amp-global"
diff --git a/extensions/amp-date-picker/validator-amp-date-picker.protoascii b/extensions/amp-date-picker/validator-amp-date-picker.protoascii
index 8beca0716..0e96a1eba 100644
--- a/extensions/amp-date-picker/validator-amp-date-picker.protoascii
+++ b/extensions/amp-date-picker/validator-amp-date-picker.protoascii
@@ -133,6 +133,10 @@ tags: {
 
 attr_lists: {
   name: "amp-date-picker-common-attributes"
+  attrs: {
+    name: "allow-blocked-end-date"
+    value: ""
+  }
   attrs: {
     name: "allow-blocked-ranges"
     value: ""
diff --git a/extensions/amp-iframe/validator-amp-iframe.protoascii b/extensions/amp-iframe/validator-amp-iframe.protoascii
index bb9475837..5b4c88fba 100644
--- a/extensions/amp-iframe/validator-amp-iframe.protoascii
+++ b/extensions/amp-iframe/validator-amp-iframe.protoascii
@@ -60,6 +60,10 @@ tags: {  # <amp-iframe>
     value: "no"
     value: "yes"
   }
+  attrs: {
+    name: "tabindex"
+    value_regex: "-?\\d+"
+  }
   attrs: {
     name: "src"
     mandatory_oneof: "['src', 'srcdoc']"
diff --git a/extensions/amp-image-lightbox/validator-amp-image-lightbox.protoascii b/extensions/amp-image-lightbox/validator-amp-image-lightbox.protoascii
index 25ca279e1..238852928 100644
--- a/extensions/amp-image-lightbox/validator-amp-image-lightbox.protoascii
+++ b/extensions/amp-image-lightbox/validator-amp-image-lightbox.protoascii
@@ -36,6 +36,8 @@ tags: {  # amp-image-lightbox
     version: "latest"
   }
   attr_lists: "common-extension-attrs"
+  deprecation: "amp-image-lightbox cannot be properly positioned in emails and will soon be invalid in AMP4EMAIL."
+  deprecation_url: "https://github.com/ampproject/amphtml/issues/23170"
 }
 tags: {  # <amp-image-lightbox>
   html_format: AMP
diff --git a/extensions/amp-lightbox/validator-amp-lightbox.protoascii b/extensions/amp-lightbox/validator-amp-lightbox.protoascii
index d433afe29..afb77bd7d 100644
--- a/extensions/amp-lightbox/validator-amp-lightbox.protoascii
+++ b/extensions/amp-lightbox/validator-amp-lightbox.protoascii
@@ -29,15 +29,27 @@ tags: {  # amp-lightbox
 }
 tags: {  # amp-lightbox
   html_format: AMP4ADS
+  tag_name: "SCRIPT"
+  spec_name: "SCRIPT[custom-element=amp-lightbox] (AMP4ADS)"
+  extension_spec: {
+    name: "amp-lightbox"
+    version: "0.1"
+    version: "latest"
+  }
+  attr_lists: "common-extension-attrs"
+}
+tags: {  # amp-lightbox
   html_format: AMP4EMAIL
   tag_name: "SCRIPT"
-  spec_name: "SCRIPT[custom-element=amp-lightbox] (AMP4EMAIL/AMP4ADS)"
+  spec_name: "SCRIPT[custom-element=amp-lightbox] (AMP4EMAIL)"
   extension_spec: {
     name: "amp-lightbox"
     version: "0.1"
     version: "latest"
   }
   attr_lists: "common-extension-attrs"
+  deprecation: "amp-lightbox cannot be properly positioned in emails and will soon be invalid in AMP4EMAIL."
+  deprecation_url: "https://github.com/ampproject/amphtml/issues/23170"
 }
 tags: {  # <amp-lightbox>
   html_format: AMP
diff --git a/extensions/amp-script/validator-amp-script.protoascii b/extensions/amp-script/validator-amp-script.protoascii
index 93781374c..59205f343 100644
--- a/extensions/amp-script/validator-amp-script.protoascii
+++ b/extensions/amp-script/validator-amp-script.protoascii
@@ -24,17 +24,50 @@ tags: {
   }
   attr_lists: "common-extension-attrs"
 }
-
+# Temporarily disable until interaction of amp-script and signed exchanges has
+# been security reviewed.
+#tags: {  # <amp-script> (local script)
+#  html_format: AMP
+#  tag_name: "SCRIPT"
+#  spec_name: "amp-script extension local script"
+#  requires_extension: "amp-script"
+#  attrs: {
+#    name: "target"
+#    mandatory: true
+#    value: "amp-script"
+#    dispatch_key: NAME_VALUE_DISPATCH
+#  }
+#  attrs: {
+#    name: "type"
+#    mandatory: true
+#    value_casei: "text/plain"
+#  }
+#  attr_lists: "mandatory-id-attr"
+#  attr_lists: "nonce-attr"
+#  cdata: {
+#    max_bytes: 10000
+#    max_bytes_spec_url: "https://amp.dev/documentation/components/amp-script#faq"
+#    blacklisted_cdata_regex: {
+#      regex: "<!--"
+#      error_message: "html comments"
+#    }
+#  }
+#  spec_url: "https://amp.dev/documentation/components/amp-script"
+#}
 tags: {  # <amp-script>
   html_format: AMP
   tag_name: "AMP-SCRIPT"
   disallowed_ancestor: "AMP-SCRIPT"
   requires_extension: "amp-script"
   requires: "amp-experiment-token"
-  unique: true # TODO(choumx): Remove when global size limit is implemented.
+  attrs: { name: "sandbox" }
+  attrs: {
+    name: "script"
+    mandatory_anyof: "['script', 'src']"
+  }
   attrs: {
     name: "src"
-    mandatory: true
+    mandatory_anyof: "['script', 'src']"
     value_url: {
       protocol: "https"
       allow_relative: false
diff --git a/extensions/amp-soundcloud/validator-amp-soundcloud.protoascii b/extensions/amp-soundcloud/validator-amp-soundcloud.protoascii
index 953ca77c4..84081d8c0 100644
--- a/extensions/amp-soundcloud/validator-amp-soundcloud.protoascii
+++ b/extensions/amp-soundcloud/validator-amp-soundcloud.protoascii
@@ -55,6 +55,12 @@ tags: {  # <amp-soundcloud>
   }
   attr_lists: "extended-amp-global"
   amp_layout: {
+    supported_layouts: FILL
+    supported_layouts: FIXED
     supported_layouts: FIXED_HEIGHT
+    supported_layouts: FLEX_ITEM
+    supported_layouts: INTRINSIC
+    supported_layouts: NODISPLAY
+    supported_layouts: RESPONSIVE
   }
 }
diff --git a/extensions/amp-story/validator-amp-story.protoascii b/extensions/amp-story/validator-amp-story.protoascii
index 4ff1f1e5f..afc738f2f 100644
--- a/extensions/amp-story/validator-amp-story.protoascii
+++ b/extensions/amp-story/validator-amp-story.protoascii
@@ -146,6 +146,11 @@ tags: {  # <amp-story-grid-layer>
     value: "thirds"
     value: "vertical"
   }
+  attrs: {
+    name: "position"
+    value: "landscape-half-left"
+    value: "landscape-half-right"
+  }
   descendant_tag_list: "amp-story-grid-layer-allowed-descendants"
   reference_points: {
     tag_spec_name: "AMP-STORY-GRID-LAYER default"
diff --git a/extensions/amp-twitter/validator-amp-twitter.protoascii b/extensions/amp-twitter/validator-amp-twitter.protoascii
index e1f218e46..02ec95854 100644
--- a/extensions/amp-twitter/validator-amp-twitter.protoascii
+++ b/extensions/amp-twitter/validator-amp-twitter.protoascii
@@ -48,23 +48,11 @@ tags: {  # <amp-twitter>
       also_requires_attr: "data-momentid"
     }
   }
-  attrs: {
-    name: "data-link-color"
-    trigger: {
-      also_requires_attr: "data-tweetid"
-    }
-  }
   attrs: {
     name: "data-momentid"
     mandatory_oneof: "['data-momentid', 'data-timeline-source-type', 'data-tweetid']"
     value_regex: "\\d+"
   }
-  attrs: {
-    name: "data-theme"
-    trigger: {
-      also_requires_attr: "data-tweetid"
-    }
-  }
   attrs: {
     name: "data-timeline-id"
     trigger: {
diff --git a/validator/validator-main.protoascii b/validator/validator-main.protoascii
index 79ad56a11..cec9e6b2b 100644
--- a/validator/validator-main.protoascii
+++ b/validator/validator-main.protoascii
@@ -26,7 +26,7 @@ min_validator_revision_required: 375
 # newer versions of the spec file. This is currently a Google internal
 # mechanism, validator.js does not use this facility. However, any
 # change to this file (validator-main.js) requires updating this revision id.
-spec_file_revision: 896
+spec_file_revision: 920
 
 styles_spec_url: "https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/style_pages"
 script_spec_url: "https://amp.dev/documentation/guides-and-tutorials/learn/spec/amphtml#html-tags"
@@ -993,6 +993,24 @@ tags: {
     dispatch_key: NAME_VALUE_DISPATCH
   }
 }
+# AMP metadata, name=amp4ads-vars-*
+# Allows advertisers to pass information from creatives
+# to be included in amp-ad-metadata.
+tags: {
+  html_format: AMP4ADS
+  tag_name: "META"
+  spec_name: "meta name=amp4ads-vars-*"
+  mandatory_parent: "HEAD"
+  attrs: {
+    name: "content"
+    mandatory: true
+  }
+  attrs: {
+    name: "name"
+    mandatory: true
+    value_regex: "amp4ads-vars-.+"
+  }
+}
 # 4.2.6 The style
 # Text contents of the style tag will be validated seperately.
 tags: {  # Special custom 'author' spreadsheet for AMP
@@ -1537,6 +1555,17 @@ tags: {
   tag_name: "ARTICLE"
 }
 # 4.3.3 The section element
+attr_lists: {
+  name: "poool-access-attrs"
+  attrs: {
+    name: "poool-access-preview"
+    requires_extension: "amp-access-poool"
+  }
+  attrs: {
+    name: "poool-access-content"
+    requires_extension: "amp-access-poool"
+  }
+}
 tags: {
   html_format: AMP
   html_format: AMP4ADS
@@ -1544,6 +1573,7 @@ tags: {
   html_format: ACTIONS
   tag_name: "SECTION"
   disallowed_ancestor: "AMP-ACCORDION"
+  attr_lists: "poool-access-attrs"
 }
 # 4.3.4 The nav element
 tags: {
@@ -4123,6 +4153,7 @@ tags {  # <form method=GET ...>
     name: "action-xhr"
     value_url: {
       protocol: "https"
+      allow_relative: false
     }
     blacklisted_value_regex: "__amp_source_origin|"
         "{{|}}"    # Mustache is disallowed in action-xhr.
@@ -6303,6 +6334,8 @@ attr_lists: {
   attrs: { name: "accesskey" }
   attrs: {
     # attribute "class" for transformed is handled within the Validator engine.
+    # If this changes, please be sure to also update:
+    # amp-experiment 1.0 implementation.
     disabled_by: "transformed"
     name: "class"
     blacklisted_value_regex: "(^|\\W)i-amphtml-"
@@ -6810,6 +6843,10 @@ error_specificity {
   code: CSS_EXCESSIVELY_NESTED
   specificity: 119
 }
+error_specificity {
+  code: DOCUMENT_SIZE_LIMIT_EXCEEDED
+  specificity: 120
+}
 # Error formats
 error_formats {
   code: UNKNOWN_CODE
@@ -7270,3 +7307,7 @@ error_formats {
   code: CSS_EXCESSIVELY_NESTED
   format: "CSS excessively nested in tag '%1'."
 }
+error_formats {
+  code: DOCUMENT_SIZE_LIMIT_EXCEEDED
+  format: "Document exceeded %1 bytes limit. Actual size %2 bytes."
+}

@westonruter westonruter added this to the v1.2.1 milestone Aug 8, 2019
@googlebot googlebot added the cla: yes Signed the Google CLA label Aug 8, 2019
This was referenced Aug 9, 2019
Closed
Merged
Closed
@westonruter westonruter mentioned this pull request Aug 9, 2019
Merged
@westonruter westonruter marked this pull request as ready for review August 9, 2019 22:46
@westonruter westonruter requested a review from swissspidy August 9, 2019 22:46
westonruter
westonruter commented Aug 10, 2019
View reviewed changes
swissspidy
swissspidy approved these changes Aug 12, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@swissspidy swissspidy merged commit e08c72e into develop Aug 12, 2019
@swissspidy swissspidy deleted the update/amphtml-v1907301630320 branch August 12, 2019 11:37
westonruter added a commit that referenced this pull request Aug 12, 2019
@westonruter
Merge branch 'develop' of github.com:ampproject/amp-wp into update/am…
94e3f33 
…p-bind-syntax

* 'develop' of github.com:ampproject/amp-wp:
  RTLCSS all the things (#2977)
  Fix AMP Story editor compatibility with code editor (#3007)
  Update dependency core-js to v3.2.1 (#3011)
  Update amphtml validator spec to v1907301630320 (#3003)
  Improve handling of unlisted Vimeo videos (#2986)
  Always hide AMP admin menu item and compatibility tool menu ite… (#3005)
  Update dependency dom-scroll-into-view to v2.0.1 (#3008)
  Hide tooltips that should be hidden (#2988)
@westonruter westonruter mentioned this pull request Aug 23, 2019
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@swissspidy swissspidy swissspidy approved these changes

Assignees

No one assigned

Labels

cla: yes Signed the Google CLA

Projects

None yet

Milestone

v1.2.1

Development

Successfully merging this pull request may close these issues.

3 participants

@westonruter @swissspidy @googlebot