From be89db2f61f70804c04ea67ef8f59914140c51ef Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Mon, 4 Jul 2016 16:59:01 +0530 Subject: [PATCH 1/8] Remove outdated comment --- src/Pass/StandardFixPass.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Pass/StandardFixPass.php b/src/Pass/StandardFixPass.php index 76716b29..9695ccda 100644 --- a/src/Pass/StandardFixPass.php +++ b/src/Pass/StandardFixPass.php @@ -43,8 +43,8 @@ class StandardFixPass extends BasePass ValidationErrorCode::MISSING_URL, ValidationErrorCode::UNESCAPED_TEMPLATE_IN_ATTR_VALUE, ValidationErrorCode::TEMPLATE_PARTIAL_IN_ATTR_VALUE - // @todo ValidationErrorCode::MUTUALLY_EXCLUSIVE_ATTRS? ]; + protected $remove_tags_for_codes = [ ValidationErrorCode::WRONG_PARENT_TAG, ValidationErrorCode::DISALLOWED_TAG, From de635bcb351ce7c1b961ba43e80af1af3fbf2be6 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Mon, 4 Jul 2016 17:14:52 +0530 Subject: [PATCH 2/8] Remove various layout attributes if they are bad --- src/Validate/ParsedTagSpec.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Validate/ParsedTagSpec.php b/src/Validate/ParsedTagSpec.php index c5393029..0f9c0fc4 100644 --- a/src/Validate/ParsedTagSpec.php +++ b/src/Validate/ParsedTagSpec.php @@ -643,21 +643,21 @@ public function validateLayout(Context $context, array $attrs_by_key, SValidatio $input_layout = self::parseLayout($layout_attr); if (!empty($layout_attr) && $input_layout === AmpLayoutLayout::UNKNOWN) { $context->addError(ValidationErrorCode::INVALID_ATTR_VALUE, - ['layout', self::getTagSpecName($this->spec), $layout_attr], $this->spec->spec_url, $result); + ['layout', self::getTagSpecName($this->spec), $layout_attr], $this->spec->spec_url, $result, 'layout'); return; } $input_width = new CssLengthAndUnit($width_attr, true); if (!$input_width->is_valid) { $context->addError(ValidationErrorCode::INVALID_ATTR_VALUE, - ['width', self::getTagSpecName($this->spec), $width_attr], $this->spec->spec_url, $result); + ['width', self::getTagSpecName($this->spec), $width_attr], $this->spec->spec_url, $result, 'width'); return; } $input_height = new CssLengthAndUnit($height_attr, true); if (!$input_height->is_valid) { $context->addError(ValidationErrorCode::INVALID_ATTR_VALUE, - ['height', self::getTagSpecName($this->spec), $height_attr], $this->spec->spec_url, $result); + ['height', self::getTagSpecName($this->spec), $height_attr], $this->spec->spec_url, $result, 'height'); return; } @@ -667,7 +667,7 @@ public function validateLayout(Context $context, array $attrs_by_key, SValidatio if ($height->is_auto && $layout !== AmpLayoutLayout::FLEX_ITEM) { $context->addError(ValidationErrorCode::INVALID_ATTR_VALUE, - ['height', self::getTagSpecName($this->spec), $height_attr], $this->spec->spec_url, $result); + ['height', self::getTagSpecName($this->spec), $height_attr], $this->spec->spec_url, $result, 'height'); return; } @@ -675,7 +675,7 @@ public function validateLayout(Context $context, array $attrs_by_key, SValidatio $code = empty($layout_attr) ? ValidationErrorCode::IMPLIED_LAYOUT_INVALID : ValidationErrorCode::SPECIFIED_LAYOUT_INVALID; $context->addError($code, - [$layout, self::getTagSpecName($this->spec)], $this->spec->spec_url, $result); + [$layout, self::getTagSpecName($this->spec)], $this->spec->spec_url, $result, $layout_attr); return; } @@ -702,7 +702,7 @@ public function validateLayout(Context $context, array $attrs_by_key, SValidatio return; } else if ($width->is_auto) { $context->addError(ValidationErrorCode::INVALID_ATTR_VALUE, - ['width', self::getTagSpecName($this->spec), 'auto'], $this->spec->spec_url, $result); + ['width', self::getTagSpecName($this->spec), 'auto'], $this->spec->spec_url, $result, 'width'); return; } } @@ -717,7 +717,7 @@ public function validateLayout(Context $context, array $attrs_by_key, SValidatio $code = empty($layout_attr) ? ValidationErrorCode::ATTR_DISALLOWED_BY_IMPLIED_LAYOUT : ValidationErrorCode::ATTR_DISALLOWED_BY_SPECIFIED_LAYOUT; $context->addError($code, - ['heights', self::getTagSpecName($this->spec), $layout], $this->spec->spec_url, $result); + ['heights', self::getTagSpecName($this->spec), $layout], $this->spec->spec_url, $result, 'heights'); return; } } From 2112678473b76b5f38a19b6c95d108da23da7968 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Mon, 4 Jul 2016 17:24:22 +0530 Subject: [PATCH 3/8] More layout error codes that should lead to removal of attributes. Small bug fix with SPECIFIED_LAYOUT_INVALID --- src/Pass/StandardFixPass.php | 7 +++++-- src/Validate/ParsedTagSpec.php | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Pass/StandardFixPass.php b/src/Pass/StandardFixPass.php index 9695ccda..8e67110f 100644 --- a/src/Pass/StandardFixPass.php +++ b/src/Pass/StandardFixPass.php @@ -42,9 +42,12 @@ class StandardFixPass extends BasePass ValidationErrorCode::DISALLOWED_ATTR, ValidationErrorCode::MISSING_URL, ValidationErrorCode::UNESCAPED_TEMPLATE_IN_ATTR_VALUE, - ValidationErrorCode::TEMPLATE_PARTIAL_IN_ATTR_VALUE + ValidationErrorCode::TEMPLATE_PARTIAL_IN_ATTR_VALUE, + ValidationErrorCode::ATTR_DISALLOWED_BY_SPECIFIED_LAYOUT, + ValidationErrorCode::ATTR_DISALLOWED_BY_IMPLIED_LAYOUT, + ValidationErrorCode::SPECIFIED_LAYOUT_INVALID ]; - + protected $remove_tags_for_codes = [ ValidationErrorCode::WRONG_PARENT_TAG, ValidationErrorCode::DISALLOWED_TAG, diff --git a/src/Validate/ParsedTagSpec.php b/src/Validate/ParsedTagSpec.php index 0f9c0fc4..32981647 100644 --- a/src/Validate/ParsedTagSpec.php +++ b/src/Validate/ParsedTagSpec.php @@ -675,7 +675,7 @@ public function validateLayout(Context $context, array $attrs_by_key, SValidatio $code = empty($layout_attr) ? ValidationErrorCode::IMPLIED_LAYOUT_INVALID : ValidationErrorCode::SPECIFIED_LAYOUT_INVALID; $context->addError($code, - [$layout, self::getTagSpecName($this->spec)], $this->spec->spec_url, $result, $layout_attr); + [$layout, self::getTagSpecName($this->spec)], $this->spec->spec_url, $result, empty($layout_attr) ? '' : 'layout'); return; } From 0a894698216564edaacfca61993b2d19ea0ce387 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Mon, 4 Jul 2016 17:27:39 +0530 Subject: [PATCH 4/8] regen tests as various layout attributes are now being removed --- .../test-data/full-html/amp_layouts.html.out | 21 ++++++++++++------- .../full-html/mandatory_dimensions.html.out | 12 +++++++---- .../full-html/several_errors.html.out | 3 ++- .../validator-amp-soundcloud.html.out | 3 ++- .../validator-amp-springboard-player.html.out | 3 ++- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/tests/test-data/full-html/amp_layouts.html.out b/tests/test-data/full-html/amp_layouts.html.out index aa9b7b71..7de86436 100644 --- a/tests/test-data/full-html/amp_layouts.html.out +++ b/tests/test-data/full-html/amp_layouts.html.out @@ -65,7 +65,7 @@ - + @@ -82,10 +82,10 @@ TODO(johannes): Long run we should get rid of container or use it. https://github.com/ampproject/amphtml/issues/1109 --> - + - + @@ -116,10 +116,10 @@ - + - + @@ -128,10 +128,10 @@ - + - + @@ -318,16 +318,19 @@ FAIL on line 76 - 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.height attribute was removed due to validation issues. - The mandatory attribute 'src' is missing in tag 'amp-img'. [code: MANDATORY_ATTR_MISSING category: AMP_TAG_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] 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. on line 99 - The attribute 'layout' in tag 'amp-img' is set to the invalid value 'unknown'. [code: INVALID_ATTR_VALUE 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. on line 130 - The implied layout 'FIXED_HEIGHT' is not supported by tag 'amp-pixel'. @@ -336,18 +339,22 @@ FAIL on line 134 - The attribute 'width' in tag 'amp-pixel' is set to the invalid value 'X'. [code: INVALID_ATTR_VALUE category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-pixel.html] + ACTION TAKEN: amp-pixel.width attribute was removed due to validation issues. on line 138 - The attribute 'height' in tag 'amp-pixel' is set to the invalid value 'X'. [code: INVALID_ATTR_VALUE category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-pixel.html] + ACTION TAKEN: amp-pixel.height attribute was removed due to validation issues. on line 154 - The attribute 'heights' in tag 'amp-img' is disallowed by implied layout 'FIXED_HEIGHT'. [code: ATTR_DISALLOWED_BY_IMPLIED_LAYOUT category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-img.html] + ACTION TAKEN: amp-img.heights attribute was removed due to validation issues. COMPONENT NAMES WITH JS PATH ------------------------------ diff --git a/tests/test-data/full-html/mandatory_dimensions.html.out b/tests/test-data/full-html/mandatory_dimensions.html.out index e44bc6ad..50638087 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 @@ - - + + @@ -122,8 +122,8 @@ - - + + @@ -320,10 +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. 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. on line 112 - The implied layout 'CONTAINER' is not supported by tag 'amp-img'. @@ -348,10 +350,12 @@ FAIL on line 119 - 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.height attribute was removed due to validation issues. on line 120 - The attribute 'width' 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.width attribute was removed due to validation issues. on line 123 - Inconsistent units for width and height in tag 'amp-img' - width is specified in 'px' whereas height is specified in 'rem'. diff --git a/tests/test-data/full-html/several_errors.html.out b/tests/test-data/full-html/several_errors.html.out index 0aa9363a..c2eb7209 100644 --- a/tests/test-data/full-html/several_errors.html.out +++ b/tests/test-data/full-html/several_errors.html.out @@ -29,7 +29,7 @@
Tables are allowed
- + @@ -104,6 +104,7 @@ FAIL on line 34 - The attribute 'width' in tag 'amp-ad' is set to the invalid value '100%'. [code: INVALID_ATTR_VALUE category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/amp-ad.html] + ACTION TAKEN: amp-ad.width attribute was removed due to validation issues. - The mandatory attribute 'type' is missing in tag 'amp-ad'. [code: MANDATORY_ATTR_MISSING category: AMP_TAG_PROBLEM see: https://www.ampproject.org/docs/reference/amp-ad.html] diff --git a/tests/test-data/full-html/validator-amp-soundcloud.html.out b/tests/test-data/full-html/validator-amp-soundcloud.html.out index 56d88988..b08b3abd 100644 --- a/tests/test-data/full-html/validator-amp-soundcloud.html.out +++ b/tests/test-data/full-html/validator-amp-soundcloud.html.out @@ -35,7 +35,7 @@ - + @@ -117,6 +117,7 @@ FAIL on line 46 - The specified layout 'RESPONSIVE' is not supported by tag 'amp-soundcloud'. [code: SPECIFIED_LAYOUT_INVALID category: AMP_LAYOUT_PROBLEM see: https://www.ampproject.org/docs/reference/extended/amp-soundcloud.html] + ACTION TAKEN: amp-soundcloud.layout attribute was removed due to validation issues. COMPONENT NAMES WITH JS PATH ------------------------------ diff --git a/tests/test-data/full-html/validator-amp-springboard-player.html.out b/tests/test-data/full-html/validator-amp-springboard-player.html.out index f44cf2be..ba904860 100644 --- a/tests/test-data/full-html/validator-amp-springboard-player.html.out +++ b/tests/test-data/full-html/validator-amp-springboard-player.html.out @@ -45,7 +45,7 @@ - + @@ -180,6 +180,7 @@ FAIL on line 80 - The specified layout 'FIXED_HEIGHT' is not supported by tag 'amp-springboard-player'. [code: SPECIFIED_LAYOUT_INVALID category: AMP_LAYOUT_PROBLEM see: https://github.com/ampproject/amphtml/blob/master/extensions/amp-springboard-player/amp-springboard-player.html] + ACTION TAKEN: amp-springboard-player.layout attribute was removed due to validation issues. - The mandatory attribute 'data-content-id' is missing in tag 'amp-springboard-player'. [code: MANDATORY_ATTR_MISSING category: AMP_TAG_PROBLEM see: https://github.com/ampproject/amphtml/blob/master/extensions/amp-springboard-player/amp-springboard-player.html] - The mandatory attribute 'data-domain' is missing in tag 'amp-springboard-player'. From ba98d2a96862294f14ee27435a2b7815e8b3be82 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Mon, 4 Jul 2016 19:25:52 +0530 Subject: [PATCH 5/8] - Make amp-img a responsive layout if the specified or implied layout is invalid - Fetch image dimensions if the layout is responsive and either/both height and width are missing --- src/AMP.php | 1 + src/Pass/AmpImgFixPass.php | 74 ++++++++++++++++++++++++++++++++ src/Pass/ImgTagTransformPass.php | 3 +- src/Utility/ActionTakenType.php | 1 + 4 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 src/Pass/AmpImgFixPass.php diff --git a/src/AMP.php b/src/AMP.php index ea74d094..9836e62a 100644 --- a/src/AMP.php +++ b/src/AMP.php @@ -65,6 +65,7 @@ class AMP 'Lullabot\AMP\Pass\TwitterTransformPass', 'Lullabot\AMP\Pass\StandardScanPass', 'Lullabot\AMP\Pass\StandardFixPass', + 'Lullabot\AMP\Pass\AmpImgFixPass', 'Lullabot\AMP\Pass\StandardFixPassTwo', 'Lullabot\AMP\Pass\MinimumValidFixPass', 'Lullabot\AMP\Pass\StatisticsPass' diff --git a/src/Pass/AmpImgFixPass.php b/src/Pass/AmpImgFixPass.php new file mode 100644 index 00000000..8a57037b --- /dev/null +++ b/src/Pass/AmpImgFixPass.php @@ -0,0 +1,74 @@ +validation_result->errors as $error) { + if (in_array($error->code, [ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::IMPLIED_LAYOUT_INVALID, ValidationErrorCode::SPECIFIED_LAYOUT_INVALID]) && + !$error->resolved && + !empty($error->dom_tag) && + $error->dom_tag->tagName == 'amp-img' + ) { + $amp_img_el = new DOMQuery($error->dom_tag); + + if (in_array($error->code, [ValidationErrorCode::IMPLIED_LAYOUT_INVALID, ValidationErrorCode::SPECIFIED_LAYOUT_INVALID])) { + $amp_img_el->attr('layout', 'responsive'); + } + + $layout = ParsedTagSpec::parseLayout($amp_img_el->attr('layout')); + if ($error->code == ValidationErrorCode::MANDATORY_ATTR_MISSING && + ($layout !== AmpLayoutLayout::RESPONSIVE || !in_array($error->params[0], ['height', 'width'])) + ) { + continue; + } + + $this->setAmpImgAttributes($amp_img_el); + $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 e0166a94..6cd91343 100644 --- a/src/Pass/ImgTagTransformPass.php +++ b/src/Pass/ImgTagTransformPass.php @@ -76,6 +76,7 @@ function pass() $el->remove(); // remove the old img tag $this->setAmpImgAttributes($new_el); + $this->setLayoutIfNoLayout($new_el, 'responsive'); $this->context->addLineAssociation($new_dom_el, $lineno); $this->addActionTaken(new ActionTakenLine('img', ActionTakenType::IMG_CONVERTED, $lineno, $context_string)); } @@ -165,7 +166,5 @@ protected function setAmpImgAttributes(DOMQuery $el) $el->attr('height', $dimensions['height']); } } - - $this->setLayoutIfNoLayout($el, 'responsive'); } } diff --git a/src/Utility/ActionTakenType.php b/src/Utility/ActionTakenType.php index 946746fb..7c76c27e 100644 --- a/src/Utility/ActionTakenType.php +++ b/src/Utility/ActionTakenType.php @@ -47,4 +47,5 @@ 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-image by adjusting one or more of: height, width, and setting layout to responsive'; } From 82494cf48702be18351314179ccee00f5112a47f Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Mon, 4 Jul 2016 19:35:24 +0530 Subject: [PATCH 6/8] Add concept of failure/success to setImgAttributes() and tweak AmpImgFixPass accordingly --- src/Pass/AmpImgFixPass.php | 12 +++++++++--- src/Pass/ImgTagTransformPass.php | 5 +++++ src/Utility/ActionTakenType.php | 3 ++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/Pass/AmpImgFixPass.php b/src/Pass/AmpImgFixPass.php index 8a57037b..2fa35b2f 100644 --- a/src/Pass/AmpImgFixPass.php +++ b/src/Pass/AmpImgFixPass.php @@ -56,6 +56,8 @@ function pass() if (in_array($error->code, [ValidationErrorCode::IMPLIED_LAYOUT_INVALID, ValidationErrorCode::SPECIFIED_LAYOUT_INVALID])) { $amp_img_el->attr('layout', 'responsive'); + $error->addActionTaken(new ActionTakenLine('amp-img', ActionTakenType::AMP_IMG_FIX_RESPONSIVE)); + $error->resolved = true; } $layout = ParsedTagSpec::parseLayout($amp_img_el->attr('layout')); @@ -65,9 +67,13 @@ function pass() continue; } - $this->setAmpImgAttributes($amp_img_el); - $error->addActionTaken(new ActionTakenLine('amp-img', ActionTakenType::AMP_IMG_FIX)); - $error->resolved = true; + $success = $this->setAmpImgAttributes($amp_img_el); + if ($success) { + $error->addActionTaken(new ActionTakenLine('amp-img', ActionTakenType::AMP_IMG_FIX)); + $error->resolved = true; + } else { + $error->resolved = false; + } } } } diff --git a/src/Pass/ImgTagTransformPass.php b/src/Pass/ImgTagTransformPass.php index 6cd91343..ab889804 100644 --- a/src/Pass/ImgTagTransformPass.php +++ b/src/Pass/ImgTagTransformPass.php @@ -164,7 +164,12 @@ protected function setAmpImgAttributes(DOMQuery $el) if ($dimensions !== false) { $el->attr('width', $dimensions['width']); $el->attr('height', $dimensions['height']); + return true; + } else { + return false; } } + + return true; } } diff --git a/src/Utility/ActionTakenType.php b/src/Utility/ActionTakenType.php index 7c76c27e..cf550c03 100644 --- a/src/Utility/ActionTakenType.php +++ b/src/Utility/ActionTakenType.php @@ -47,5 +47,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-image 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 adjusting one or more of: height, width, and setting layout to responsive'; + const AMP_IMG_FIX_RESPONSIVE = 'tried to fix problems with amp-img by setting layout to responsive'; } From 7c7a9805ba2b989522204d446c704453bbe13413 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Mon, 4 Jul 2016 19:50:38 +0530 Subject: [PATCH 7/8] Abort img transformation if dimensions could not be retreived --- src/Pass/ImgTagTransformPass.php | 12 ++++++++++-- src/Utility/ActionTakenType.php | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Pass/ImgTagTransformPass.php b/src/Pass/ImgTagTransformPass.php index ab889804..6b9f1766 100644 --- a/src/Pass/ImgTagTransformPass.php +++ b/src/Pass/ImgTagTransformPass.php @@ -73,9 +73,17 @@ function pass() $context_string = $this->getContextString($dom_el); $new_dom_el = $this->cloneAndRenameDomElement($dom_el, 'amp-img'); $new_el = $el->prev(); - $el->remove(); // remove the old img tag - $this->setAmpImgAttributes($new_el); + $success = $this->setAmpImgAttributes($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)); + // Abort the conversion and remove the new img tag + $new_el->remove(); + continue; + } + + $el->remove(); // remove the old img tag $this->setLayoutIfNoLayout($new_el, 'responsive'); $this->context->addLineAssociation($new_dom_el, $lineno); $this->addActionTaken(new ActionTakenLine('img', ActionTakenType::IMG_CONVERTED, $lineno, $context_string)); diff --git a/src/Utility/ActionTakenType.php b/src/Utility/ActionTakenType.php index cf550c03..9e138cdb 100644 --- a/src/Utility/ActionTakenType.php +++ b/src/Utility/ActionTakenType.php @@ -24,6 +24,7 @@ class ActionTakenType const PROPERTY_REMOVED = 'property value pair was removed from attribute due to validation issues.'; const PROPERTY_REMOVED_ATTRIBUTE_REMOVED = 'property value pair was removed from attribute due to validation issues. The resulting attribute was empty and was also removed.'; const IMG_CONVERTED = 'tag was converted to the amp-img tag.'; + const IMG_COULD_NOT_BE_CONVERTED = 'tag could NOT be converted to the amp-img tag as the image is not accessible.'; const INSTAGRAM_CONVERTED = 'instagram embed code was converted to the amp-instagram tag.'; const PINTEREST_CONVERTED = 'pinterest embed code was converted to the amp-pinterest tag.'; const VINE_CONVERTED = 'vine embed code was converted to the amp-vine tag.'; From 1525bccc4dfd168ce27b9140674f8adf4cdebdef Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Mon, 4 Jul 2016 19:52:14 +0530 Subject: [PATCH 8/8] regen mandatory_dimensions.html expected 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 50638087..ab515a83 100644 --- a/tests/test-data/full-html/mandatory_dimensions.html.out +++ b/tests/test-data/full-html/mandatory_dimensions.html.out @@ -115,7 +115,7 @@ - + @@ -330,6 +330,7 @@ FAIL on line 112 - 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 setting layout to responsive on line 115 - The mandatory attribute 'height' is missing in tag 'amp-img'.