Skip to content

Commit

Permalink
Fix #208
Browse files Browse the repository at this point in the history
  • Loading branch information
DirtyF committed Feb 6, 2018
1 parent 0666d4f commit e24f30c
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions lib/jekyll-feed/feed.xml
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@
{% assign post_image = post.image.path | default: post.image %}
{% if post_image %}
{% unless post_image contains "://" %}
{% assign post_image = post_image | absolute_url | xml_escape %}
{% assign post_image = post_image | absolute_url %}
{% endunless %}
<media:thumbnail xmlns:media="http://search.yahoo.com/mrss/" url="{{ post_image }}" />
<media:thumbnail xmlns:media="http://search.yahoo.com/mrss/" url="{{ post_image | xml_escape }}" />
{% endif %}
</entry>
{% endfor %}
Expand Down
2 changes: 1 addition & 1 deletion spec/fixtures/_posts/2014-03-02-march-the-second.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
image: "https://cdn.example.org/absolute.png"
image: https://cdn.example.org/absolute.png?h=188&w=250
---

March the second!
2 changes: 1 addition & 1 deletion spec/jekyll-feed_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@

it "includes the item image" do
expect(contents).to include('<media:thumbnail xmlns:media="http://search.yahoo.com/mrss/" url="http://example.org/image.png" />')
expect(contents).to include('<media:thumbnail xmlns:media="http://search.yahoo.com/mrss/" url="https://cdn.example.org/absolute.png" />')
expect(contents).to include('<media:thumbnail xmlns:media="http://search.yahoo.com/mrss/" url="https://cdn.example.org/absolute.png?h=188&amp;w=250" />')
expect(contents).to include('<media:thumbnail xmlns:media="http://search.yahoo.com/mrss/" url="http://example.org/object-image.png" />')
end

Expand Down

4 comments on commit e24f30c

@bittlingmayer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, in one code path, xml_escape will now be run twice. Is it idempotent, or will it double-escape?

@DirtyF
Copy link
Member Author

@DirtyF DirtyF commented on e24f30c Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still runs once.

Just modify your Gemfile to test this on your site:

gem "jekyll-feed", git: "https://github.com/jekyll/jekyll-feed"

@pathawks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m having trouble spotting it.
@bittlingmayer Could you point to line numbers? It would be nice to avoid the double call (for performance reasons only; correctness is not in question)

@bittlingmayer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pathawks I'm wrong, @DirtyF is correct, somehow I had not seen that it was removed from the other line.

Please sign in to comment.