Validation for amp-kaltura-player. Similar to amp-youtube etc., in th…

…is (#3181) case the data-partner attribute is mandatory and everything else is optional. I would be happy to validate more, e.g., with regexes, but don't know what the specs are for these parameters. Also one small fix in amp-kaltura-player.js itself, where the error message differs from the attribute that's being checked. Also a small edit to the amp-kaltura-player.md file to indicate that data-partner is a mandatory attribute.
ampproject · May 10, 2016 · bbc75d5 · bbc75d5
1 parent 2b734ea
commit bbc75d5
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 4 deletions.
diff --git a/extensions/amp-kaltura-player/0.1/amp-kaltura-player.js b/extensions/amp-kaltura-player/0.1/amp-kaltura-player.js
@@ -46,7 +46,7 @@ class AmpKaltura extends AMP.BaseElement {
     const height = this.element.getAttribute('height');
     const partnerid = user.assert(
         this.element.getAttribute('data-partner'),
-        'The data-account attribute is required for <amp-kaltura-player> %s',
+        'The data-partner attribute is required for <amp-kaltura-player> %s',
         this.element);
     const uiconfid = this.element.getAttribute('data-uiconf') ||
     this.element.getAttribute('data-uiconf-id') ||
@@ -81,7 +81,7 @@ class AmpKaltura extends AMP.BaseElement {
     const height = this.element.getAttribute('height');
     const partnerid = user.assert(
       this.element.getAttribute('data-partner'),
-      'The data-account attribute is required for <amp-kaltura-player> %s',
+      'The data-partner attribute is required for <amp-kaltura-player> %s',
       this.element);
     const entryid = this.element.getAttribute('data-entryid') || 'default';
     let src = `https://cdnapisec.kaltura.com/p/${encodeURIComponent(partnerid)}/thumbnail/entry_id/${encodeURIComponent(entryid)}`;

diff --git a/extensions/amp-kaltura-player/0.1/test/test-amp-kaltura-player.js b/extensions/amp-kaltura-player/0.1/test/test-amp-kaltura-player.js
@@ -69,7 +69,7 @@ describe('amp-kaltura-player', () => {
 
   it('requires data-account', () => {
     return getKaltura({}).should.eventually.be.rejectedWith(
-            /The data-account attribute is required for/);
+            /The data-partner attribute is required for/);
   });
 
   it('should pass data-param-* attributes to the iframe src', () => {

diff --git a/extensions/amp-kaltura-player/0.1/test/validator-amp-kaltura-player.html b/extensions/amp-kaltura-player/0.1/test/validator-amp-kaltura-player.html
@@ -0,0 +1,43 @@
+<!--
+  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:
+  Tests for the amp-youtube tag. See the inline comments.
+-->
+<!doctype html>
+<html ⚡>
+<head>
+  <meta charset="utf-8">
+  <link rel="canonical" href="./regular-html-version.html" />
+  <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 custom-element="amp-kaltura-player" src="https://cdn.ampproject.org/v0/amp-kaltura-player-latest.js"></script>
+  <script async src="https://cdn.ampproject.org/v0.js"></script>
+</head>
+<body>
+  <!-- valid example from the documentation -->
+  <amp-kaltura-player
+     data-uiconf="33502051"
+     data-partner="1281471"
+     data-entryid="1_3ts1ms9c"
+     data-param-streamerType = "auto"
+     layout="responsive" width="480" height="270">
+  </amp-kaltura-player>
+
+  <!-- invalid example: data-partner is missing -->
+  <amp-kaltura-player width="480" height="270"></amp-kaltura-player>
+</body>
+</html>
diff --git a/extensions/amp-kaltura-player/0.1/test/validator-amp-kaltura-player.out b/extensions/amp-kaltura-player/0.1/test/validator-amp-kaltura-player.out
@@ -0,0 +1,2 @@
+FAIL
+amp-kaltura-player/0.1/test/validator-amp-kaltura-player.html:41:2 The mandatory attribute 'data-partner' is missing in tag 'amp-kaltura-player'. (see https://www.ampproject.org/docs/reference/extended/amp-kaltura-player.html) [AMP_TAG_PROBLEM]
diff --git a/extensions/amp-kaltura-player/0.1/validator-amp-kaltura-player.protoascii b/extensions/amp-kaltura-player/0.1/validator-amp-kaltura-player.protoascii
@@ -0,0 +1,69 @@
+#
+# Copyright 2016 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.
+#
+
+tags: {  # amp-kaltura-player
+  tag_name: "script"
+  spec_name: "amp-kaltura-player extension .js script"
+  mandatory_parent: "head"
+  attrs: {
+    name: "async"
+    mandatory: true
+    value: ""
+  }
+  attrs: {
+    name: "custom-element"
+    mandatory: true
+    value: "amp-kaltura-player"
+    dispatch_key: true
+  }
+  attrs: {
+    name: "src"
+    mandatory: true
+    value_regex: "https://cdn\\.ampproject\\.org/v0/amp-kaltura-player-(latest|0\\.1)\\.js"
+  }
+  attrs: {
+    name: "type"
+    value: "text/javascript"
+  }
+  cdata: {
+    blacklisted_cdata_regex: {
+      regex: "."
+      error_message: "contents"
+    }
+  }
+  spec_url: "https://www.ampproject.org/docs/reference/extended/amp-kaltura-player.html"
+}
+# amp-kaltura-player either comes with a video-id attribute, or with
+# a data-videoid attribute.
+tags: {  # <amp-kaltura-player>
+  tag_name: "amp-kaltura-player"
+  disallowed_ancestor: "head"
+  disallowed_ancestor: "amp-sidebar"
+  also_requires_tag: "amp-kaltura-player extension .js script"
+
+  attrs: { name: "data-partner" mandatory: true }
+
+  attr_lists: "extended-amp-global"
+  spec_url: "https://www.ampproject.org/docs/reference/extended/amp-kaltura-player.html"
+  amp_layout: {
+    supported_layouts: FILL
+    supported_layouts: FIXED
+    supported_layouts: FIXED_HEIGHT
+    supported_layouts: FLEX_ITEM
+    supported_layouts: NODISPLAY
+    supported_layouts: RESPONSIVE
+  }
+}
diff --git a/extensions/amp-kaltura-player/amp-kaltura-player.md b/extensions/amp-kaltura-player/amp-kaltura-player.md
@@ -85,7 +85,7 @@ Example:
 
 **data-partner**
 
-The Kaltura partner id.
+The Kaltura partner id. This attribute is mandatory.
 
 **data-uiconf**
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		FAIL
		amp-kaltura-player/0.1/test/validator-amp-kaltura-player.html:41:2 The mandatory attribute 'data-partner' is missing in tag 'amp-kaltura-player'. (see https://www.ampproject.org/docs/reference/extended/amp-kaltura-player.html) [AMP_TAG_PROBLEM]