Skip to content

Commit

Permalink
Changes RSS item creation to prevent encoding errors
Browse files Browse the repository at this point in the history
SimpleRss is unreliable with parsing RSS feeds that contain German Umlauts. 
For example this feed http://www.lauffeuer-lb.de/api/v2/articles.xml can't be parsed by SimpleRss. Discourse's logs are full of  

```
Job exception: Wrapped Encoding::CompatibilityError: incompatible character encodings: ASCII-8BIT and UTF-8  
Job exception: incompatible character encodings: ASCII-8BIT and UTF-8
```

The embedding fails because the feed can't be parsed.  

This change forces the encoding which prevents the numerous encoding errors.
  • Loading branch information
5minpause committed May 12, 2015
1 parent 54b63bf commit 47f3c54
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions app/jobs/scheduled/poll_feed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,15 @@ def url
end

def content
@article_rss_item.content || @article_rss_item.description
if @article_rss_item.content
@article_rss_item.content.force_encoding(Encoding::UTF_8)
else
@article_rss_item.description.force_encoding(Encoding::UTF_8)
end
end

def title
@article_rss_item.title
@article_rss_item.title.force_encoding(Encoding::UTF_8)
end

def user
Expand Down

9 comments on commit 47f3c54

@darix
Copy link

@darix darix commented on 47f3c54 May 12, 2015

Choose a reason for hiding this comment

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

This is wrong. If you read the post on the forums the proper way is to disallow simple-rss to break the encoding.

@5minpause
Copy link
Owner Author

Choose a reason for hiding this comment

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

Can you please point to the post you are talking about?

@darix
Copy link

@darix darix commented on 47f3c54 May 12, 2015

Choose a reason for hiding this comment

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

@5minpause
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah yes. Now I understand what you are talking about. Sure. The best and the right way is to fix SimpleRss. But I don't know when this will happen. My pull request is a fix for right now and fixes my problem — and I guess I am not the only one.

@5minpause
Copy link
Owner Author

Choose a reason for hiding this comment

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

Btw the pull request you linked doesn't even fix every problem. I created a test case for SimpleRss, where it drops German Umlauts from titles. Even after merging with the PR you linked. SimpleRss just doesn't seem to work (properly for German Umlauts). Thanks for your input.

@darix
Copy link

@darix darix commented on 47f3c54 May 12, 2015

Choose a reason for hiding this comment

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

you sure are not the only one. but you fix the problem in the wrong place. :) We run discuss.pixls.us with the simple-rss patch and my own instance with the feedjira patch.

@5minpause
Copy link
Owner Author

Choose a reason for hiding this comment

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

That's debatable. Of course I am all for fixing SimpleRss. I am even more for changing Discourse to use Feedjirra or something else. But I can't make that fix for Discourse and I can't fix SimpleRss. I am not in the position to fork Discourse right now. That's why I am going the route of least changes for Discourse, hoping that they might see it like this as well.
If I can help in fixing SimpleRss or contribute in any other way to fix the root of the problem let me know how.

@darix
Copy link

@darix darix commented on 47f3c54 May 12, 2015

Choose a reason for hiding this comment

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

can you give the feedjira patch a try?

@5minpause
Copy link
Owner Author

Choose a reason for hiding this comment

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

No unfortunately I don't have the time right now to do that. Sorry.

Please sign in to comment.