Allow any layout for amp-social-share, with test. (#3099)

* Allow any layout for amp-social-share, with test. Fixes #3014 * Fix amp-social-share validation and tests. - Any type is allowed, it's just a string. - data-share-endpoint is optional. If given, it must be a URL with one of the whitelisted protocols. Relative URLs are not allowed. * Add j a v a s c r i p t : example. * allow tel: protocol as well * add snapchat
ampproject · May 10, 2016 · dd47548 · dd47548
1 parent 3545bd4
commit dd47548
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 74 deletions.
diff --git a/extensions/amp-social-share/0.1/test/validator-amp-social-share-pass.html b/extensions/amp-social-share/0.1/test/validator-amp-social-share-pass.html
diff --git a/extensions/amp-social-share/0.1/test/validator-amp-social-share-pass.out b/extensions/amp-social-share/0.1/test/validator-amp-social-share-pass.out
diff --git a/extensions/amp-social-share/0.1/test/validator-amp-social-share.html b/extensions/amp-social-share/0.1/test/validator-amp-social-share.html
@@ -25,21 +25,59 @@
   <meta name="viewport" content="width=device-width,minimum-scale=1">
   <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
   <script async src="https://cdn.ampproject.org/v0.js"></script>
+  <script async custom-element="amp-social-share" src="https://cdn.ampproject.org/v0/amp-social-share-0.1.js"></script>
 </head>
 <body>
-  <!-- Error, container layout not allowed -->
-  <amp-social-share layout="container" height="50" width="50">
+  <!-- Valid: simplest possible example -->
+  <amp-social-share type="twitter"></amp-social-share>
+
+  <!-- Valid: a simple example with params -->
+  <amp-social-share type="linkedin" width="60" height="44"
+                    data-param-text="Hello world"
+                    data-param-url="https://example.com/">
   </amp-social-share>
-  <amp-social-share height="50" width="50" type="email">
+
+  <!-- Valid: A share endpoint that's a URL with whitelisted app protocol -->
+  <amp-social-share type="whatsapp"
+                    layout="container"
+                    data-share-endpoint="whatsapp://send"
+                    data-param-text="Check this out: TITLE - CANONICAL_URL">
+    Share on Whatsapp
   </amp-social-share>
-  <amp-social-share type="pinterest" width="60" height="44">
-    <script type="application/json">
-      {
-        "text": "Hello world",
-        "url": "https://example.com/",
-        "attribution": "AMPhtml"
-      }
-    </script>
+
+  <!-- Valid: A share endpoint that's a URL with the whitelisted https protocol
+    -->
+  <amp-social-share type="vote-this-up"
+                    layout="container"
+                    data-share-endpoint="https://example.com/vote-this-up"
+                    data-param-text="Check this out: TITLE - CANONICAL_URL">
+    Share on Whatsapp
+  </amp-social-share>
+
+  <!-- Valid: unknown type, no endpoint given
+       Note: It would be good to consider this invalid, but given the choices
+       around required / optional parameters for this tag, we don't have a
+       convenient way to do that. This will probablly yield a runtime error.
+    -->
+  <amp-social-share type="unknown" width="60" height="44"
+                    data-param-text="Hello world"
+                    data-param-url="https://example.com/">
+  </amp-social-share>
+
+  <!-- Invalid: Endpoint given with a protocol that doesn't fit the whitelist -->
+  <amp-social-share type="vote-this-up"
+                    layout="container"
+                    data-share-endpoint="votethisup://vote"
+                    data-param-text="Check this out: TITLE - CANONICAL_URL">
+    Share on Whatsapp
+  </amp-social-share>
+
+  <!-- Invalid: Endpoint given with a protocol that doesn't fit the whitelist -->
+  <amp-social-share type="uhm-no-hahaha"
+                    layout="container"
+                    data-share-endpoint="j a v a s c r i p t : alert('oh hi')"
+                    data-param-text="Check this out: TITLE - CANONICAL_URL">
+    Share on Whatsapp
   </amp-social-share>
 </body>
 </html>
diff --git a/extensions/amp-social-share/0.1/test/validator-amp-social-share.out b/extensions/amp-social-share/0.1/test/validator-amp-social-share.out
@@ -1,3 +1,3 @@
 FAIL
-amp-social-share/0.1/test/validator-amp-social-share.html:31:2 The specified layout 'CONTAINER' is not supported by tag 'amp-social-share'. (see https://www.ampproject.org/docs/reference/extended/amp-social-share.html) [AMP_LAYOUT_PROBLEM]
-amp-social-share/0.1/test/validator-amp-social-share.html:45:7 The tag 'amp-social-share extension .js script' is missing or incorrect, but required by 'amp-social-share'. (see https://www.ampproject.org/docs/reference/extended/amp-social-share.html) [AMP_TAG_PROBLEM]
+amp-social-share/0.1/test/validator-amp-social-share.html:68:2 Invalid URL protocol 'votethisup:' for attribute 'data-share-endpoint' in tag 'amp-social-share'. (see https://www.ampproject.org/docs/reference/extended/amp-social-share.html) [AMP_TAG_PROBLEM]
+amp-social-share/0.1/test/validator-amp-social-share.html:76:2 Invalid URL protocol 'j a v a s c r i p t :' for attribute 'data-share-endpoint' in tag 'amp-social-share'. (see https://www.ampproject.org/docs/reference/extended/amp-social-share.html) [AMP_TAG_PROBLEM]
diff --git a/extensions/amp-social-share/0.1/validator-amp-social-share.protoascii b/extensions/amp-social-share/0.1/validator-amp-social-share.protoascii
@@ -46,24 +46,6 @@ tags: {  # amp-social-share
   }
   spec_url: "https://www.ampproject.org/docs/reference/extended/amp-social-share.html"
 }
-tags: {  # amp-social-share (json)
-  tag_name: "script"
-  spec_name: "amp-social-share extension .json script"
-  mandatory_parent: "amp-social-share"
-  attrs: {
-    name: "type"
-    mandatory: true
-    value: "application/json"
-    dispatch_key: true
-  }
-  cdata: {
-    blacklisted_cdata_regex: {
-      regex: "<!--"
-      error_message: "html comments"
-    }
-  }
-  spec_url: "https://www.ampproject.org/docs/reference/extended/amp-social-share.html"
-}
 tags: {  # <amp-social-share>
   tag_name: "amp-social-share"
   disallowed_ancestor: "head"
@@ -72,11 +54,33 @@ tags: {  # <amp-social-share>
   attrs: {
     name: "type"
     mandatory: true
-    value_regex: "(twitter|facebook|pinterest|linkedin|gplus|email)"
+  }
+  attrs: {
+    name: "data-share-endpoint"
+    value_url: {
+      allowed_protocol: "ftp"
+      allowed_protocol: "http"
+      allowed_protocol: "https"
+      allowed_protocol: "mailto"
+      # Whitelisting additional commonly observed third party
+      # protocols which should be safe
+      allowed_protocol: "snapchat"
+      allowed_protocol: "sms"
+      allowed_protocol: "tel"
+      allowed_protocol: "viber"
+      allowed_protocol: "whatsapp"
+      allow_relative: false
+    }
   }
   attr_lists: "extended-amp-global"
   spec_url: "https://www.ampproject.org/docs/reference/extended/amp-social-share.html"
   amp_layout: {
+    supported_layouts: CONTAINER
+    supported_layouts: FILL
     supported_layouts: FIXED
+    supported_layouts: FIXED_HEIGHT
+    supported_layouts: FLEX_ITEM
+    supported_layouts: NODISPLAY
+    supported_layouts: RESPONSIVE
   }
 }