New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

picture tag within hyperlinks generates mangled HTML #104

Closed
user-none opened this Issue Feb 3, 2019 · 9 comments

Comments

Projects
None yet
3 participants
@user-none
Copy link
Contributor

user-none commented Feb 3, 2019

Problem

I wrap all of my images in a link to the full size image. Using <img> tag output everything works properly. However, <picture> output is mangled.

Using either markdown links or <a> tags directly is broken. The output is generated differently (broken in a different way) if markdown vs <a> tags are used. I can get it working with a <div> wrapping an <a> and the {% picture %} within the <a>.

I prefer to use markdown links throughout all my _posts and the <div> wrapper is a hack I'd rather not put thought my blog.

Steps to reproduce

  1. Create an empty Jekyll site. jekyll new myblog
  2. Install jekyll-picture-image

_config.yml

picture:
  source: "images"
  output: "generated"
  suppress_warnings: false

_data/picture.yml

markup_presets:
  default:
    markup: picture
    formats: [webp, original]
    widths: [200, 400, 800, 1600]

Input/Output

Markdown Links

generic_post.markdown

[{% picture s.png %}](/images/s.png)

Generated HTML (BAD)

<p>[<picture></picture></p>
<source srcset="http://localhost:4000/generated/s-200by26-70cbfd.webp 200w, http://localhost:4000/generated/s-400by53-70cbfd.webp 400w, http://localhost:4000/generated/s-800by106-70cbfd.webp 800w, http://localhost:4000/generated/s-1288by170-70cbfd.webp 1288w" type="image/webp" />

<source srcset="http://localhost:4000/generated/s-200by26-70cbfd.png 200w, http://localhost:4000/generated/s-400by53-70cbfd.png 400w, http://localhost:4000/generated/s-800by106-70cbfd.png 800w, http://localhost:4000/generated/s-1288by170-70cbfd.png 1288w" type="image/png" />

<p><img src="http://localhost:4000/generated/s-800by106-70cbfd.png" />
&lt;/picture&gt;
](/images/s.png)</p>

<a> tag

generic_post_atag.markdown

<a href="/images/s.png">{% picture s.png %}</a>

Generated HTML (BAD)

<p><a href="/images/s.png"><picture></picture></a></p>
<source srcset="http://localhost:4000/generated/s-200by26-70cbfd.webp 200w, http://localhost:4000/generated/s-400by53-70cbfd.webp 400w, http://localhost:4000/generated/s-800by106-70cbfd.webp 800w, http://localhost:4000/generated/s-1288by170-70cbfd.webp 1288w" type="image/webp" />

<source srcset="http://localhost:4000/generated/s-200by26-70cbfd.png 200w, http://localhost:4000/generated/s-400by53-70cbfd.png 400w, http://localhost:4000/generated/s-800by106-70cbfd.png 800w, http://localhost:4000/generated/s-1288by170-70cbfd.png 1288w" type="image/png" />

<p><img src="http://localhost:4000/generated/s-800by106-70cbfd.png" />
&lt;/picture&gt;
&lt;/a&gt;</p>

Working with <div> wrapper

The only way I've been able to get proper links around the picture-tag images is to have a <div> around an <a> tag. I have not been able to find a way to get this working with markdown links.

generic_post_working.markdown

<div><a href="/images/s.png">{% picture s.png %}</a></div>

Generated HTML (Good)

<div><a href="/images/s.png"><picture>
  <source srcset="http://localhost:4000/generated/s-200by26-70cbfd.webp 200w, http://localhost:4000/generated/s-400by53-70cbfd.webp 400w, http://localhost:4000/generated/s-800by106-70cbfd.webp 800w, http://localhost:4000/generated/s-1288by170-70cbfd.webp 1288w" type="image/webp" />
  <source srcset="http://localhost:4000/generated/s-200by26-70cbfd.png 200w, http://localhost:4000/generated/s-400by53-70cbfd.png 400w, http://localhost:4000/generated/s-800by106-70cbfd.png 800w, http://localhost:4000/generated/s-1288by170-70cbfd.png 1288w" type="image/png" />
  <img src="http://localhost:4000/generated/s-800by106-70cbfd.png" />
</picture>
</a></div>

@rbuchberger rbuchberger added the bug label Feb 4, 2019

@rbuchberger

This comment has been minimized.

Copy link
Collaborator

rbuchberger commented Feb 4, 2019

I see what you're talking about, I can reproduce it. It's really odd; j-p-t doesn't look at anything outside of its enclosing {% %}, so I'm baffled as to how its output is getting jumbled up with the surrounding anchor tag.

html files don't have this issue, which means it has something to do with markdown parsing. Will need further investigation. Thanks for the bug report!

@rbuchberger

This comment has been minimized.

Copy link
Collaborator

rbuchberger commented Feb 6, 2019

I've been testing this some more, it happens even with plain HTML. With this code in a markdown file:

<a href="example.com">
  <picture>
    <source srcset="image.webp 1.0x, image2.webp 1.5x, image3.webp 2.0x" type="image/webp" />
    <source srcset="image.jpg 1.0x, image2.jpg 1.5x, image3.jpg 2.0x" type="image/jpeg" />
    <img alt="old way" src="fallback.jpg" />
  </picture>
</a>

You get this output:

<p><a href="example.com"></a></p>
<picture>
    <source srcset="image.webp 1.0x, image2.webp 1.5x, image3.webp 2.0x" type="image/webp" />
    <source srcset="image.jpg 1.0x, image2.jpg 1.5x, image3.jpg 2.0x" type="image/jpeg" />
    <img alt="old way" src="fallback.jpg" />
  </picture>
<p>&lt;/a&gt;</p>

Clearly kramdown doesn't know this is HTML and it shouldn't be touched. I've tried setting markdown="0" (as well as block) in the surrounding anchor tag, and wrapping the whole thing in {% raw %} (...) {% endraw %}. The only workaround I can see so far is wrapping the whole thing in a <div> tag, which convinces kramdown that it really is HTML and don't touch it.

Reading the relevant documentation hasn't gotten me anywhere, though interestingly support for <picture> and <source> aren't explicitly stated.

@ashmaroli

This comment has been minimized.

Copy link

ashmaroli commented Feb 7, 2019

Perhaps this observation should be filed at the kramdown repo.

@rbuchberger

This comment has been minimized.

Copy link
Collaborator

rbuchberger commented Feb 7, 2019

That was going to be my next step, but first I wanted to be sure it's not some mistake of mine. Thanks!

@rbuchberger

This comment has been minimized.

Copy link
Collaborator

rbuchberger commented Feb 7, 2019

Ok, further testing shows that it's definitely an issue with kramdown:

If I create a simple test script using the same version as jekyll:

# Gemfile
source 'https://rubygems.org'
gem 'kramdown', '~>1.14'
# test.rb
require 'kramdown'

def tag
  <<-HEREDOC
<a href="example.com" markdown="block">
  <picture>
    <source srcset="image.webp 1.0x, image2.webp 1.5x, image3.webp 2.0x" type="image/webp" />
    <source srcset="image.jpg 1.0x, image2.jpg 1.5x, image3.jpg 2.0x" type="image/jpeg" />
    <img alt="old way" src="fallback.jpg" />
  </picture>
</a>
  HEREDOC
end

puts Kramdown::Document.new(tag).to_html

I get the mangled output:

<p><a href="example.com"></a></p>
<picture>
    <source srcset="image.webp 1.0x, image2.webp 1.5x, image3.webp 2.0x" type="image/webp" />
    <source srcset="image.jpg 1.0x, image2.jpg 1.5x, image3.jpg 2.0x" type="image/jpeg" />
    <img alt="old way" src="fallback.jpg" />
  </picture>
<p>&lt;/a&gt;</p>

Updating to the most recent version doesn't fix it. Removing markdown="block" from the <a> tag doesn't fix it.

@rbuchberger

This comment has been minimized.

Copy link
Collaborator

rbuchberger commented Feb 7, 2019

They gave me a solution! The {::nomarkdown} tag. This should work:

{::nomarkdown}
<a href="example.jpg">{% picture example.jpg %}</a>
{/:}

I'll write a conditional to check if it's a markdown file and insert those tags automatically. Automatic linking to the source image would be a nice feature as well, and not too difficult to add.

@user-none

This comment has been minimized.

Copy link
Contributor Author

user-none commented Feb 7, 2019

The plan is to detect markdown files then around the j-p-t html that's inserted you'll have it surrounded by {::nomarkdown} ?

So we'll see this after j-p-t runs but before kramdown:

[{% picture s.png %}](/images/s.png)
->
[{::nomarkdown}<picture>...</picture>{/:}](/images/s.png)
@rbuchberger

This comment has been minimized.

Copy link
Collaborator

rbuchberger commented Feb 7, 2019

Yep, exactly. That gives a result of:

<p><a href="/images/s.png"><picture>...</picture></a></p>

But I'd also like to add a feature to automatically wrap it in an anchor tag pointing to the source image. j-p-t already has that information, there's no reason I can't add that option for you.

@user-none

This comment has been minimized.

Copy link
Contributor Author

user-none commented Feb 7, 2019

Fantastic! Thanks!

This was referenced Feb 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment