Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validator updates (Polymer-based webui, FLEX_ITEM) #3064

Merged
merged 3 commits into from
May 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions extensions/amp-anim/0.1/validator-amp-anim.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ tags: { # <amp-anim>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ tags: { # <amp-brid-player>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ tags: { # <amp-brightcove>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ tags: { # <amp-carousel>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ tags: { # <amp-dailymotion>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: RESPONSIVE
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ tags: { # <amp-facebook>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ tags: { # <amp-fit-text>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-iframe/0.1/validator-amp-iframe.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ tags: { # <amp-iframe>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ tags: { # <amp-instagram>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ tags: { # <amp-jwplayer>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-list/0.1/validator-amp-list.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ tags: { # <amp-list>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ tags: { # <amp-pinterest>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ tags: { # <amp-reach-player>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: RESPONSIVE
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ tags: { # <amp-springboard-player>
amp_layout: {
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FLEX_ITEM
supported_layouts: RESPONSIVE
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ tags: { # <amp-twitter>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-vimeo/0.1/validator-amp-vimeo.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ tags: { # <amp-vimeo>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: RESPONSIVE
}
}
1 change: 1 addition & 0 deletions extensions/amp-vine/0.1/validator-amp-vine.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ tags: { # <amp-vine>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ tags: { # <amp-youtube>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
16 changes: 16 additions & 0 deletions validator/testdata/feature_tests/amp_layouts.html
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,22 @@
<!-- valid: layout=fill -->
<amp-img layout="fill" src="itshappening.gif"></amp-img>

<!-- invalid: layout=fill with height=auto; height=auto is only allowed
for flex-item -->
<amp-img layout="fill" width="50" height="auto"></amp-img>

<!-- valid: layout=flex-item -->
<amp-img layout="flex-item" src="itshappening.gif"></amp-img>
<!-- valid: layout=flex-item with width="auto" -->
<amp-img layout="flex-item" width="auto" height="50"
src="itshappening.gif"></amp-img>
<!-- valid: layout=flex-item with height=auto -->
<amp-img layout="flex-item" width="50" height="auto"
src="itshappening.gif"></amp-img>
<!-- valid: layout=flex-item with width and height -->
<amp-img layout="flex-item" width="50" height="50"
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.
Expand Down
15 changes: 8 additions & 7 deletions validator/testdata/feature_tests/amp_layouts.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ FAIL
feature_tests/amp_layouts.html:41:2 The mandatory attribute 'height' is missing in tag 'amp-img'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
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://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:61:2 The mandatory attribute 'height' is missing in tag 'amp-img'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:80:2 The specified layout 'CONTAINER' is not supported by tag 'amp-img'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:83:2 The attribute 'layout' in tag 'amp-img' is set to the invalid value 'unknown'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:113:2 The implied layout 'FIXED_HEIGHT' is not supported by tag 'amp-pixel'. (see https://www.ampproject.org/docs/reference/amp-pixel.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:117:2 The attribute 'width' in tag 'amp-pixel' is set to the invalid value 'X'. (see https://www.ampproject.org/docs/reference/amp-pixel.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:121:2 The attribute 'height' in tag 'amp-pixel' is set to the invalid value 'X'. (see https://www.ampproject.org/docs/reference/amp-pixel.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:133:2 The attribute 'heights' in tag 'amp-img' is disallowed by specified layout 'FIXED'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:137:2 The attribute 'heights' in tag 'amp-img' is disallowed by implied layout 'FIXED_HEIGHT'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:76:2 The attribute 'height' in tag 'amp-img' is set to the invalid value 'auto'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:96:2 The specified layout 'CONTAINER' is not supported by tag 'amp-img'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:99:2 The attribute 'layout' in tag 'amp-img' is set to the invalid value 'unknown'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:129:2 The implied layout 'FIXED_HEIGHT' is not supported by tag 'amp-pixel'. (see https://www.ampproject.org/docs/reference/amp-pixel.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:133:2 The attribute 'width' in tag 'amp-pixel' is set to the invalid value 'X'. (see https://www.ampproject.org/docs/reference/amp-pixel.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:137:2 The attribute 'height' in tag 'amp-pixel' is set to the invalid value 'X'. (see https://www.ampproject.org/docs/reference/amp-pixel.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:149:2 The attribute 'heights' in tag 'amp-img' is disallowed by specified layout 'FIXED'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
feature_tests/amp_layouts.html:153:2 The attribute 'heights' in tag 'amp-img' is disallowed by implied layout 'FIXED_HEIGHT'. (see https://www.ampproject.org/docs/reference/amp-img.html) [AMP_LAYOUT_PROBLEM]
8 changes: 6 additions & 2 deletions validator/validator-main.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -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-main.protoascii) are always released together.
min_validator_revision_required: 131
min_validator_revision_required: 133
# 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 (validator-main.js) requires updating this revision id.
spec_file_revision: 211
spec_file_revision: 212

# Validator extensions.
# =====================
Expand Down Expand Up @@ -2148,6 +2148,7 @@ tags: { # <amp-ad>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand All @@ -2174,6 +2175,7 @@ tags: { # <amp-embed>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand All @@ -2193,6 +2195,7 @@ tags: { # <amp-img>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down Expand Up @@ -2243,6 +2246,7 @@ tags: { # <amp-video>
supported_layouts: FILL
supported_layouts: FIXED
supported_layouts: FIXED_HEIGHT
supported_layouts: FLEX_ITEM
supported_layouts: NODISPLAY
supported_layouts: RESPONSIVE
}
Expand Down
12 changes: 11 additions & 1 deletion validator/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -2203,7 +2203,7 @@ class ParsedTagSpec {
return;
}
const inputHeight = new amp.validator.CssLengthAndUnit(
heightAttr, /* allowAuto */ false);
heightAttr, /* allowAuto */ true);
if (!inputHeight.isValid) {
context.addError(
amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
Expand All @@ -2219,6 +2219,16 @@ class ParsedTagSpec {
const layout =
CalculateLayout(inputLayout, width, height, sizesAttr, heightsAttr);

// height="auto" is only allowed if the layout is FLEX_ITEM.
if (height.isAuto &&
layout !== amp.validator.AmpLayout.Layout.FLEX_ITEM) {
context.addError(
amp.validator.ValidationError.Code.INVALID_ATTR_VALUE,
/* params */['height', getTagSpecName(this.spec_), heightAttr],
this.spec_.specUrl, result);
return;
}

// Does the tag support the computed layout?
if (this.spec_.ampLayout.supportedLayouts.indexOf(layout) === -1) {
const code = layoutAttr === undefined ?
Expand Down
1 change: 1 addition & 0 deletions validator/validator.proto
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ message AmpLayout {
RESPONSIVE = 4;
CONTAINER = 5;
FILL = 6;
FLEX_ITEM = 7;
}
// Specifies which layouts are supported by this element.
repeated Layout supported_layouts = 1;
Expand Down
Loading