From e44741d66a6afca5190b3790db35f0ec16f3fc4c Mon Sep 17 00:00:00 2001
From: Greg Grothaus <ggrothau@gmail.com>
Date: Thu, 14 Jan 2016 15:59:13 -0800
Subject: [PATCH 1/4] Tighten validation around custom elements.

Require a specific version of each custom element.
Implement dispatch keys to associate error messages with the correct tagspec.
Require script import for each custom element.

Github Issue #875
---
 .../testdata/feature_tests/no_custom_js.out   |   2 +-
 validator/validator.js                        | 119 ++++-
 validator/validator.proto                     |   7 +
 validator/validator.protoascii                | 409 +++++++++++++++++-
 validator/validator_test.js                   |   8 +-
 5 files changed, 508 insertions(+), 37 deletions(-)

diff --git a/validator/testdata/feature_tests/no_custom_js.out b/validator/testdata/feature_tests/no_custom_js.out
index 2fca0e4c9df7..b2fe26471236 100644
--- a/validator/testdata/feature_tests/no_custom_js.out
+++ b/validator/testdata/feature_tests/no_custom_js.out
@@ -1,3 +1,3 @@
 FAIL
 feature_tests/no_custom_js.html:28:3 The attribute 'src' in tag 'amphtml engine v0.js script' is set to the invalid value 'https://example.com/v0-not-allowed.js'. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#scrpt)
-feature_tests/no_custom_js.html:29:3 The attribute 'src' in tag 'amphtml custom element' is set to the invalid value 'https://example.com/v0/not-allowed.js'.
+feature_tests/no_custom_js.html:29:3 The attribute 'custom-element' in tag 'amp-access extension .js script' is set to the invalid value 'amp-foo'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-access/amp-access.md)
diff --git a/validator/validator.js b/validator/validator.js
index c7cd7b32b550..4af540fb1753 100644
--- a/validator/validator.js
+++ b/validator/validator.js
@@ -1265,6 +1265,16 @@ const ParsedTagSpec = function ParsedTagSpec(
    * @private
    */
   this.shouldRecordTagspecValidated_ = shouldRecordTagspecValidated;
+  /**
+   * @type {!Array<number>}
+   * @private
+   */
+  this.alsoRequires_ = [];
+  /**
+   * @type {number}
+   * @private
+   */
+  this.dispatchKeyAttrSpec_ = -1;
 
   const attrs = GetAttrsFor(tagSpec, attrListsByName);
   for (let i = 0; i < attrs.length; ++i) {
@@ -1281,14 +1291,12 @@ const ParsedTagSpec = function ParsedTagSpec(
     for (const altName of altNames) {
       this.attrsByName_[altName] = parsedAttrSpec;
     }
+    if (parsedAttrSpec.getSpec().dispatchKey) {
+      this.dispatchKeyAttrSpec_ = i;
+    }
   }
   this.mandatoryOneofs_ = sortAndUniquify(this.mandatoryOneofs_);
 
-  /**
-   * @type {!Array<number>}
-   * @private
-   */
-  this.alsoRequires_ = [];
   for (const detail of tagSpec.alsoRequires) {
     this.alsoRequires_.push(tagspecIdsByDetail.get(detail));
   }
@@ -1310,6 +1318,27 @@ ParsedTagSpec.prototype.getSpec = function() {
   return this.spec_;
 };
 
+/**
+ * A dispatch key is a mandatory attribute name/value unique to this
+ * TagSpec. If an encountered tag matches this dispatch key, it is
+ * validated first against this TagSpec in order to improve validation
+ * performance and error message selection. Not all TagSpecs have a
+ * dispatch key.
+ * @return {boolean}
+ */
+ParsedTagSpec.prototype.hasDispatchKey = function() {
+  return this.dispatchKeyAttrSpec_ !== -1;
+}
+/**
+ * You must check hasDispatchKey before accessing
+ * @return {string}
+ */
+ParsedTagSpec.prototype.getDispatchKey = function() {
+  goog.asserts.assert(this.hasDispatchKey());
+  const parsedSpec = this.attrsById_[this.dispatchKeyAttrSpec_];
+  return parsedSpec.getSpec().name + '=' + parsedSpec.getSpec().value;
+};
+
 /**
  * A TagSpec may specify other tags to be required as well, when that
  * tag is used. This accessor returns the IDs for the tagspecs that
@@ -1739,6 +1768,25 @@ ParsedTagSpec.prototype.validateDisallowedAncestorTags = function(
   }
 };
 
+/**
+ * This small class (struct) stores the dispatch rules for all TagSpecs
+ * with the same tag name.
+ * @constructor
+ */
+const TagSpecDispatch = function TagSpecDispatch() {
+  /**
+   * TagSpecs for a specific attribute name/value dispatch key.
+   * @type {!goog.structs.Map<!Array<string>, number>}
+   * @public
+   */
+  this.tagSpecsByDispatch = new goog.structs.Map();
+  /**
+   * @type {!Array<number>}
+   * @public
+   */
+  this.allTagSpecs = [];
+};
+
 /**
  * This wrapper class provides access to the validation rules.
  * @constructor
@@ -1752,7 +1800,7 @@ const ParsedValidatorRules = function ParsedValidatorRules() {
   this.tagSpecById_ = [];
   /**
    * ParsedTagSpecs keyed by name
-   * @type {!goog.structs.Map<string, !Array<!ParsedTagSpec>>}
+   * @type {!goog.structs.Map<string, !TagSpecDispatch>}
    * @private
    */
   this.tagSpecByTagName_ = new goog.structs.Map();
@@ -1796,9 +1844,12 @@ const ParsedValidatorRules = function ParsedValidatorRules() {
     this.tagSpecById_.push(parsedTagSpec);
     goog.asserts.assert(tag.name !== null);
     if (!this.tagSpecByTagName_.containsKey(tag.name)) {
-      this.tagSpecByTagName_.set(tag.name, []);
+      this.tagSpecByTagName_.set(tag.name, new TagSpecDispatch());
     }
-    this.tagSpecByTagName_.get(tag.name).push(parsedTagSpec);
+    const tagnameDispatch = this.tagSpecByTagName_.get(tag.name);
+    if (parsedTagSpec.hasDispatchKey())
+      tagnameDispatch.tagSpecsByDispatch.set(parsedTagSpec.getDispatchKey(), i);
+    tagnameDispatch.allTagSpecs.push(parsedTagSpec);
     if (tag.mandatory)
       this.mandatoryTagSpecs_.push(i);
   }
@@ -1830,8 +1881,8 @@ ParsedValidatorRules.prototype.getFormatByCode = function() {
  */
 ParsedValidatorRules.prototype.validateTag = function(
     context, tagName, encounteredAttrs, validationResult) {
-  const allTagSpecs = this.tagSpecByTagName_.get(tagName);
-  if (allTagSpecs === undefined) {
+  const tagSpecDispatch = this.tagSpecByTagName_.get(tagName);
+  if (tagSpecDispatch === undefined) {
     context.addError(
         amp.validator.ValidationError.Code.DISALLOWED_TAG,
         tagName,
@@ -1840,13 +1891,43 @@ ParsedValidatorRules.prototype.validateTag = function(
     return;
   }
   let resultForBestAttempt = new amp.validator.ValidationResult();
-  resultForBestAttempt.status = amp.validator.ValidationResult.Status.UNKNOWN;
-  for (const parsedSpec of allTagSpecs) {
-    this.validateTagAgainstSpec(parsedSpec, context, encounteredAttrs,
-                                resultForBestAttempt);
-    if (resultForBestAttempt.status !==
-        amp.validator.ValidationResult.Status.FAIL) {
-      break;  // Exit early on success
+  resultForBestAttempt.status = amp.validator.ValidationResult.Status.FAIL;
+  // Attempt to validate against dispatched tagspecs first.
+  if (!tagSpecDispatch.tagSpecsByDispatch.isEmpty()) {
+    for (let i = 0; i < encounteredAttrs.length; i += 2) {
+      let attrName = encounteredAttrs[i];
+      let attrValue = encounteredAttrs[i + 1];
+      // Our html parser repeats the key as the value if there is no value. We
+      // replace the value with an empty string instead in this case.
+      if (attrName === attrValue)
+        attrValue = '';
+      attrName = attrName.toLowerCase();
+
+      const match = tagSpecDispatch.tagSpecsByDispatch.get(
+          attrName + '=' + attrValue);
+      if (match !== undefined) {
+        const parsedSpec = this.tagSpecById_[match];
+        this.validateTagAgainstSpec(parsedSpec, context, encounteredAttrs,
+                                    resultForBestAttempt);
+        if (resultForBestAttempt.status !==
+            amp.validator.ValidationResult.Status.FAIL) {
+          validationResult.mergeFrom(resultForBestAttempt);
+          return;  // Exit early on success
+        }
+      }
+    }
+  }
+  // Validate against all tagspecs if we still haven't matched.
+  if (resultForBestAttempt.status ===
+      amp.validator.ValidationResult.Status.FAIL) {
+    for (const parsedSpec of tagSpecDispatch.allTagSpecs) {
+      this.validateTagAgainstSpec(parsedSpec, context, encounteredAttrs,
+                                  resultForBestAttempt);
+      if (resultForBestAttempt.status !==
+          amp.validator.ValidationResult.Status.FAIL) {
+        validationResult.mergeFrom(resultForBestAttempt);
+        return;  // Exit early on success
+      }
     }
   }
   validationResult.mergeFrom(resultForBestAttempt);
@@ -1861,8 +1942,8 @@ ParsedValidatorRules.prototype.validateTag = function(
  */
 ParsedValidatorRules.prototype.validateTagAgainstSpec = function(
     parsedSpec, context, encounteredAttrs, resultForBestAttempt) {
-  goog.asserts.assert(resultForBestAttempt.status !==
-      amp.validator.ValidationResult.Status.PASS);
+  goog.asserts.assert(resultForBestAttempt.status ===
+      amp.validator.ValidationResult.Status.FAIL);
   let resultForAttempt = new amp.validator.ValidationResult();
   resultForAttempt.status = amp.validator.ValidationResult.Status.UNKNOWN;
   parsedSpec.validateAttributes(context, encounteredAttrs, resultForAttempt);
diff --git a/validator/validator.proto b/validator/validator.proto
index 52576dfac91f..6d268ce1739a 100644
--- a/validator/validator.proto
+++ b/validator/validator.proto
@@ -41,6 +41,7 @@ message PropertySpecList {
 // Attributes that are not covered by at least one of these specs are
 // disallowed. Within a given context (e.g., for a given TagSpec),
 // names are unique.
+// NEXT AVAILABLE TAG: 14
 message AttrSpec {
   // Use lower-case attribute names only.
   optional string name = 1;
@@ -79,6 +80,12 @@ message AttrSpec {
   // severity ERROR is generated.
   optional string dev_mode_enabled = 9;
   optional string dev_mode_enabled_url = 10;
+
+  // If set true, the TagSpec containing this AttrSpec will be evaluated first
+  // for any encountered tag which matches the tag name and this attribute spec.
+  // May only be set for an AttrSpec where mandatory=true and the value field
+  // is set.
+  optional bool dispatch_key = 13;
 }
 
 // Some tags share several attributes, so they're identified by unique key
diff --git a/validator/validator.protoascii b/validator/validator.protoascii
index a6e3a656cbbc..3e5197c9acf2 100644
--- a/validator/validator.protoascii
+++ b/validator/validator.protoascii
@@ -20,12 +20,12 @@
 # in production from crashing. This id is not relevant to validator.js
 # because thus far, engine (validator.js) and spec file (validator.protoascii)
 # are always released together.
-min_validator_revision_required: 74
+min_validator_revision_required: 75
 # The spec file revision allows the validator engine to distinguish
 # 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 requires updating this revision id.
-spec_file_revision: 111
+spec_file_revision: 112
 # Rules for AMP HTML
 # (https://github.com/google/amphtml/blob/master/spec/amp-html-format.md).
 # These rules are just enough to validate the example from the spec.
@@ -1477,6 +1477,7 @@ tags: { name: "button" }
 # allowed. The AMP Engine is the first in this specfile so that the
 # 'development' attribute shows up as the disallowed attribute instead of
 # 'src' for a development flagged AMP engine script.
+
 tags: {
   name: "script"
   mandatory: true
@@ -1510,40 +1511,400 @@ tags: {
     value: "application/ld+json"
   }
 }
-tags: {
+# Specific script tags for custom elements and runtime imports.
+tags: {  # amp-access
   name: "script"
   mandatory_parent: "head"
-  detail: "amphtml custom element"
-  attrs: { mandatory: true name: "custom-element" value_regex: "amp-.*" }
+  detail: "amp-access extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-access/amp-access.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-access"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
   attrs: {
     name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-access-0.1.js"
     mandatory: true
-    value_regex: "https://cdn\\.ampproject\\.org/v\\d+/[\\w\\.-]+\\.js"
   }
+}
+tags: {  # amp-analytics
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-analytics extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-analytics/amp-analytics.md"
   attrs: {
-    name: "async"
-    value: ""
+    name: "custom-element"
+    value: "amp-analytics"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-analytics-0.1.js"
     mandatory: true
   }
 }
-tags: {
+tags: {  # amp-anim
   name: "script"
   mandatory_parent: "head"
-  detail: "amp-mustache runtime"
-  spec_url: "https://github.com/ampproject/amphtml/blob/master/spec/amp-html-templates.md"
+  detail: "amp-anim extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-anim/amp-anim.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-anim"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-anim-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-audio
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-audio extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-audio/amp-audio.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-audio"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-audio-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-brightcove
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-brightcove extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-brightcove/amp-brightcove.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-brightcove"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-brightcove-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-carousel
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-carousel extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-carousel/amp-carousel.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-carousel"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-carousel-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-dynamic-css-classes
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-dynamic-css-classes extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-dynamic-css-classes/amp-dynamic-css-classes.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-dynamic-css-classes"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-dynamic-css-classes-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-fit-text
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-fit-text extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-fit-text/amp-fit-text.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-fit-text"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-fit-text-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-font
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-font extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-font/amp-font.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-font"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-font-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-iframe
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-iframe extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-iframe/amp-iframe.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-iframe"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-iframe-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-image-lightbox
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-image-lightbox extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-image-lightbox/amp-image-lightbox.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-image-lightbox"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-image-lightbox-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-instagram
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-instagram extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-instagram/amp-instagram.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-instagram"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-instagram-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-install-serviceworker
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-install-serviceworker extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-install-serviceworker/amp-install-serviceworker.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-install-serviceworker"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-install-serviceworker-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-lightbox
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-lightbox extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-lightbox/amp-lightbox.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-lightbox"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-lightbox-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-mustache
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-mustache extension"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-mustache/amp-mustache.md"
   attrs: {
     name: "custom-template"
     value: "amp-mustache"
     mandatory: true
+    dispatch_key: true
   }
+  attrs: { name: "async" value: "" mandatory: true }
   attrs: {
-    name: "async"
-    value: ""
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-mustache-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-list
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-list extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-list"
     mandatory: true
+    dispatch_key: true
   }
+  attrs: { name: "async" value: "" mandatory: true }
   attrs: {
     name: "src"
-    value: "https://cdn.ampproject.org/v0/amp-mustache-0.1.js"
+    value: "https://cdn.ampproject.org/v0/amp-list-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-pinterest
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-pinterest extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-pinterest/amp-pinterest.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-pinterest"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-pinterest-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-slides
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-slides extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-slides/amp-slides.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-slides"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-slides-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-twitter
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-twitter extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-twitter/amp-twitter.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-twitter"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-twitter-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-user-notification
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-user-notification extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-user-notification/amp-user-notification.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-user-notification"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-user-notification-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-vine
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-vine extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-vine/amp-vine.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-vine"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-vine-0.1.js"
+    mandatory: true
+  }
+}
+tags: {  # amp-youtube
+  name: "script"
+  mandatory_parent: "head"
+  detail: "amp-youtube extension .js script"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-youtube/amp-youtube.md"
+  attrs: {
+    name: "custom-element"
+    value: "amp-youtube"
+    mandatory: true
+    dispatch_key: true
+  }
+  attrs: { name: "async" value: "" mandatory: true }
+  attrs: {
+    name: "src"
+    value: "https://cdn.ampproject.org/v0/amp-youtube-0.1.js"
     mandatory: true
   }
 }
@@ -1585,6 +1946,7 @@ tags: {
     value: "amp-mustache"
     mandatory: true
   }
+  also_requires: "amp-mustache extension"
 }
 
 # Some additional tags, microformats, allowances.
@@ -1659,6 +2021,7 @@ attr_lists: {
 tags: {
   name: "amp-brightcove"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-brightcove/amp-brightcove.md"
+  also_requires: "amp-brightcove extension .js script"
   attr_lists: "extended-amp-global"
 
   attrs: { name: "data-account" mandatory: true }
@@ -1683,6 +2046,7 @@ tags: {
 tags: {
   name: "amp-font"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-font/amp-font.md"
+  also_requires: "amp-font extension .js script"
   attr_lists: "extended-amp-global"
   attrs: {
     name: "font-family"
@@ -1769,6 +2133,7 @@ tags: {
 tags: {
   name: "amp-fit-text"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-fit-text/amp-fit-text.md"
+  also_requires: "amp-fit-text extension .js script"
   attr_lists: "extended-amp-global"
   attrs: { name: "min-font-size" }
   attrs: { name: "max-font-size" }
@@ -1785,6 +2150,7 @@ tags: {
 tags: {
   name: "amp-carousel"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-carousel/amp-carousel.md"
+  also_requires: "amp-carousel extension .js script"
   attr_lists: "extended-amp-global"
   attrs: { name: "autoplay" value: "" }
   attrs: { name: "loop" value: "" }
@@ -1805,6 +2171,7 @@ tags: {
 tags: {
   name: "amp-anim"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-anim/amp-anim.md"
+  also_requires: "amp-anim extension .js script"
   attr_lists: "extended-amp-global"
   attr_lists: "mandatory-src-or-srcset"
   attrs: { name: "alt" }
@@ -1824,6 +2191,7 @@ tags: {
 tags: {
   name: "amp-analytics"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-analytics/amp-analytics.md"
+  also_requires: "amp-analytics extension .js script"
   attrs: { name: "type" value: "googleanalytics" }
   # TODO(gregable): This should require a valid https URL as the attr value.
   attrs: { name: "config" }
@@ -1836,6 +2204,7 @@ tags: {
   name: "amp-youtube"
   attr_lists: "extended-amp-global"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-youtube/amp-youtube.md"
+  also_requires: "amp-youtube extension .js script"
   attrs: {
     name: "src"
     mandatory_oneof: "['src', 'data-videoid']"
@@ -1862,6 +2231,8 @@ tags: {
 # <amp-vine>
 tags: {
   name: "amp-vine"
+  spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-vine/amp-vine.md"
+  also_requires: "amp-vine extension .js script"
   attr_lists: "extended-amp-global"
   # data-* is generally allowed, but it's not generally mandatory.
   attrs: { name: "data-vineid" mandatory: true }
@@ -1879,6 +2250,7 @@ tags: {
 tags: {
   name: "amp-pinterest"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-pinterest/amp-pinterest.md"
+  also_requires: "amp-pinterest extension .js script"
   attr_lists: "extended-amp-global"
   # data-* is generally allowed, but it's not generally mandatory.
   attrs: { name: "data-do" mandatory: true }
@@ -1896,6 +2268,7 @@ tags: {
 tags: {
   name: "amp-twitter"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-twitter/amp-twitter.md"
+  also_requires: "amp-twitter extension .js script"
   attr_lists: "extended-amp-global"
   attrs: { name: "data-tweetid" mandatory_oneof: "['data-tweetid', 'src']" }
   attrs: { name: "src" mandatory_oneof: "['data-tweetid', 'src']" }
@@ -1912,6 +2285,7 @@ tags: {
 tags: {
   name: "amp-instagram"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-instagram/amp-instagram.md"
+  also_requires: "amp-instagram extension .js script"
   attr_lists: "extended-amp-global"
   attrs: {
     name: "data-shortcode"
@@ -1940,6 +2314,7 @@ tags: {
 tags: {
   name: "amp-iframe"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-iframe/amp-iframe.md"
+  also_requires: "amp-iframe extension .js script"
   attr_lists: "extended-amp-global"
   attrs: { name: "sandbox" }
   attrs: { name: "scrolling" value_regex: "auto|yes|no" }
@@ -1979,6 +2354,7 @@ tags: {
 tags: {
   name: "amp-audio"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-audio/amp-audio.md"
+  also_requires: "amp-audio extension .js script"
   attr_lists: "extended-amp-global"
   attrs: { name: "src" }
   attrs: {
@@ -2006,6 +2382,7 @@ tags: {
   name: "amp-lightbox"
   attr_lists: "extended-amp-global"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-lightbox/amp-lightbox.md"
+  also_requires: "amp-lightbox extension .js script"
   attrs: { name: "from" }
   attrs: { name: "controls" }
   amp_layout: {
@@ -2017,6 +2394,7 @@ tags: {
 tags: {
   name: "amp-image-lightbox"  # See http://b/24381009.
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-image-lightbox/amp-image-lightbox.md"
+  also_requires: "amp-image-lightbox extension .js script"
   # Support dimensions / sizes / etc. like an amp-lightbox.
   attr_lists: "extended-amp-global"
   attrs: { name: "controls" }
@@ -2029,6 +2407,7 @@ tags: {
 tags: {
   name: "amp-list"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md"
+  also_requires: "amp-list extension .js script"
   attr_lists: "extended-amp-global"
 
   attrs: { name: "src" mandatory: true }
@@ -2046,6 +2425,7 @@ tags: {
 tags: {
   name: "amp-install-serviceworker"
   spec_url: "https://github.com/ampproject/amphtml/blob/master/extensions/amp-install-serviceworker/amp-install-serviceworker.md"
+  also_requires: "amp-install-serviceworker extension .js script"
   attrs: { name: "src" }
 }
 
@@ -2262,3 +2642,4 @@ error_formats {
   code: TEMPLATE_IN_ATTR_NAME
   format: "Mustache template syntax in attribute name '%1' in tag '%2'."
 }
+
diff --git a/validator/validator_test.js b/validator/validator_test.js
index d57ebe87ecc0..c494531fe13e 100644
--- a/validator/validator_test.js
+++ b/validator/validator_test.js
@@ -135,9 +135,11 @@ describe('ValidatorOutput', () => {
         '\'https://example.com/v0-not-allowed.js\'. ' +
         '(see https://github.com/ampproject/amphtml/blob/master/spec/' +
         'amp-html-format.md#scrpt)\n' +
-        'http://google.com/foo.html:29:3 The attribute \'src\' in tag ' +
-        '\'amphtml custom element\' is set to the invalid value ' +
-        '\'https://example.com/v0/not-allowed.js\'.';
+        'http://google.com/foo.html:29:3 The attribute \'custom-element\' ' +
+        'in tag \'amp-access extension .js script\' is set to the invalid ' +
+        'value \'amp-foo\'. ' +
+        '(see https://github.com/ampproject/amphtml/blob/master/extensions/' +
+        'amp-access/amp-access.md)';
     test.run();
   });
 });

From a7757e984b2b62f8a5e2be03eb5feeb6ac5ea3ba Mon Sep 17 00:00:00 2001
From: Johannes Henkel <johannes@google.com>
Date: Thu, 14 Jan 2016 15:59:31 -0800
Subject: [PATCH 2/4] Remove the detail field population It's no longer used.

---
 validator/validator.js | 70 ++++++------------------------------------
 1 file changed, 10 insertions(+), 60 deletions(-)

diff --git a/validator/validator.js b/validator/validator.js
index 4af540fb1753..8f82d32ee792 100644
--- a/validator/validator.js
+++ b/validator/validator.js
@@ -520,8 +520,6 @@ CdataMatcher.prototype.match = function(cdata, context, validationResult) {
     const bytes = byteLength(cdata);
     if (bytes > cdataSpec.maxBytes) {
       context.addError(amp.validator.ValidationError.Code.STYLESHEET_TOO_LONG,
-                       'seen: ' + bytes + ' bytes, limit: ' +
-                       cdataSpec.maxBytes + ' bytes',
                        /* params */ [bytes, cdataSpec.maxBytes],
                        cdataSpec.maxBytesSpecUrl, validationResult);
       // We return early if the byte length is violated as parsing
@@ -540,7 +538,6 @@ CdataMatcher.prototype.match = function(cdata, context, validationResult) {
       context.addError(
           amp.validator.ValidationError.Code.
               MANDATORY_CDATA_MISSING_OR_INCORRECT,
-          getDetailOrName(this.tagSpec_),
           /* params */ [getDetailOrName(this.tagSpec_)],
           this.tagSpec_.specUrl, validationResult);
     }
@@ -554,7 +551,6 @@ CdataMatcher.prototype.match = function(cdata, context, validationResult) {
       context.addError(
           amp.validator.ValidationError.Code.
               MANDATORY_CDATA_MISSING_OR_INCORRECT,
-          getDetailOrName(this.tagSpec_),
           /* params */ [getDetailOrName(this.tagSpec_)],
           this.tagSpec_.specUrl, validationResult);
       return;
@@ -581,7 +577,6 @@ CdataMatcher.prototype.match = function(cdata, context, validationResult) {
       const lineCol = new LineCol(errorToken.line, errorToken.col);
       context.addErrorWithLineCol(
           lineCol, amp.validator.ValidationError.Code.CSS_SYNTAX,
-          errorToken.msg,
           /* params */ [getDetailOrName(this.tagSpec_), errorToken.msg],
           /* url */ '', validationResult);
     }
@@ -597,7 +592,6 @@ CdataMatcher.prototype.match = function(cdata, context, validationResult) {
           context.addErrorWithLineCol(
               lineCol,
               amp.validator.ValidationError.Code.CSS_SYNTAX_INVALID_AT_RULE,
-              cssRule.name,
               /* params */ [getDetailOrName(this.tagSpec_), cssRule.name],
               /* url */ '', validationResult);
         }
@@ -621,7 +615,6 @@ CdataMatcher.prototype.match = function(cdata, context, validationResult) {
       context.addError(
           amp.validator.ValidationError.Code.
               MANDATORY_CDATA_VIOLATES_BLACKLIST,
-          blacklist.errorMessage,
           /* params */ [getDetailOrName(this.tagSpec_), blacklist.errorMessage],
           this.tagSpec_.specUrl, validationResult);
     }
@@ -749,14 +742,13 @@ Context.prototype.getProgress = function(validationResult) {
  * Returns true if the result was changed; false otherwise.
  * @param {LineCol|amp.htmlparser.DocLocator} lineCol a line / column pair.
  * @param {!amp.validator.ValidationError.Code} validationErrorCode Error code
- * @param {string} validationErrorDetail Error detail
  * @param {!Array<!string>} params
  * @param {string} specUrl a link (URL) to the amphtml spec
  * @param {!amp.validator.ValidationResult} validationResult
  * @return {boolean}
  */
 Context.prototype.addErrorWithLineCol = function(
-    lineCol, validationErrorCode, validationErrorDetail, params, specUrl,
+    lineCol, validationErrorCode, params, specUrl,
     validationResult) {
   const progress = this.getProgress(validationResult);
   if (progress.complete) {
@@ -775,7 +767,6 @@ Context.prototype.addErrorWithLineCol = function(
     const error = new amp.validator.ValidationError();
     error.severity = severity;
     error.code = validationErrorCode;
-    error.detail = validationErrorDetail;
     error.params = params;
     error.line = lineCol.getLine();
     error.col = lineCol.getCol();
@@ -789,24 +780,18 @@ Context.prototype.addErrorWithLineCol = function(
 /**
  * Returns true if the result was changed; false otherwise.
  * @param {!amp.validator.ValidationError.Code} validationErrorCode Error code
- * @param {?string} validationErrorDetail Error detail
  * @param {!Array<!string>} params
  * @param {?string} specUrl a link (URL) to the amphtml spec
  * @param {!amp.validator.ValidationResult} validationResult
  * @return {boolean}
  */
-Context.prototype.addError = function(
-    validationErrorCode, validationErrorDetail, params, specUrl,
-    validationResult) {
-  if (validationErrorDetail === null) {
-    validationErrorDetail = '';
-  }
+Context.prototype.addError = function(validationErrorCode, params, specUrl,
+                                      validationResult) {
   if (specUrl === null) {
     specUrl = '';
   }
   return this.addErrorWithLineCol(
-      this.docLocator_, validationErrorCode, validationErrorDetail, params,
-      specUrl, validationResult);
+      this.docLocator_, validationErrorCode, params, specUrl, validationResult);
 };
 
 /**
@@ -946,7 +931,6 @@ ParsedAttrSpec.prototype.validateAttrValueProperties = function(
     if (!this.valuePropertyByName_.containsKey(name)) {
       context.addError(
           amp.validator.ValidationError.Code.DISALLOWED_PROPERTY_IN_ATTR_VALUE,
-          attrName + '="...' + name + '=..."',
           /* params */ [name, attrName, getDetailOrName(tagSpec)],
           specUrl, result);
       continue;
@@ -957,7 +941,6 @@ ParsedAttrSpec.prototype.validateAttrValueProperties = function(
         context.addError(
             amp.validator.ValidationError.Code.
                 INVALID_PROPERTY_VALUE_IN_ATTR_VALUE,
-            attrName + '="...' + name + '=' + value + '..."',
             /* params */ [name, attrName, getDetailOrName(tagSpec), value],
             specUrl, result);
       }
@@ -966,7 +949,6 @@ ParsedAttrSpec.prototype.validateAttrValueProperties = function(
         context.addError(
             amp.validator.ValidationError.Code.
                 INVALID_PROPERTY_VALUE_IN_ATTR_VALUE,
-            attrName + '="...' + name + '=' + value + '..."',
             /* params */ [name, attrName, getDetailOrName(tagSpec), value],
             specUrl, result);
       }
@@ -977,7 +959,6 @@ ParsedAttrSpec.prototype.validateAttrValueProperties = function(
     context.addError(
         amp.validator.ValidationError.Code.
             MANDATORY_PROPERTY_MISSING_FROM_ATTR_VALUE,
-        attrName + '="...' + name + '=' + '..."',
         /* params */ [name, attrName, getDetailOrName(tagSpec)],
         specUrl, result);
   }
@@ -1422,7 +1403,6 @@ ParsedTagSpec.prototype.validateLayout = function(context, attrsByKey, result) {
       inputLayout === amp.validator.AmpLayout.Layout.UNKNOWN) {
     context.addError(
         amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
-        'layout',
         /* params */ ["layout", getDetailOrName(this.spec_), layoutAttr],
         this.spec_.specUrl, result);
     return;
@@ -1432,7 +1412,6 @@ ParsedTagSpec.prototype.validateLayout = function(context, attrsByKey, result) {
   if (!inputWidth.isValid) {
     context.addError(
         amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
-        'width',
         /* params */ ["width", getDetailOrName(this.spec_), widthAttr],
         this.spec_.specUrl, result);
     return;
@@ -1442,7 +1421,6 @@ ParsedTagSpec.prototype.validateLayout = function(context, attrsByKey, result) {
   if (!inputHeight.isValid) {
     context.addError(
         amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
-        'height',
         /* params */ ["height", getDetailOrName(this.spec_), heightAttr],
         this.spec_.specUrl, result);
     return;
@@ -1459,8 +1437,7 @@ ParsedTagSpec.prototype.validateLayout = function(context, attrsByKey, result) {
     const code = layoutAttr === undefined ?
         amp.validator.ValidationError.Code.IMPLIED_LAYOUT_INVALID :
         amp.validator.ValidationError.Code.SPECIFIED_LAYOUT_INVALID;
-    context.addError(code, 'layout ' + layout + ' not supported',
-                     /* params */ [layout, getDetailOrName(this.spec_)],
+    context.addError(code, /* params */ [layout, getDetailOrName(this.spec_)],
                      this.spec_.specUrl, result);
     return;
   }
@@ -1470,7 +1447,6 @@ ParsedTagSpec.prototype.validateLayout = function(context, attrsByKey, result) {
        layout === amp.validator.AmpLayout.Layout.RESPONSIVE) &&
       !height.isSet) {
     context.addError(amp.validator.ValidationError.Code.MANDATORY_ATTR_MISSING,
-                     'height',
                      /* params */ ['height', getDetailOrName(this.spec_)],
                      this.spec_.specUrl, result);
     return;
@@ -1479,7 +1455,6 @@ ParsedTagSpec.prototype.validateLayout = function(context, attrsByKey, result) {
       width.isSet && !width.isAuto) {
     context.addError(
         amp.validator.ValidationError.Code.ATTR_VALUE_REQUIRED_BY_LAYOUT,
-        "layout FIXED_HEIGHT requires width='auto'",
         /* params */ [widthAttr, 'width', getDetailOrName(this.spec_),
                       'FIXED_HEIGHT', 'auto'],
         this.spec_.specUrl, result);
@@ -1490,14 +1465,12 @@ ParsedTagSpec.prototype.validateLayout = function(context, attrsByKey, result) {
     if (!width.isSet) {
       context.addError(
           amp.validator.ValidationError.Code.MANDATORY_ATTR_MISSING,
-          'width',
           /* params */ ['width', getDetailOrName(this.spec_)],
           this.spec_.specUrl, result);
       return;
     } else if (width.isAuto) {
       context.addError(
           amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
-          'width=auto',
           /* params */ ['width', getDetailOrName(this.spec_), 'auto'],
           this.spec_.specUrl, result);
       return;
@@ -1508,7 +1481,6 @@ ParsedTagSpec.prototype.validateLayout = function(context, attrsByKey, result) {
       context.addError(
           amp.validator.ValidationError.Code.
               INCONSISTENT_UNITS_FOR_WIDTH_AND_HEIGHT,
-          width.unit + ' != ' + height.unit,
           /* params */ [getDetailOrName(this.spec_), width.unit, height.unit],
           this.spec_.specUrl, result);
       return;
@@ -1575,13 +1547,11 @@ ParsedTagSpec.prototype.validateAttributes = function(
       if (encounteredAttrName.indexOf('{{') != -1) {
         context.addError(
             amp.validator.ValidationError.Code.TEMPLATE_IN_ATTR_NAME,
-            encounteredAttrName,
             /* params */ [encounteredAttrName, getDetailOrName(this.spec_)],
             this.templateSpecUrl_, resultForAttempt);
       } else {
         context.addError(
             amp.validator.ValidationError.Code.DISALLOWED_ATTR,
-            encounteredAttrName,
             /* params */ [encounteredAttrName, getDetailOrName(this.spec_)],
             this.spec_.specUrl, resultForAttempt);
       }
@@ -1592,7 +1562,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
       if (this.valueHasUnescapedTemplateSyntax(encounteredAttrValue)) {
         context.addError(
             amp.validator.ValidationError.Code.UNESCAPED_TEMPLATE_IN_ATTR_VALUE,
-            encounteredAttrName + '=' + encounteredAttrValue,
             /* params */ [encounteredAttrName, getDetailOrName(this.spec_),
                          encounteredAttrValue],
             this.templateSpecUrl_, resultForAttempt);
@@ -1601,7 +1570,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
       if (this.valueHasPartialsTemplateSyntax(encounteredAttrValue)) {
         context.addError(
             amp.validator.ValidationError.Code.TEMPLATE_PARTIAL_IN_ATTR_VALUE,
-            encounteredAttrName + '=' + encounteredAttrValue,
             /* params */ [encounteredAttrName, getDetailOrName(this.spec_),
                          encounteredAttrValue],
             this.templateSpecUrl_, resultForAttempt);
@@ -1612,7 +1580,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
     if (parsedSpec.getSpec().deprecation !== null) {
       context.addError(
           amp.validator.ValidationError.Code.DEPRECATED_ATTR,
-          encounteredAttrName + ' - ' + parsedSpec.getSpec().deprecation,
           /* params */ [encounteredAttrName, getDetailOrName(this.spec_),
                         parsedSpec.getSpec().deprecation],
           parsedSpec.getSpec().deprecationUrl, resultForAttempt);
@@ -1621,8 +1588,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
     if (parsedSpec.getSpec().devModeEnabled !== null) {
       context.addError(
           amp.validator.ValidationError.Code.DEV_MODE_ENABLED,
-          encounteredAttrName + ' - ' +
-              parsedSpec.getSpec().devModeEnabled,
           /* params */ [encounteredAttrName, getDetailOrName(this.spec_)],
           parsedSpec.getSpec().devModeEnabledUrl,
           resultForAttempt);
@@ -1641,7 +1606,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
         if (encounteredAttrValue != parsedSpec.getSpec().value) {
           context.addError(
               amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
-              encounteredAttrName + '=' + encounteredAttrValue,
               /* params */ [encounteredAttrName, getDetailOrName(this.spec_),
                             encounteredAttrValue],
               this.spec_.specUrl, resultForAttempt);
@@ -1654,7 +1618,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
         if (!valueRegex.test(encounteredAttrValue)) {
           context.addError(
               amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
-              encounteredAttrName + '=' + encounteredAttrValue,
               /* params */ [encounteredAttrName, getDetailOrName(this.spec_),
                             encounteredAttrValue],
               this.spec_.specUrl, resultForAttempt);
@@ -1678,7 +1641,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
       if (blacklistedValueRegex.test(encounteredAttrValue)) {
         context.addError(
             amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
-            encounteredAttrName + '=' + encounteredAttrValue,
             /* params */ [encounteredAttrName, getDetailOrName(this.spec_),
                           encounteredAttrValue],
             this.spec_.specUrl, resultForAttempt);
@@ -1695,7 +1657,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
         mandatoryOneofsSeen.contains(parsedSpec.getSpec().mandatoryOneof)) {
       context.addError(
           amp.validator.ValidationError.Code.MUTUALLY_EXCLUSIVE_ATTRS,
-          parsedSpec.getSpec().mandatoryOneof,
           /* params */ [getDetailOrName(this.spec_),
                         parsedSpec.getSpec().mandatoryOneof],
           this.spec_.specUrl, resultForAttempt);
@@ -1709,7 +1670,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
     if (!mandatoryOneofsSeen.contains(mandatoryOneof)) {
       context.addError(
           amp.validator.ValidationError.Code.MANDATORY_ONEOF_ATTR_MISSING,
-          'pick one of ' + mandatoryOneof,
           /* params */ [getDetailOrName(this.spec_), mandatoryOneof],
           this.spec_.specUrl, resultForAttempt);
     }
@@ -1720,7 +1680,6 @@ ParsedTagSpec.prototype.validateAttributes = function(
   for (const diff of diffs) {
     context.addError(
         amp.validator.ValidationError.Code.MANDATORY_ATTR_MISSING,
-        this.attrsById_[diff].getSpec().name,
         /* params */ [this.attrsById_[diff].getSpec().name,
                       getDetailOrName(this.spec_)],
         this.spec_.specUrl, resultForAttempt);
@@ -1742,7 +1701,6 @@ ParsedTagSpec.prototype.validateParentTag = function(
     // both succinct and should be well understood by web developers.
     context.addError(
         amp.validator.ValidationError.Code.WRONG_PARENT_TAG,
-        context.getTagNames().getParent() + ' > ' + this.spec_.name,
         /* params */ [this.spec_.name, context.getTagNames().getParent(),
                       this.spec_.mandatoryParent],
         this.spec_.specUrl, validationResult);
@@ -1760,7 +1718,6 @@ ParsedTagSpec.prototype.validateDisallowedAncestorTags = function(
     if (context.getTagNames().hasAncestor(disallowedAncestor)) {
       context.addError(
           amp.validator.ValidationError.Code.DISALLOWED_TAG_ANCESTOR,
-          disallowedAncestor + ' > ... > ' + this.spec_.name,
           /* params */ [this.spec_.name, disallowedAncestor],
           this.spec_.specUrl, validationResult);
       return;
@@ -1883,11 +1840,8 @@ ParsedValidatorRules.prototype.validateTag = function(
     context, tagName, encounteredAttrs, validationResult) {
   const tagSpecDispatch = this.tagSpecByTagName_.get(tagName);
   if (tagSpecDispatch === undefined) {
-    context.addError(
-        amp.validator.ValidationError.Code.DISALLOWED_TAG,
-        tagName,
-        /* params */ [tagName],
-        /* specUrl */ '', validationResult);
+    context.addError(amp.validator.ValidationError.Code.DISALLOWED_TAG,
+                     /* params */ [tagName], /* specUrl */ '', validationResult);
     return;
   }
   let resultForBestAttempt = new amp.validator.ValidationResult();
@@ -1970,8 +1924,8 @@ ParsedValidatorRules.prototype.validateTagAgainstSpec = function(
       const spec = parsedSpec.getSpec();
       context.addError(
           amp.validator.ValidationError.Code.DUPLICATE_UNIQUE_TAG,
-          getDetailOrName(spec), /* params */ [getDetailOrName(spec)],
-          spec.specUrl, resultForBestAttempt);
+          /* params */ [getDetailOrName(spec)], spec.specUrl,
+          resultForBestAttempt);
       return;
     }
   }
@@ -1998,7 +1952,6 @@ ParsedValidatorRules.prototype.maybeEmitMandatoryTagValidationErrors =
       const spec = this.tagSpecById_[tagspecId].getSpec();
       if (!context.addError(
           amp.validator.ValidationError.Code.MANDATORY_TAG_MISSING,
-          getDetailOrName(spec),
           /* params */ [getDetailOrName(spec)],
           spec.specUrl, validationResult)) {
         return;
@@ -2024,8 +1977,6 @@ ParsedValidatorRules.prototype.maybeEmitAlsoRequiresValidationErrors =
         const alsoRequiresSpec = this.tagSpecById_[alsoRequiresTagspecId];
         if (!context.addError(
             amp.validator.ValidationError.Code.TAG_REQUIRED_BY_MISSING,
-            getDetailOrName(alsoRequiresSpec.getSpec()) + ' required by ' +
-            getDetailOrName(spec.getSpec()),
             /* params */ [getDetailOrName(
                 alsoRequiresSpec.getSpec()), getDetailOrName(spec.getSpec())],
             spec.getSpec().specUrl,
@@ -2062,8 +2013,7 @@ ParsedValidatorRules.prototype.maybeEmitMandatoryAlternativesSatisfiedErrors =
   for (const tagMissing of sortAndUniquify(missing)) {
     if (!context.addError(
         amp.validator.ValidationError.Code.MANDATORY_TAG_MISSING,
-        tagMissing, /* params */ [tagMissing],
-        /* specUrl */ specUrlsByMissing[tagMissing],
+        /* params */ [tagMissing], /* specUrl */ specUrlsByMissing[tagMissing],
         validationResult)) {
       return;
     }

From 193b71c9bf44eb02eecdf10c7b6c26c0ad9532bc Mon Sep 17 00:00:00 2001
From: Johannes Henkel <johannes@google.com>
Date: Thu, 14 Jan 2016 16:00:54 -0800
Subject: [PATCH 3/4] Insert linebreak to accomodate 80 col punchcards.

---
 validator/build.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/validator/build.py b/validator/build.py
index f5ca0c9522e2..1c4cdbf1b29a 100755
--- a/validator/build.py
+++ b/validator/build.py
@@ -223,7 +223,8 @@ def GenerateValidateBin(out_dir):
         var args = process.argv.slice(2);
         var full_path = args[0];
 
-        if (full_path.indexOf('http://') === 0 || full_path.indexOf('https://') === 0) {
+        if (full_path.indexOf('http://') === 0 ||
+            full_path.indexOf('https://') === 0) {
           var callback = function(response) {
             var chunks = [];
 
@@ -270,7 +271,8 @@ def RunSmokeTest(out_dir):
 
   # Run dist/validate on an empty file and observe that it fails.
   open('%s/empty.html' % out_dir, 'w').close()
-  p = subprocess.Popen(['node', '%s/validate' % out_dir, '%s/empty.html' % out_dir],
+  p = subprocess.Popen(['node', '%s/validate' % out_dir, '%s/empty.html' %
+                        out_dir],
                        stdout=subprocess.PIPE, stderr=subprocess.PIPE)
   (stdout, stderr) = p.communicate()
   if p.returncode != 1:

From dd3551797373d23845858356f650c513b3050961 Mon Sep 17 00:00:00 2001
From: Greg Grothaus <ggrothau@gmail.com>
Date: Thu, 14 Jan 2016 16:01:11 -0800
Subject: [PATCH 4/4] Fix broken English sentence.

---
 .../testdata/feature_tests/amp_brightcove.out      |  2 +-
 .../feature_tests/amp_identification_missing.out   |  2 +-
 validator/testdata/feature_tests/amp_layouts.out   |  4 ++--
 validator/testdata/feature_tests/amp_list.out      |  2 +-
 validator/testdata/feature_tests/dog_doc_type.out  |  2 +-
 .../feature_tests/mandatory_dimensions.out         | 14 +++++++-------
 .../testdata/feature_tests/several_errors.out      |  2 +-
 validator/validator.protoascii                     |  4 ++--
 8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/validator/testdata/feature_tests/amp_brightcove.out b/validator/testdata/feature_tests/amp_brightcove.out
index 9737ade763cd..2ee0b13429c7 100644
--- a/validator/testdata/feature_tests/amp_brightcove.out
+++ b/validator/testdata/feature_tests/amp_brightcove.out
@@ -1,2 +1,2 @@
 FAIL
-feature_tests/amp_brightcove.html:48:2 The mandatory attribute 'data-account' missing in tag 'amp-brightcove'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-brightcove/amp-brightcove.md)
+feature_tests/amp_brightcove.html:48:2 The mandatory attribute 'data-account' is missing in tag 'amp-brightcove'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-brightcove/amp-brightcove.md)
diff --git a/validator/testdata/feature_tests/amp_identification_missing.out b/validator/testdata/feature_tests/amp_identification_missing.out
index 4f859e9a6cf9..af69069176ee 100644
--- a/validator/testdata/feature_tests/amp_identification_missing.out
+++ b/validator/testdata/feature_tests/amp_identification_missing.out
@@ -1,3 +1,3 @@
 FAIL
-feature_tests/amp_identification_missing.html:22:0 The mandatory attribute '⚡' missing in tag 'html ⚡ for top-level html'. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#ampd)
+feature_tests/amp_identification_missing.html:22:0 The mandatory attribute '⚡' is missing in tag 'html ⚡ for top-level html'. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#ampd)
 feature_tests/amp_identification_missing.html:33:7 The mandatory tag 'html ⚡ for top-level html' is missing or incorrect. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#ampd)
diff --git a/validator/testdata/feature_tests/amp_layouts.out b/validator/testdata/feature_tests/amp_layouts.out
index 465f389999ee..3f85db369cbd 100644
--- a/validator/testdata/feature_tests/amp_layouts.out
+++ b/validator/testdata/feature_tests/amp_layouts.out
@@ -1,7 +1,7 @@
 FAIL
-feature_tests/amp_layouts.html:41:2 The mandatory attribute 'height' missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
+feature_tests/amp_layouts.html:41:2 The mandatory attribute 'height' is missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/amp_layouts.html:51:2 Invalid value '300' for attribute 'width' in tag 'amp-img' - for layout 'FIXED_HEIGHT', set the attribute 'width' to value 'auto'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
-feature_tests/amp_layouts.html:61:2 The mandatory attribute 'height' missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
+feature_tests/amp_layouts.html:61:2 The mandatory attribute 'height' is missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/amp_layouts.html:80:2 The specified layout 'CONTAINER' is not supported by tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/amp_layouts.html:83:2 The attribute 'layout' in tag 'amp-img' is set to the invalid value 'unknown'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/amp_layouts.html:113:2 The implied layout 'FIXED_HEIGHT' is not supported by tag 'amp-pixel'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-pixel.md)
diff --git a/validator/testdata/feature_tests/amp_list.out b/validator/testdata/feature_tests/amp_list.out
index 8c9d2eac0683..32088f1597c9 100644
--- a/validator/testdata/feature_tests/amp_list.out
+++ b/validator/testdata/feature_tests/amp_list.out
@@ -1,4 +1,4 @@
 FAIL
 feature_tests/amp_list.html:36:2 The attribute 'wdith' may not appear in tag 'amp-list'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md)
-feature_tests/amp_list.html:41:2 The mandatory attribute 'src' missing in tag 'amp-list'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md)
+feature_tests/amp_list.html:41:2 The mandatory attribute 'src' is missing in tag 'amp-list'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md)
 feature_tests/amp_list.html:46:2 The implied layout 'CONTAINER' is not supported by tag 'amp-list'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md)
diff --git a/validator/testdata/feature_tests/dog_doc_type.out b/validator/testdata/feature_tests/dog_doc_type.out
index fbf4923a4f9c..f9509ac591f1 100644
--- a/validator/testdata/feature_tests/dog_doc_type.out
+++ b/validator/testdata/feature_tests/dog_doc_type.out
@@ -1,4 +1,4 @@
 FAIL
 feature_tests/dog_doc_type.html:23:0 The attribute '🐶' may not appear in tag 'html doctype'.
-feature_tests/dog_doc_type.html:24:0 The mandatory attribute '⚡' missing in tag 'html ⚡ for top-level html'. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#ampd)
+feature_tests/dog_doc_type.html:24:0 The mandatory attribute '⚡' is missing in tag 'html ⚡ for top-level html'. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#ampd)
 feature_tests/dog_doc_type.html:35:7 The mandatory tag 'html ⚡ for top-level html' is missing or incorrect. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#ampd)
diff --git a/validator/testdata/feature_tests/mandatory_dimensions.out b/validator/testdata/feature_tests/mandatory_dimensions.out
index 90ec114d13a7..bd8dbd43663a 100644
--- a/validator/testdata/feature_tests/mandatory_dimensions.out
+++ b/validator/testdata/feature_tests/mandatory_dimensions.out
@@ -2,19 +2,19 @@ FAIL
 feature_tests/mandatory_dimensions.html:109:4 The specified layout 'CONTAINER' is not supported by tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/mandatory_dimensions.html:110:4 The specified layout 'CONTAINER' is not supported by tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/mandatory_dimensions.html:111:4 The implied layout 'CONTAINER' is not supported by tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
-feature_tests/mandatory_dimensions.html:114:4 The mandatory attribute 'height' missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
-feature_tests/mandatory_dimensions.html:115:4 The mandatory attribute 'height' missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
-feature_tests/mandatory_dimensions.html:116:4 The mandatory attribute 'height' missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
-feature_tests/mandatory_dimensions.html:117:4 The mandatory attribute 'width' missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
+feature_tests/mandatory_dimensions.html:114:4 The mandatory attribute 'height' is missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
+feature_tests/mandatory_dimensions.html:115:4 The mandatory attribute 'height' is missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
+feature_tests/mandatory_dimensions.html:116:4 The mandatory attribute 'height' is missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
+feature_tests/mandatory_dimensions.html:117:4 The mandatory attribute 'width' is missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/mandatory_dimensions.html:118:4 The attribute 'height' in tag 'amp-img' is set to the invalid value 'auto'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/mandatory_dimensions.html:119:4 The attribute 'width' in tag 'amp-img' is set to the invalid value 'auto'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/mandatory_dimensions.html:122:4 Inconsistent units for width and height in tag 'amp-img' - width is specified in 'px' whereas height is specified in 'rem'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
-feature_tests/mandatory_dimensions.html:125:4 The mandatory attribute 'src' missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
-feature_tests/mandatory_dimensions.html:126:4 The mandatory attribute 'src' missing in tag 'amp-anim'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-anim/amp-anim.md)
+feature_tests/mandatory_dimensions.html:125:4 The mandatory attribute 'src' is missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
+feature_tests/mandatory_dimensions.html:126:4 The mandatory attribute 'src' is missing in tag 'amp-anim'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-anim/amp-anim.md)
 feature_tests/mandatory_dimensions.html:129:4 The attribute 'srcset' may not appear in tag 'amp-audio'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-audio/amp-audio.md)
 feature_tests/mandatory_dimensions.html:130:4 The attribute 'srcset' may not appear in tag 'amp-ad'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-ad.md)
 feature_tests/mandatory_dimensions.html:133:4 The tag 'amp-iframe' is missing a mandatory attribute - pick one of ['src', 'srcdoc']. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-iframe/amp-iframe.md)
-feature_tests/mandatory_dimensions.html:134:4 The mandatory attribute 'src' missing in tag 'amp-pixel'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-pixel.md)
+feature_tests/mandatory_dimensions.html:134:4 The mandatory attribute 'src' is missing in tag 'amp-pixel'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-pixel.md)
 feature_tests/mandatory_dimensions.html:137:4 The attribute 'src' may not appear in tag 'amp-fit-text'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-fit-text/amp-fit-text.md)
 feature_tests/mandatory_dimensions.html:138:4 The attribute 'src' may not appear in tag 'amp-carousel'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-carousel/amp-carousel.md)
 feature_tests/mandatory_dimensions.html:139:4 The attribute 'srcset' may not appear in tag 'amp-youtube'. (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-youtube/amp-youtube.md)
diff --git a/validator/testdata/feature_tests/several_errors.out b/validator/testdata/feature_tests/several_errors.out
index 36bef9528d86..9cec9ffe4974 100644
--- a/validator/testdata/feature_tests/several_errors.out
+++ b/validator/testdata/feature_tests/several_errors.out
@@ -1,7 +1,7 @@
 FAIL
 feature_tests/several_errors.html:23:2 The attribute 'charset' in tag 'charset utf-8 declaration' is set to the invalid value 'pick-a-key'. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#chrs)
 feature_tests/several_errors.html:26:2 The attribute 'type' in tag 'script' is set to the invalid value 'javascript'.
-feature_tests/several_errors.html:32:2 The mandatory attribute 'height' missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
+feature_tests/several_errors.html:32:2 The mandatory attribute 'height' is missing in tag 'amp-img'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
 feature_tests/several_errors.html:34:2 The attribute 'width' in tag 'amp-ad' is set to the invalid value '100%'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-ad.md)
 feature_tests/several_errors.html:37:2 The attribute 'made_up_attribute' may not appear in tag 'amp-ad'. (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-ad.md)
 feature_tests/several_errors.html:39:7 The mandatory tag 'charset utf-8 declaration' is missing or incorrect. (see https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#chrs)
diff --git a/validator/validator.protoascii b/validator/validator.protoascii
index 3e5197c9acf2..c5437c90b194 100644
--- a/validator/validator.protoascii
+++ b/validator/validator.protoascii
@@ -25,7 +25,7 @@ min_validator_revision_required: 75
 # 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 requires updating this revision id.
-spec_file_revision: 112
+spec_file_revision: 113
 # Rules for AMP HTML
 # (https://github.com/google/amphtml/blob/master/spec/amp-html-format.md).
 # These rules are just enough to validate the example from the spec.
@@ -2556,7 +2556,7 @@ error_formats {
 }
 error_formats {
   code: MANDATORY_ATTR_MISSING
-  format: "The mandatory attribute '%1' missing in tag '%2'."
+  format: "The mandatory attribute '%1' is missing in tag '%2'."
 }
 error_formats {
   code: INCONSISTENT_UNITS_FOR_WIDTH_AND_HEIGHT