Skip to content

Commit

Permalink
Merge pull request #1143 from powdercloud/layouts
Browse files Browse the repository at this point in the history
Make AMP Layouts first class in the validator.
  • Loading branch information
powdercloud committed Dec 11, 2015
2 parents 1f78f94 + ca2e59c commit 2a6187d
Show file tree
Hide file tree
Showing 14 changed files with 876 additions and 552 deletions.
124 changes: 124 additions & 0 deletions validator/testdata/feature_tests/amp_layouts.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
<!--
Copyright 2015 The AMP HTML Authors. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS-IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the license.
-->
<!--
Test Description:
This is a (partial) transcription from test/functional/test-layout.js.
-->
<!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>body {opacity: 0}</style><noscript><style>body {opacity: 1}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
</head>
<body>
<!-- valid: layout=nodisplay -->
<amp-img layout="nodisplay" src="itshappening.gif"></amp-img>

<!-- valid: layout=fixed -->
<amp-img layout="fixed" width="100" height="200"
src="itshappening.gif"></amp-img>

<!-- valid: layout=fixed default width/height -->
<amp-img width="100" height="200" src="itshappening.gif"></amp-img>

<!-- invalid: layout=fixed - requires width/height -->
<amp-img layout="fixed" src="itshappening.gif"></amp-img>

<!-- valid: layout=fixed-height -->
<amp-img layout="fixed-height" height="200" src="itshappening.gif"></amp-img>

<!-- valid: layout=fixed-height, with width=auto -->
<amp-img layout="fixed-height" height="200" width="auto"
src="itshappening.gif"></amp-img>

<!-- invalid: layout=fixed-height, prohibit width!=auto -->
<amp-img layout="fixed-height" height="200" width="300"
src="itshappening.gif"></amp-img>

<!-- valid: layout=fixed-height - default with height -->
<amp-img height="200" src="itshappening.gif"></amp-img>

<!-- valid: layout=fixed-height - default with height and width=auto -->
<amp-img height="200" width="auto" src="itshappening.gif"></amp-img>

<!-- invalid: layout=fixed-height - requires height -->
<amp-img layout="fixed-height" src="itshappening.gif"></amp-img>

<!-- valid: layout=responsive -->
<amp-img layout="responsive" width="100" height="200"
src="itshappening.gif"></amp-img>

<!-- valid: layout=responsive with sizes -->
<amp-img layout="responsive" width="100" height="200" sizes="50vw"
src="itshappening.gif"></amp-img>

<!-- valid: layout=fill -->
<amp-img layout="fill" src="itshappening.gif"></amp-img>

<!-- invalid: layout=container
Note that this would be valid if amp-img allowed container.
It doesn't and there is no other element that does.
TODO(johannes): Long run we should get rid of container or use it.
https://github.com/ampproject/amphtml/issues/1109
-->
<amp-img layout="container" src="itshappening.gif"></amp-img>

<!-- invalid: layout=unknown -->
<amp-img layout="unknown" src="itshappening.gif"></amp-img>

<!-- valid: should configure natural dimensions; default layout -->
<amp-pixel src="https://www.example.com/make-my-visit-count"></amp-pixel>

<!-- valid: should configure natural dimensions; default layout;
with width -->
<amp-pixel src="https://www.example.com/make-my-visit-count" width="11">
</amp-pixel>

<!-- valid: should configure natural dimensions; default layout;
with height -->
<amp-pixel src="https://www.example.com/make-my-visit-count" height="11">
</amp-pixel>

<!-- valid: should configure natural dimensions; layout=fixed -->
<amp-pixel src="https://www.example.com/make-my-visit-count" layout="fixed">
</amp-pixel>

<!-- valid: should configure natural dimensions; layout=fixed -->
<amp-pixel src="https://www.example.com/make-my-visit-count"
layout="fixed">
</amp-pixel>

<!-- valid: width and hight set with valid css lengths -->
<amp-pixel src="https://www.example.com/make-my-visit-count"
width="1px" height="1px"></amp-pixel>

<!-- invalid: width=auto won't work because amp-pixel doesn't
support fixed-height layout. -->
<amp-pixel src="https://www.example.com/make-my-visit-count"
width="auto" height="1px"></amp-pixel>

<!-- invalid: width=X. -->
<amp-pixel src="https://www.example.com/make-my-visit-count"
width="X" height="1px"></amp-pixel>

<!-- invalid: height=X. -->
<amp-pixel src="https://www.example.com/make-my-visit-count"
height="X" width="1px"></amp-pixel>
</body>
</html>
9 changes: 9 additions & 0 deletions validator/testdata/feature_tests/amp_layouts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FAIL
feature_tests/amp_layouts.html:41:2 MANDATORY_ATTR_MISSING height (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
feature_tests/amp_layouts.html:51:2 INVALID_ATTR_VALUE layout FIXED_HEIGHT requires width='auto' (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
feature_tests/amp_layouts.html:61:2 MANDATORY_ATTR_MISSING height (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
feature_tests/amp_layouts.html:80:2 INVALID_ATTR_VALUE layout CONTAINER not supported (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
feature_tests/amp_layouts.html:83:2 INVALID_ATTR_VALUE layout (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-img.md)
feature_tests/amp_layouts.html:113:2 IMPLIED_LAYOUT_INVALID layout FIXED_HEIGHT not supported (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-pixel.md)
feature_tests/amp_layouts.html:117:2 INVALID_ATTR_VALUE width (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-pixel.md)
feature_tests/amp_layouts.html:121:2 INVALID_ATTR_VALUE height (see https://github.com/ampproject/amphtml/blob/master/builtins/amp-pixel.md)
10 changes: 8 additions & 2 deletions validator/testdata/feature_tests/amp_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,17 @@
</head>
<body>
<!-- Example of a valid amp-list -->
<amp-list src="https://data.com/articles.json?ref=CANONICAL_URL">
<amp-list width=10 height=10
src="https://data.com/articles.json?ref=CANONICAL_URL">
<div></div>
</amp-list>
<!-- Invalid: src attribute is missing. -->
<amp-list>
<amp-list wdith=10 height=10>
<div></div>
</amp-list>
<!-- also invalid: width/height missing, so it's container layout
which isn't supported. -->
<amp-list src="https://data.com/articles.json?ref=CANONICAL_URL">
<div></div>
</amp-list>
</body>
Expand Down
3 changes: 2 additions & 1 deletion validator/testdata/feature_tests/amp_list.out
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
FAIL
feature_tests/amp_list.html:35:2 MANDATORY_ATTR_MISSING src (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md)
feature_tests/amp_list.html:36:2 DISALLOWED_ATTR wdith (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md)
feature_tests/amp_list.html:41:2 IMPLIED_LAYOUT_INVALID layout CONTAINER not supported (see https://github.com/ampproject/amphtml/blob/master/extensions/amp-list/amp-list.md)
95 changes: 56 additions & 39 deletions validator/testdata/feature_tests/mandatory_dimensions.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,34 @@
<amp-img src="" layout="responsive" width="42" height="42"></amp-img>
<amp-img src="" layout="fixed" width="42" height="42"></amp-img>

<!-- Layout of nodisplay/fill/container with/without width/height -->
<!-- Layout of nodisplay/fill with/without width/height -->
<amp-img src="" layout="nodisplay" width="42" height="42"></amp-img>
<amp-img src="" layout="fill" width="42" height="42"></amp-img>
<amp-img src="" layout="container" width="42" height="42"></amp-img>
<amp-img src="" layout="nodisplay"></amp-img>
<amp-img src="" layout="fill"></amp-img>
<amp-img src="" layout="container"></amp-img>

<!-- Layout of nodisplay/fill/container with partial width/height.
<!-- Layout of nodisplay/fill with partial width/height.
These should not validate, but currently do. -->
<amp-img src="" layout="nodisplay" width="42"></amp-img>
<amp-img src="" layout="fill" height="42"></amp-img>
<amp-img src="" layout="container" width="42"></amp-img>

<!-- Missing layout implies layout="container", width/height optional -->
<!-- Missing layout implies layout="fixed" when width and height are set. -->
<amp-img src="" width="42" height="42"></amp-img>
<amp-img src="" width="42"></amp-img>
<amp-img src=""></amp-img>

<!-- fixed-height layout allows width=auto. -->
<amp-img layout="fixed-height" src="" width="auto" height="42"></amp-img>
<amp-img layout="fixed-height" src="" height="42"></amp-img>

<!-- Missing layout implies layout="fixed-height" when height is set. -->
<amp-img src="" width="auto" height="42"></amp-img>
<amp-img src="" height="42"></amp-img>

<!-- It's OK to have inconsistent units for layout="fixed" -->
<amp-img src="" layout="fixed" width="42px" height="42rem">


<!-- throw in some fully optional attrs -->
<amp-img src="" placeholder=""></amp-img>
<amp-img src="" width=42 height=42 placeholder=""></amp-img>
<amp-img src="" layout="nodisplay" alt="" attribution=""></amp-img>
<amp-img src="" layout="fixed" width="42" height="42" on="" media="">
</amp-img>
Expand All @@ -65,67 +72,77 @@
<amp-audio src="" layout="fixed"></amp-audio>
<amp-pixel src="" layout="fixed" width="42"></amp-pixel>
<amp-pixel src="" layout="fixed"></amp-pixel>
<amp-lightbox layout="fixed" width="42"></amp-lightbox>
<amp-lightbox layout="fixed"></amp-lightbox>
<amp-lightbox layout="nodisplay" width="42"></amp-lightbox>
<amp-lightbox layout="nodisplay"></amp-lightbox>

<!-- src or srcset or both are all valid -->
<amp-img src=""></amp-img>
<amp-img srcset=""></amp-img>
<amp-img src="" srcset=""></amp-img>
<amp-anim src=""></amp-anim>
<amp-anim srcset=""></amp-anim>
<amp-anim src="" srcset=""></amp-anim>
<amp-img width="42" height="42" src=""></amp-img>
<amp-img width="42" height="42" srcset=""></amp-img>
<amp-img width="42" height="42" src="" srcset=""></amp-img>
<amp-anim width="42" height="42" src=""></amp-anim>
<amp-anim width="42" height="42" srcset=""></amp-anim>
<amp-anim width="42" height="42" src="" srcset=""></amp-anim>

<!-- src optional -->
<amp-audio src=""></amp-audio>
<amp-audio></amp-audio>
<amp-video src=""></amp-video>
<amp-video></amp-video>
<amp-ad type="" src=""></amp-ad>
<amp-ad type=""></amp-ad>
<amp-video width="42" height="42" src=""></amp-video>
<amp-video width="42" height="42"></amp-video>
<amp-ad width="42" height="42" type="" src=""></amp-ad>
<amp-ad width="42" height="42" type=""></amp-ad>

<!-- src required -->
<amp-iframe src=""></amp-iframe>
<amp-iframe width="42" height="42" src=""></amp-iframe>
<amp-pixel src=""></amp-pixel>

<!-- disallow a src or srcset -->
<amp-fit-text></amp-fit-text>
<amp-carousel></amp-carousel>
<amp-youtube data-videoid=""></amp-youtube>
<amp-twitter data-tweetid=""></amp-twitter>
<amp-instagram data-shortcode=""></amp-instagram>
<amp-lightbox></amp-lightbox>
<amp-fit-text height="42"></amp-fit-text>
<amp-carousel height="42"></amp-carousel>
<amp-youtube data-videoid="" height="42"></amp-youtube>
<amp-twitter data-tweetid="" height="42"></amp-twitter>
<amp-instagram data-shortcode="" height="42"></amp-instagram>
<amp-lightbox layout="nodisplay"></amp-lightbox>
<!-- /Valid Examples -->

<!-- Invalid Examples -->
<!-- Container layout isn't supported by amp-img. -->
<amp-img src="" layout="container">
<amp-img src="" layout="container" width="42" height="42">
<amp-img src="">

<!-- Layout of responsive/fixed without width/height -->
<amp-img src="" layout="responsive">
<amp-img src="" layout="fixed">
<amp-img src="" layout="responsive" width="42">
<amp-img src="" layout="fixed" height="42">
<amp-img src="" layout="fixed" height="auto">
<amp-img src="" layout="fixed" width="auto" height="42">

<!-- Inconsistent units -->
<amp-img src="" layout="responsive" width="42px" height="42rem">

<!-- src or srcset or both are all valid -->
<amp-img></amp-img>
<amp-anim></amp-anum>
<amp-img width="42" height="42"></amp-img>
<amp-anim width="42" height="42"></amp-anum>

<!-- src optional -->
<amp-audio srcset=""></amp-audio>
<amp-ad type="" srcset=""></amp-ad>
<amp-ad height="42" type="" srcset=""></amp-ad>

<!-- src required -->
<amp-iframe></amp-iframe>
<amp-iframe height="42"></amp-iframe>
<amp-pixel></amp-pixel>

<!-- disallow a src or srcset -->
<amp-fit-text src=""></amp-fit-text>
<amp-carousel src=""></amp-carousel>
<amp-youtube data-videoid="" srcset=""></amp-youtube>
<amp-twitter data-tweetid="" srcset=""></amp-twitter>
<amp-instagram data-shortcode="" srcset=""></amp-instagram>
<amp-lightbox src=""></amp-lightbox>
<amp-fit-text height="42" src=""></amp-fit-text>
<amp-carousel height="42" src=""></amp-carousel>
<amp-youtube height="42" data-videoid="" srcset=""></amp-youtube>
<amp-twitter height="42" data-tweetid="" srcset=""></amp-twitter>
<amp-instagram height="42" data-shortcode="" srcset=""></amp-instagram>
<amp-lightbox layout="nodisplay" src=""></amp-lightbox>

<!-- Not-whitelisted attributes -->
<amp-img src="" foo="bar"></amp-img>
<amp-img height="42" src="" foo="bar"></amp-img>
<!-- /Invalid Examples -->

</body>
Expand Down
Loading

0 comments on commit 2a6187d

Please sign in to comment.