Improve logic to detect explicitly marked hero images #1198
Conversation
|
|
||
| isMarkedAsHeroImage(node) { | ||
| while (node) { | ||
| if (!node.tagName) { |
There was a problem hiding this comment.
Why would tagName ever be empty?
There was a problem hiding this comment.
If it's not an element, but a different type of node. I initially tried to check for HTMLElement, but that didn't work as I expected.
There was a problem hiding this comment.
It seems elements are only ever passed into this function, but I guess this is good for hardening.
There was a problem hiding this comment.
Yes, but this check is within a loop that loops from parent to parent, so this does not only check what is passed into the function, but every ancestor of it as well. And as elements can also be children of non-elements (see https://www.w3schools.com/jsref/prop_node_nodetype.asp), I think this is needed for some edge cases.
…bute from removed fallback img
|
@schlessera @sebastianbenz I amended the PR with 16a1a85 which I've done in PHP via ampproject/amp-toolbox-php#149. |
The logic in #1196 fails to properly detect a nested construct, like if an iframe if marked with
data-heroand its placeholder image is SSRed.This PR improves the logic to properly detect these cases.