rbuchberger / jekyll_picture_tag

Easy responsive images for Jekyll.
https://rbuchberger.github.io/jekyll_picture_tag/
BSD 3-Clause "New" or "Revised" License
622 stars 106 forks source link

Don't use void tags / output XML #307

Open KjellMorgenstern opened 5 months ago

KjellMorgenstern commented 5 months ago

Can jekyll_picture_tag output closed img tags?

{% picture {{ post.image }} preset="thumbnail" %}

produces a void tag like <img preset="thumbnail" src="/generated/assets/images/my_post.png" srcset="/generated/assets/images/my_post-175-43e233202.webp 175w" sizes="() 100vw">

XML parsers frown on this, e.g when the output gets fed to a newsfeed, it should be <img preset="thumnail" src=... />

with a closing />

To work around this, I use (*)

  {% capture img_tag %}
    {% picture {{ post.image }} preset="thumbnail" %}
  {% endcapture %}
  {% assign img_tag = img_tag | replace: '>', ' />' %}
  {{ img_tag | replace: ' />/', '/>' }}

and this seems to work. However, it looks bad, and I think it will fail on a more complex use case. Can jekyll_picture_tag simply close the tag? Or am I missing somthing important?

(*) if its a few images, just manually write the img tag and add the thumbnail to assets

rbuchberger commented 5 months ago

Thanks for the idea!

So the HTML spec recommends against closing slashes in void elements (which includes the img tag) -

Then, if the element is one of the void elements, or if the element is a foreign element, then there may be a single U+002F SOLIDUS character (/), which on foreign elements marks the start tag as self-closing. On void elements, it does not mark the start tag as self-closing but instead is unnecessary and has no effect of any kind. For such void elements, it should be used only with caution — especially since, if directly preceded by an unquoted attribute value, it becomes part of the attribute value rather than being discarded by the parser.

MDN docs on void elements - easier to read

That said, if you're feeding this to react or something then the closing slash is required. It makes sense to add it as an option, disabled by default. Not sure when I'll get to this, but it's a good idea.