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..2fa35b2f
--- /dev/null
+++ b/src/Pass/AmpImgFixPass.php
@@ -0,0 +1,80 @@
+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');
+ $error->addActionTaken(new ActionTakenLine('amp-img', ActionTakenType::AMP_IMG_FIX_RESPONSIVE));
+ $error->resolved = true;
+ }
+
+ $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;
+ }
+
+ $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 e0166a94..6b9f1766 100644
--- a/src/Pass/ImgTagTransformPass.php
+++ b/src/Pass/ImgTagTransformPass.php
@@ -73,9 +73,18 @@ 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));
}
@@ -163,9 +172,12 @@ protected function setAmpImgAttributes(DOMQuery $el)
if ($dimensions !== false) {
$el->attr('width', $dimensions['width']);
$el->attr('height', $dimensions['height']);
+ return true;
+ } else {
+ return false;
}
}
- $this->setLayoutIfNoLayout($el, 'responsive');
+ return true;
}
}
diff --git a/src/Pass/StandardFixPass.php b/src/Pass/StandardFixPass.php
index 76716b29..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
- // @todo ValidationErrorCode::MUTUALLY_EXCLUSIVE_ATTRS?
+ 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/Utility/ActionTakenType.php b/src/Utility/ActionTakenType.php
index 946746fb..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.';
@@ -47,4 +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_RESPONSIVE = 'tried to fix problems with amp-img by setting layout to responsive';
}
diff --git a/src/Validate/ParsedTagSpec.php b/src/Validate/ParsedTagSpec.php
index c5393029..32981647 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, empty($layout_attr) ? '' : 'layout');
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;
}
}
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 @@
-
+
-
+