Problem/Motivation
IMHO if SchemaArticleImage url is empty, we shouldn't render the object at all. If we use a token for url, and that token is empty, we render e.g.:
"image": {
"@type": "ImageObject",
"representativeOfPage": "True",
"width": "499",
"height": "376"
},
In SchemaMetatagManager we have logic to avoid those situations, skipping data if @type is the only value set, but that doesn't cover cases like this one, where more than @type is provided, but still the object has no semantic meaning at all without a defined property.
Steps to reproduce
1. Map image url to a field image with an image style and absolute url.
2. Hardcode width and height because we used an image style defining width and height, and representativeOfPage because we know which value we want there.
3. Don't have an image in our content.
Proposed resolution
Override SchemaArticleImage::output() like
public function output() {
$result = parent::output();
if (empty($result['#attributes']['content']['url'])) {
return '';
}
else {
return $result;
}
}
Remaining tasks
Patch. Tests?
User interface changes
If no url, no image object is rendered.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | schema_metatag-3339740-avoid-image-without-url-8.patch | 10.33 KB | penyaskito |
Comments
Comment #2
penyaskitoImproved wording. Added STR.
Comment #3
damienmckennaThat seems entirely reasonable.
At a surface level we should make sure that the Image object does not output if the URL value is empty.
We should (separately) look at whether it would be possible to indicate required values that would cause the structure to not be output at all if they are not present.
Comment #4
penyaskito@DamienMcKenna I was wondering if it would make sense to introduce e.g. a SchemaImageObjectBase class, and try to imitate more accurately the types specificity of Schema.org Types instead of all tags inheriting from SchemaNameBase. But yes, definitely out of scope here.
Comment #5
penyaskitoWell, maybe not that out of scope. I'm pretty sure I will encounter the same issue if I test HowTo schemas. This should fix all.
Comment #6
penyaskitoThis is problematic. We use empty string, but the function contract returned value is array.
This is already a problem in HEAD with
SchemaNameBase::output()I'm using 2.4 so this might work for me, but definitely we need a different solution here.
Comment #7
damienmckennaThat's pretty good, thank you.
What happens if it returns an empty array instead of an empty string?
Comment #8
penyaskitoAh, it actually works, wasn't 100% confident.
Comment #10
damienmckennaCommitted. Thank you.
Comment #12
penyaskitoJust found that I copypasted this and the comment should say
All Schema.org **images** should extend this class.
Comment #15
damienmckennaCommitted. Thanks for the follow-up.