From 7219361680eed63b9cda6adf90e642cb057a2fe8 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 10:39:44 +0530 Subject: [PATCH 1/8] Improvements to amp-img fixing - Handle INVALID_ATTR_VALUE error also - If height/width are non-numeric (e.g. width='auto') try to fetch dimensions from the image itself - Stop marking errors are resolved simply because they have been dealt with in the StandardFixPass --- src/Pass/AmpImgFixPass.php | 4 ++-- src/Pass/ImgTagTransformPass.php | 6 +++++- src/Pass/StandardFixPass.php | 4 ---- src/Utility/ActionTakenType.php | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Pass/AmpImgFixPass.php b/src/Pass/AmpImgFixPass.php index 2fa35b2f..b505deaa 100644 --- a/src/Pass/AmpImgFixPass.php +++ b/src/Pass/AmpImgFixPass.php @@ -47,7 +47,7 @@ function pass() { /** @var SValidationError $error */ foreach ($this->validation_result->errors as $error) { - if (in_array($error->code, [ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::IMPLIED_LAYOUT_INVALID, ValidationErrorCode::SPECIFIED_LAYOUT_INVALID]) && + if (in_array($error->code, [ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::INVALID_ATTR_VALUE, ValidationErrorCode::IMPLIED_LAYOUT_INVALID, ValidationErrorCode::SPECIFIED_LAYOUT_INVALID]) && !$error->resolved && !empty($error->dom_tag) && $error->dom_tag->tagName == 'amp-img' @@ -61,7 +61,7 @@ function pass() } $layout = ParsedTagSpec::parseLayout($amp_img_el->attr('layout')); - if ($error->code == ValidationErrorCode::MANDATORY_ATTR_MISSING && + if (in_array($error->code, [ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::INVALID_ATTR_VALUE]) && ($layout !== AmpLayoutLayout::RESPONSIVE || !in_array($error->params[0], ['height', 'width'])) ) { continue; diff --git a/src/Pass/ImgTagTransformPass.php b/src/Pass/ImgTagTransformPass.php index 6b9f1766..2a9015b7 100644 --- a/src/Pass/ImgTagTransformPass.php +++ b/src/Pass/ImgTagTransformPass.php @@ -163,11 +163,15 @@ protected function getImageUrl($src) /** * @param DOMQuery $el + * @return bool */ protected function setAmpImgAttributes(DOMQuery $el) { + $width =$el->attr('width'); + $height = $el->attr('height'); + // If height or image is not set, get it from the image - if (!$el->attr('width') || !$el->attr('height')) { + if (!is_numeric($width) || !is_numeric($height)) { $dimensions = $this->getImageWidthHeight($el->attr('src')); if ($dimensions !== false) { $el->attr('width', $dimensions['width']); diff --git a/src/Pass/StandardFixPass.php b/src/Pass/StandardFixPass.php index 8e67110f..deb82139 100644 --- a/src/Pass/StandardFixPass.php +++ b/src/Pass/StandardFixPass.php @@ -96,11 +96,9 @@ public function pass() if (empty($new_attr_value_trimmed)) { // There is nothing here now so we should just remove the attribute $error->dom_tag->removeAttribute($error->attr_name); $error->addActionTaken(new ActionTakenLine("In $tag_name.$error->attr_name the \"$error->segment\"", ActionTakenType::PROPERTY_REMOVED_ATTRIBUTE_REMOVED, $error->line)); - $error->resolved = true; } else { $error->dom_tag->setAttribute($error->attr_name, $new_attr_value); $error->addActionTaken(new ActionTakenLine("In $tag_name.$error->attr_name the \"$error->segment\"", ActionTakenType::PROPERTY_REMOVED, $error->line)); - $error->resolved = true; } } @@ -111,7 +109,6 @@ public function pass() ) { $error->dom_tag->removeAttribute($error->attr_name); $error->addActionTaken(new ActionTakenLine("$tag_name.$error->attr_name", ActionTakenType::ATTRIBUTE_REMOVED, $error->line)); - $error->resolved = true; } // Tags @@ -119,7 +116,6 @@ public function pass() // Remove the offending tag $error->dom_tag->parentNode->removeChild($error->dom_tag); $error->addActionTaken(new ActionTakenLine($tag_name, ActionTakenType::TAG_REMOVED, $error->line)); - $error->resolved = true; } } diff --git a/src/Utility/ActionTakenType.php b/src/Utility/ActionTakenType.php index 9e138cdb..ee2f15e4 100644 --- a/src/Utility/ActionTakenType.php +++ b/src/Utility/ActionTakenType.php @@ -48,6 +48,6 @@ class ActionTakenType const TAG_REMOVED_FROM_HEAD_AFTER_REVALIDATE_FAILED = 'tag removed from head as it still does not validate. Could not fix tag validation problems.'; const ATTRIBUTE_REMOVED_MUTUALLY_EXCLUSIVE = 'attribute(s) removed as they were mutually exclusive.'; const ISSUE_RESOLVED = 'no further action required as this issue was resolved due to an earlier fix'; - const AMP_IMG_FIX = 'tried to fix problems with amp-img by adjusting one or more of: height, width, and setting layout to responsive'; + const AMP_IMG_FIX = 'tried to fix problems with amp-img by fetching height, width from image directly and/or setting layout to responsive'; const AMP_IMG_FIX_RESPONSIVE = 'tried to fix problems with amp-img by setting layout to responsive'; } From 5c41bc8b189f1e76af4070656cfadeb5228da201 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 10:52:16 +0530 Subject: [PATCH 2/8] regen expected test output as amp-img fixing code has changed --- tests/test-data/full-html/amp_layouts.html.out | 4 ++-- tests/test-data/full-html/mandatory_dimensions.html.out | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test-data/full-html/amp_layouts.html.out b/tests/test-data/full-html/amp_layouts.html.out index 7de86436..bf216fbd 100644 --- a/tests/test-data/full-html/amp_layouts.html.out +++ b/tests/test-data/full-html/amp_layouts.html.out @@ -82,7 +82,7 @@ TODO(johannes): Long run we should get rid of container or use it. https://github.com/ampproject/amphtml/issues/1109 --> - + @@ -325,7 +325,7 @@ FAIL on line 96 - The specified layout 'CONTAINER' is not supported by tag 'amp-img'. [code: SPECIFIED_LAYOUT_INVALID category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] - ACTION TAKEN: amp-img.layout attribute was removed due to validation issues. + ACTION TAKEN: amp-img tried to fix problems with amp-img by setting layout to responsive on line 99 - The attribute 'layout' in tag 'amp-img' is set to the invalid value 'unknown'. diff --git a/tests/test-data/full-html/mandatory_dimensions.html.out b/tests/test-data/full-html/mandatory_dimensions.html.out index ab515a83..db830c76 100644 --- a/tests/test-data/full-html/mandatory_dimensions.html.out +++ b/tests/test-data/full-html/mandatory_dimensions.html.out @@ -113,8 +113,8 @@ - - + + @@ -320,12 +320,12 @@ FAIL on line 110 - The specified layout 'CONTAINER' is not supported by tag 'amp-img'. [code: SPECIFIED_LAYOUT_INVALID category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] - ACTION TAKEN: amp-img.layout attribute was removed due to validation issues. + ACTION TAKEN: amp-img tried to fix problems with amp-img by setting layout to responsive on line 111 - The specified layout 'CONTAINER' is not supported by tag 'amp-img'. [code: SPECIFIED_LAYOUT_INVALID category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] - ACTION TAKEN: amp-img.layout attribute was removed due to validation issues. + ACTION TAKEN: amp-img tried to fix problems with amp-img by fetching height, width from image directly and/or setting layout to responsive on line 112 - The implied layout 'CONTAINER' is not supported by tag 'amp-img'. From aaef1f2cb31cfaed33873c1f63b37edce1f84e50 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 11:17:26 +0530 Subject: [PATCH 3/8] Close amp-img tags in the mandatory_dimensions.html test (see https://github.com/ampproject/amphtml/issues/3713 ) --- tests/README.md | 1 + .../full-html/mandatory_dimensions.html | 22 ++++----- .../full-html/mandatory_dimensions.html.out | 46 +++++++++---------- 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/tests/README.md b/tests/README.md index b8a6d42f..9511f95e 100644 --- a/tests/README.md +++ b/tests/README.md @@ -13,6 +13,7 @@ Note: An explicit commit is noted as these files can change. * `test-data/full-html/link_meta_values.html` https://github.com/ampproject/amphtml/blob/96335e0540532264b3b38d498070f17473692b21/validator/testdata/feature_tests/link_meta_values.html * `test-data/full-html/mandatory-dimensions.html` https://github.com/ampproject/amphtml/blob/e1aa24df8432963423ee6cec1ce4d57529767e6e/validator/testdata/feature_tests/mandatory_dimensions.html There is a trivial modification: `` is changed to ``. See https://github.com/ampproject/amphtml/issues/3609 + Also all `` tags are closed. See https://github.com/ampproject/amphtml/issues/3713 * `test-data/full-html/minimum_valid_amp.html` https://github.com/ampproject/amphtml/blob/96335e0540532264b3b38d498070f17473692b21/validator/testdata/feature_tests/minimum_valid_amp.html * `test-data/full-html/new_and_old_boilerplate_mixed.html` https://github.com/ampproject/amphtml/blob/3bd74c3915f9e29824eebedbf6f14f6c531a3449/validator/testdata/feature_tests/new_and_old_boilerplate_mixed.html * `test-data/full-html/new_and_old_boilerplate_mixed2.html` https://github.com/ampproject/amphtml/blob/d17719548ca786ec8c3778743da7f2020c7d0460/validator/testdata/feature_tests/new_and_old_boilerplate_mixed2.html diff --git a/tests/test-data/full-html/mandatory_dimensions.html b/tests/test-data/full-html/mandatory_dimensions.html index 9f8a5c40..7558b425 100644 --- a/tests/test-data/full-html/mandatory_dimensions.html +++ b/tests/test-data/full-html/mandatory_dimensions.html @@ -58,7 +58,7 @@ - + @@ -107,20 +107,20 @@ - - - + + + - - - - - - + + + + + + - + diff --git a/tests/test-data/full-html/mandatory_dimensions.html.out b/tests/test-data/full-html/mandatory_dimensions.html.out index db830c76..27c8f91a 100644 --- a/tests/test-data/full-html/mandatory_dimensions.html.out +++ b/tests/test-data/full-html/mandatory_dimensions.html.out @@ -64,7 +64,7 @@ - + @@ -113,20 +113,20 @@ - - - + + + - - - - - - + + + + + + - + @@ -152,7 +152,7 @@ - + @@ -219,7 +219,7 @@ Line 57: Line 58: Line 59: Line 60: -Line 61: +Line 61: Line 62: Line 63: Line 64: @@ -268,20 +268,20 @@ Line 106: Line 107: Line 108: Line 109: -Line 110: -Line 111: -Line 112: +Line 110: +Line 111: +Line 112: Line 113: Line 114: -Line 115: -Line 116: -Line 117: -Line 118: -Line 119: -Line 120: +Line 115: +Line 116: +Line 117: +Line 118: +Line 119: +Line 120: Line 121: Line 122: -Line 123: +Line 123: Line 124: Line 125: Line 126: From f2c55de9b635f042acee0a45a690744bb5bdc57b Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 11:44:46 +0530 Subject: [PATCH 4/8] Try to fix INCONSISTENT_UNITS_FOR_WIDTH_AND_HEIGHT also in amp-img --- src/Pass/AmpImgFixPass.php | 8 ++++---- src/Pass/ImgTagTransformPass.php | 32 ++++++++++++++++---------------- src/Utility/ActionTakenType.php | 2 +- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Pass/AmpImgFixPass.php b/src/Pass/AmpImgFixPass.php index b505deaa..83743979 100644 --- a/src/Pass/AmpImgFixPass.php +++ b/src/Pass/AmpImgFixPass.php @@ -47,7 +47,7 @@ function pass() { /** @var SValidationError $error */ foreach ($this->validation_result->errors as $error) { - if (in_array($error->code, [ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::INVALID_ATTR_VALUE, ValidationErrorCode::IMPLIED_LAYOUT_INVALID, ValidationErrorCode::SPECIFIED_LAYOUT_INVALID]) && + if (in_array($error->code, [ValidationErrorCode::INCONSISTENT_UNITS_FOR_WIDTH_AND_HEIGHT, ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::INVALID_ATTR_VALUE, ValidationErrorCode::IMPLIED_LAYOUT_INVALID, ValidationErrorCode::SPECIFIED_LAYOUT_INVALID]) && !$error->resolved && !empty($error->dom_tag) && $error->dom_tag->tagName == 'amp-img' @@ -61,13 +61,13 @@ function pass() } $layout = ParsedTagSpec::parseLayout($amp_img_el->attr('layout')); - if (in_array($error->code, [ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::INVALID_ATTR_VALUE]) && - ($layout !== AmpLayoutLayout::RESPONSIVE || !in_array($error->params[0], ['height', 'width'])) + if (in_array($error->code, [ValidationErrorCode::INCONSISTENT_UNITS_FOR_WIDTH_AND_HEIGHT, ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::INVALID_ATTR_VALUE]) && + ($layout !== AmpLayoutLayout::RESPONSIVE || !in_array($error->params[0], ['height', 'width', 'amp-img'])) ) { continue; } - $success = $this->setAmpImgAttributes($amp_img_el); + $success = $this->setResponsiveImgAttributes($amp_img_el); if ($success) { $error->addActionTaken(new ActionTakenLine('amp-img', ActionTakenType::AMP_IMG_FIX)); $error->resolved = true; diff --git a/src/Pass/ImgTagTransformPass.php b/src/Pass/ImgTagTransformPass.php index 2a9015b7..78d6b158 100644 --- a/src/Pass/ImgTagTransformPass.php +++ b/src/Pass/ImgTagTransformPass.php @@ -18,6 +18,7 @@ namespace Lullabot\AMP\Pass; use Lullabot\AMP\Utility\ParseUrl; +use Lullabot\AMP\Validate\CssLengthAndUnit; use Lullabot\AMP\Validate\GroupedValidationResult; use Lullabot\AMP\Validate\Scope; use Lullabot\AMP\Utility\ActionTakenLine; @@ -74,7 +75,7 @@ function pass() $new_dom_el = $this->cloneAndRenameDomElement($dom_el, 'amp-img'); $new_el = $el->prev(); - $success = $this->setAmpImgAttributes($new_el); + $success = $this->setResponsiveImgAttributes($new_el); // We were not able to get the image dimensions, abort conversion. if(!$success) { $this->addActionTaken(new ActionTakenLine('img', ActionTakenType::IMG_COULD_NOT_BE_CONVERTED, $lineno, $context_string)); @@ -165,23 +166,22 @@ protected function getImageUrl($src) * @param DOMQuery $el * @return bool */ - protected function setAmpImgAttributes(DOMQuery $el) + protected function setResponsiveImgAttributes(DOMQuery $el) { - $width =$el->attr('width'); - $height = $el->attr('height'); - - // If height or image is not set, get it from the image - if (!is_numeric($width) || !is_numeric($height)) { - $dimensions = $this->getImageWidthHeight($el->attr('src')); - if ($dimensions !== false) { - $el->attr('width', $dimensions['width']); - $el->attr('height', $dimensions['height']); - return true; - } else { - return false; - } + $wcss = new CssLengthAndUnit($el->attr('width'), false); + $hcss = new CssLengthAndUnit($el->attr('height'), false); + + if ($wcss->is_set && $wcss->is_valid && $wcss->is_set && $wcss->is_valid && $wcss->unit == $hcss->unit) { + return true; } - return true; + $dimensions = $this->getImageWidthHeight($el->attr('src')); + if ($dimensions !== false) { + $el->attr('width', $dimensions['width']); + $el->attr('height', $dimensions['height']); + return true; + } else { + return false; + } } } diff --git a/src/Utility/ActionTakenType.php b/src/Utility/ActionTakenType.php index ee2f15e4..9562202b 100644 --- a/src/Utility/ActionTakenType.php +++ b/src/Utility/ActionTakenType.php @@ -48,6 +48,6 @@ class ActionTakenType const TAG_REMOVED_FROM_HEAD_AFTER_REVALIDATE_FAILED = 'tag removed from head as it still does not validate. Could not fix tag validation problems.'; const ATTRIBUTE_REMOVED_MUTUALLY_EXCLUSIVE = 'attribute(s) removed as they were mutually exclusive.'; const ISSUE_RESOLVED = 'no further action required as this issue was resolved due to an earlier fix'; - const AMP_IMG_FIX = 'tried to fix problems with amp-img by fetching height, width from image directly and/or setting layout to responsive'; + const AMP_IMG_FIX = 'tried to fix problems with amp-img by trying to fetch height, width from image directly and/or setting layout to responsive'; const AMP_IMG_FIX_RESPONSIVE = 'tried to fix problems with amp-img by setting layout to responsive'; } From 21b48dd9e6e5512d98178719c48acb492012acbd Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 11:53:11 +0530 Subject: [PATCH 5/8] regen mandatory_dimensions.html test output --- tests/test-data/full-html/mandatory_dimensions.html.out | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test-data/full-html/mandatory_dimensions.html.out b/tests/test-data/full-html/mandatory_dimensions.html.out index 27c8f91a..bb51c186 100644 --- a/tests/test-data/full-html/mandatory_dimensions.html.out +++ b/tests/test-data/full-html/mandatory_dimensions.html.out @@ -325,7 +325,7 @@ FAIL on line 111 - The specified layout 'CONTAINER' is not supported by tag 'amp-img'. [code: SPECIFIED_LAYOUT_INVALID category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] - ACTION TAKEN: amp-img tried to fix problems with amp-img by fetching height, width from image directly and/or setting layout to responsive + ACTION TAKEN: amp-img tried to fix problems with amp-img by trying to fetch height, width from image directly and/or setting layout to responsive on line 112 - The implied layout 'CONTAINER' is not supported by tag 'amp-img'. @@ -343,6 +343,7 @@ FAIL on line 117 - The mandatory attribute 'height' is missing in tag 'amp-img'. [code: MANDATORY_ATTR_MISSING category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] + ACTION TAKEN: amp-img tried to fix problems with amp-img by trying to fetch height, width from image directly and/or setting layout to responsive on line 118 - The mandatory attribute 'width' is missing in tag 'amp-img'. From 67c231f868e064ad548e36e55817cbc4d6fa449c Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 12:03:50 +0530 Subject: [PATCH 6/8] Add a static cache to the setResponsiveImgHeightAndWidth method --- src/Pass/AmpImgFixPass.php | 2 +- src/Pass/ImgTagTransformPass.php | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/Pass/AmpImgFixPass.php b/src/Pass/AmpImgFixPass.php index 83743979..04c48def 100644 --- a/src/Pass/AmpImgFixPass.php +++ b/src/Pass/AmpImgFixPass.php @@ -67,7 +67,7 @@ function pass() continue; } - $success = $this->setResponsiveImgAttributes($amp_img_el); + $success = $this->setResponsiveImgHeightAndWidth($amp_img_el); if ($success) { $error->addActionTaken(new ActionTakenLine('amp-img', ActionTakenType::AMP_IMG_FIX)); $error->resolved = true; diff --git a/src/Pass/ImgTagTransformPass.php b/src/Pass/ImgTagTransformPass.php index 78d6b158..da2d8e64 100644 --- a/src/Pass/ImgTagTransformPass.php +++ b/src/Pass/ImgTagTransformPass.php @@ -75,9 +75,9 @@ function pass() $new_dom_el = $this->cloneAndRenameDomElement($dom_el, 'amp-img'); $new_el = $el->prev(); - $success = $this->setResponsiveImgAttributes($new_el); + $success = $this->setResponsiveImgHeightAndWidth($new_el); // We were not able to get the image dimensions, abort conversion. - if(!$success) { + if (!$success) { $this->addActionTaken(new ActionTakenLine('img', ActionTakenType::IMG_COULD_NOT_BE_CONVERTED, $lineno, $context_string)); // Abort the conversion and remove the new img tag $new_el->remove(); @@ -166,8 +166,11 @@ protected function getImageUrl($src) * @param DOMQuery $el * @return bool */ - protected function setResponsiveImgAttributes(DOMQuery $el) + protected function setResponsiveImgHeightAndWidth(DOMQuery $el) { + // Static cache + static $image_dimensions_cache = []; + $wcss = new CssLengthAndUnit($el->attr('width'), false); $hcss = new CssLengthAndUnit($el->attr('height'), false); @@ -175,8 +178,19 @@ protected function setResponsiveImgAttributes(DOMQuery $el) return true; } - $dimensions = $this->getImageWidthHeight($el->attr('src')); + $src = trim($el->attr('src')); + if (empty($src)) { + return false; + } + + if (isset($image_dimensions_cache[$src])) { + $dimensions = $image_dimensions_cache[$src]; + } else { + $dimensions = $this->getImageWidthHeight($src); + } + if ($dimensions !== false) { + $image_dimensions_cache[$src] = $dimensions; $el->attr('width', $dimensions['width']); $el->attr('height', $dimensions['height']); return true; From 48e116944a93bd64933c4a8af4dabd4a31ae86c3 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 12:58:59 +0530 Subject: [PATCH 7/8] [#52] add a new test for checking amp-img correction and img to amp-img transformations --- .../fragment-html/img-test-fragment.html | 22 +++++ .../fragment-html/img-test-fragment.html.out | 93 +++++++++++++++++++ .../img-test-fragment.options.json | 4 + 3 files changed, 119 insertions(+) create mode 100644 tests/test-data/fragment-html/img-test-fragment.html create mode 100644 tests/test-data/fragment-html/img-test-fragment.html.out create mode 100644 tests/test-data/fragment-html/img-test-fragment.options.json diff --git a/tests/test-data/fragment-html/img-test-fragment.html b/tests/test-data/fragment-html/img-test-fragment.html new file mode 100644 index 00000000..93cd96c4 --- /dev/null +++ b/tests/test-data/fragment-html/img-test-fragment.html @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/test-data/fragment-html/img-test-fragment.html.out b/tests/test-data/fragment-html/img-test-fragment.html.out new file mode 100644 index 00000000..3b2c5bab --- /dev/null +++ b/tests/test-data/fragment-html/img-test-fragment.html.out @@ -0,0 +1,93 @@ + + + + + + + + + + + + + + + + + + + + + + + +ORIGINAL HTML +--------------- +Line 1: +Line 2: +Line 3: +Line 4: +Line 5: +Line 6: +Line 7: +Line 8: +Line 9: +Line 10: +Line 11: +Line 12: +Line 13: +Line 14: +Line 15: +Line 16: +Line 17: +Line 18: +Line 19: +Line 20: +Line 21: +Line 22: + + +Transformations made from HTML tags to AMP custom tags +------------------------------------------------------- + + at line 4 + ACTION TAKEN: img tag was converted to the amp-img tag. + + at line 7 + ACTION TAKEN: img tag was converted to the amp-img tag. + + at line 10 + ACTION TAKEN: img tag could NOT be converted to the amp-img tag as the image is not accessible. + + +AMP-HTML Validation Issues and Fixes +------------------------------------- +FAIL + + on line 10 +- The tag 'img' may only appear as a descendant of tag 'noscript'. Did you mean 'amp-img'? + [code: MANDATORY_TAG_ANCESTOR_WITH_HINT category: DISALLOWED_HTML_WITH_AMP_EQUIVALENT see: https://www.ampproject.org/docs/reference/amp-img.html] + + on line 13 +- The implied layout 'CONTAINER' is not supported by tag 'amp-img'. + [code: IMPLIED_LAYOUT_INVALID category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] + ACTION TAKEN: amp-img tried to fix problems with amp-img by trying to fetch height, width from image directly and/or setting layout to responsive + + on line 16 +- The mandatory attribute 'height' is missing in tag 'amp-img'. + [code: MANDATORY_ATTR_MISSING category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] + ACTION TAKEN: amp-img tried to fix problems with amp-img by trying to fetch height, width from image directly and/or setting layout to responsive + + on line 19 +- The attribute 'height' in tag 'amp-img' is set to the invalid value 'auto'. + [code: INVALID_ATTR_VALUE category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] + ACTION TAKEN: amp-img tried to fix problems with amp-img by trying to fetch height, width from image directly and/or setting layout to responsive + + on line 22 +- Inconsistent units for width and height in tag 'amp-img' - width is specified in 'rem' whereas height is specified in 'px'. + [code: INCONSISTENT_UNITS_FOR_WIDTH_AND_HEIGHT category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] + ACTION TAKEN: amp-img tried to fix problems with amp-img by trying to fetch height, width from image directly and/or setting layout to responsive + +COMPONENT NAMES WITH JS PATH +------------------------------ +No custom amp script includes required diff --git a/tests/test-data/fragment-html/img-test-fragment.options.json b/tests/test-data/fragment-html/img-test-fragment.options.json new file mode 100644 index 00000000..2ff4ce13 --- /dev/null +++ b/tests/test-data/fragment-html/img-test-fragment.options.json @@ -0,0 +1,4 @@ +{ + "_readme" : "requires_internet is just for information for the test runner and has no significance for the functioning of library", + "requires_internet": "true" +} \ No newline at end of file From ace24a0300bd420a69edbc8d8aa9828b70c61237 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 13:40:58 +0530 Subject: [PATCH 8/8] Update README.md: Add information on html correction --- README.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e694fe9e..35cc10f0 100644 --- a/README.md +++ b/README.md @@ -10,12 +10,20 @@ The AMP PHP Library is an open source and pure PHP Library that: - Reports compliance of a whole/partial HTML document with the [AMP HTML specification](https://www.ampproject.org/). We implement an AMP HTML validator in pure PHP to report compliance of an arbitrary HTML document / HTML fragment with the AMP HTML standard. This validator is a ported subset of the [canonical validator](https://github.com/ampproject/amphtml/tree/master/validator) that is implemented in JavaScript - Specifically, the PHP validator supports tag specification validation, attribute specification validation, CDATA validation, CSS validation, layout validation, template validation and attribute property-value pair validation. It will report tags and attributes that are missing, illegal, mandatory according to spec but not present, unique according to spec but multiply present, having wrong parents or ancestors or children and so forth. - _Note_: while the AMP PHP library (already) supports many of the features and capabilities of the canonical validator, it is not intended to achieve parity in _every_ respect with the canonical validator. Even _within_ the features we support (e.g. CSS validation) there may be certain validation issues that we don't flag but the canonical validator does. - - Using the feedback given by the in-house PHP validator, the AMP PHP library tries to "correct" some issues found in the HTML to make it more AMP HTML compliant. This would, for example, involve removing: - - Illegal attributes e.g. `style` within `` tag - - Illegal tags e.g. `