Opened 9 years ago

Closed 4 years ago

#328 closed enhancement (fixed)

Atom feed thumbnails

Reported by: maiki interi Owned by:
Priority: minor Milestone:
Component: programming Keywords: kuneco
Cc: Parent Tickets:

Description (last modified by Will Kahn-Greene)

Not entirely on topic with `#658 </issues/658>`_ or
`#659 </issues/659>`_, but related.

My expectation was for the atom feed to contain the thumbnail of
the image (or equivalent of other media types). This is important
to me, as I want to widgetize/aggregate the feed on other sites.

I found two places where feeds are generated,
``mediagoblin/user_pages/views.py`` and
``mediagoblin/listings/views.py``:

::

        for entry in cursor:
            feed.add(entry.get('title'),
                entry.get('description_html'),
                content_type='html',
                author=request.matchdict['user'],
                updated=entry.get('created'),
                url=entry.url_for_self(request.urlgen))

I am not sure what the thumbnail is, but I am betting it is a
**summary** or maybe **content** (see the
`werkzeug atom docs <http://werkzeug.pocoo.org/docs/contrib/atom/>`_).
I think this is important, even if we are moving to make it a full
ActivityStream, since the federation I have in mind is something
akin to a dashboard view of new photos posted on my friends'
sites.

So, can we get a thumbnail in there? =]



Subtickets

Attachments (1)

328_atom_feed_thumbnails.patch (1.6 KB) - added by Ben Sturmfels 4 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 9 years ago by Christopher Allan Webber

Yes we certainly need a way to share this...



comment:2 Changed 9 years ago by Inconexo ø

I have done a try. Not very happy because I had to:


1) Write html code inside python code. I don't feel that very
   orthodox.
2) Img comes inside html content. I don't know if there is a better
   way to include this, but I haven't found it inside the atom library
   documentation.

Please, tell me any comment that would improve my implementation.

git:
[https://gitorious.org/\ :sub:`inconexo/mediagoblin/inconexos-mediagoblin](https://gitorious.org/`\ inconexo/mediagoblin/inconexos-mediagoblin)
branch: 676\_feed\_thumbs



comment:3 Changed 9 years ago by Inconexo ø

Status: NewFeedback

comment:3 Changed 9 years ago by Christopher Allan Webber

Owner: set to Inconexo ø
Hey Inconexo, thanks for the work on this. :)

I think the best route would be to actually shift over the work to
making our own atom feed generator... I think we'll need to for
`#659 </issues/659>`_ anyway. And this would avoid such grossness
as we're extending that code.

Inconexo: if you're interested in pushing us toward a beautiful
federated future, a good way to start might be to help write an
atom feed generator using lxml :)
`http://lxml.de/tutorial.html <http://lxml.de/tutorial.html>`_

If you're interested in this, let me know! If you are interested
but have no experience with lxml, read that tutorial and ping me on
irc... you'll find it's pretty easy though, I think.



comment:4 Changed 9 years ago by Inconexo ø

Well, finally, feed library is written. Please notice I am a
beginner with Python. Please comment any problem and I will fix
it.

I have kept two goals:


1) Same interface as Werkzeurg's. This way, changing one feed
   generator to other is changing the import.
2) Extensibility. I have made a generator class. If we wanted to
   generate RSS or other feed systems, we could write another
   generator, reusing a lot of code.

I have also added thumbnails capabilities. First, I added
media:thumbnails elements, but it doesn't seem to work with
firefox' and other's agregators. Any help with that is appreciated.
So, I used the same old trick: including the thumbnail inside the
content.

Anyway, you can take a look:

git:
[https://gitorious.org/\ :sub:`inconexo/mediagoblin/inconexos-mediagoblin](https://gitorious.org/`\ inconexo/mediagoblin/inconexos-mediagoblin)
branch: feed\_library



comment:5 Changed 9 years ago by Christopher Allan Webber

Milestone: 0.2.00.2.1

comment:5 Changed 9 years ago by Elrond

Okay, this one diverged somewhat into "we need a feed library". And
one was created.
This is actually bug `#711 </issues/711>`_.
So made this one block'd by 711.

When 711 is done, we should revisit this one.



comment:6 Changed 9 years ago by Will Kahn-Greene

The original url for this bug was http://bugs.foocorp.net/issues/676 .
Relations:
#310: related, #311: related, #358: blocked

comment:7 Changed 9 years ago by Elrond

Keywords: kuneco added
Milestone: 0.2.1

comment:8 Changed 8 years ago by spaetz

To post media in the atom feed, we'll need to add something like:

<link rel="enclosure" type="image/jpeg" href="http://farm9.staticflickr.com/8343/8187137313_2e211c431f_b.jpg" />

to the atom feed. At least this is what the flickr atom feed does.

comment:9 Changed 8 years ago by spaetz

I pushed a Work-In-Progess branch 328_media_in_atom_feed to my repo at git://gitorious.org/~spaetz/mediagoblin/spaetz-mediagoblin.git

It does not implement a new Atom feed generator, but at least it embeds the posted media as a
<link href="....jpg" rel="enclosure"/> which is what the flickr atom feed also does to convey the posted media.

I stumbled upon 2 roadblocks while implementing: mg_globals.public_store.file_url(media_entry) returns a relativ URL without HOST and we need a real fully external URL there. How can we achieve that?

Second, how can I determine the media type of the media so that browsers detect it correctly? Right now I hardcoded 'type': 'image/jpeg' which is obviously bad.

comment:10 Changed 8 years ago by Will Kahn-Greene

Description: modified (diff)

Given that this is about thumbnails and not enclosing the actual media item, it's not corrrect to use <link href="..." rel="enclosure"/>. That's for enclosing media items like for podcasts, picturecasts, etc. Flickr encloses the entire media item.

To have thumbnails, you should use the MRSS module:

http://www.feedforall.com/mediarss.htm

Firefox feed preview shows enclosures, but not thumbnails (weirdly, I helped write that).

What YouTube does is create an HTML thingy with the thumbnail in that and put that in the description. I suggest you do something like that. That's very common with podcast-like feeds.

comment:11 Changed 8 years ago by Christopher Allan Webber

Will says MRSS should work with Atom.

comment:12 Changed 8 years ago by mrb

FYI: https://gist.github.com/4398058 is my local patch to have an enclosure link.

comment:13 Changed 7 years ago by Christopher Allan Webber

Owner: Inconexo ø deleted
Status: acceptedreview

comment:14 Changed 7 years ago by Christopher Allan Webber

Heya,

I've been delayed in looking at mrb's patch... however, I've finally reviewed it!

Stylistically and code-cleanliness wise this patch seems very nice. I'm afraid it makes some assumptions that we can't make though: it's assuming that all different media types provide an "original" file (not all do!) and that this is what we want to serve.

Maybe a good solution instead is to take advantage of the MediaManager class and add a "enclosure" method. The method could take a request as an object and return a list(?) of files to give in the enclosure? (I remember that Atom feeds can have multiple enclosures for different sizes / file types...)

Thoughts?

comment:15 Changed 7 years ago by Christopher Allan Webber

Status: reviewaccepted

comment:16 Changed 4 years ago by Ben Sturmfels

Status: acceptedreview
Type: defectenhancement

I've revised mrb's patch to include both a thumbnail image and enclosures for all available media files.

Diff:
https://gitlab.com/BenSturmfels/mediagoblin/compare/master...328-atom-thumbnails

Check out:
git fetch https://gitlab.com/BenSturmfels/mediagoblin.git 328-atom-thumbnails
git checkout FETCH_HEAD

Will recommended MRSS's thumbnail element, but I couldn't find any examples of this in an Atom feed in the wild. Found some in RSS, but not Atom. Did some tests and wasn't able to get a thumbnail to show up in Firefox or Liferea using something like <media:thumbnail xmlns:media="http://search.yahoo.com/mrss/" url="..." />. Also, we can't add a thumbnail element without subclassing and modifying werkzeug.contrib.atom.AtomFeed. Flickr uses an <img> in the Atom content, so I'll go with that for now, unless anyone can suggest an live Atom feed using thumbnail and an Atom client that renders thumbnail.

I've built on mrb's patch, but added fully-qualified URLs for enclosures, which I believe is required. I've also included all media files as enclosures, instead of just hard-coding to original.

I also investigated some more creative solutions such as specifying an image *as* the content, as per http://www.ibm.com/developerworks/library/x-atom10/index.html#N10111. This both doesn't play well with the interface provided by werkzeug.contrib.atom.AtomFeed, and I also couldn't get a client to render it.

comment:17 Changed 4 years ago by ayleph

Hey sturm, thanks for revising the original patch. I tested this on one of my instances, and there are a couple of things that I'm not sure about.

  1. On Firefox, I see the main image as well as the enclosure you described. However, when I open the atom link in Chromium, it seems to display a plaintext xml page instead of an atom feed. Did you happen to try this? Maybe this is just a known issue with Chromium and atom feeds.
  1. I don't know if it makes sense to show all of the available media in the enclosure. Based on what I've seen on other sites, just showing the thumbnail image and providing a link back to the hosted media is what I'd prefer. I don't see much value in including links to the various media files in the atom feed. How do you feel about this?

edited for spelling

Last edited 4 years ago by ayleph (previous) (diff)

comment:18 Changed 4 years ago by ayleph

After browsing to various sites, it appears that Chromium is a lot stricter about feed formats than Firefox. For example, Engadget's RSS feed works fine in Firefox but not Chromium, while Slashdot's RSS feed works in both. This appears to have nothing to do with the current patch, so it seems to be out of scope for this ticket.

comment:19 in reply to:  17 Changed 4 years ago by Ben Sturmfels

Replying to ayleph:

  1. I don't know if it makes sense to show all of the available media in the enclosure. Based on what I've seen on other sites, just showing the thumbnail image and providing a link back to the hosted media is what I'd prefer. I don't see much value in including links to the various media files in the atom feed. How do you feel about this?

Yep, I agree. It gets a bit messy if you start including all the attachments. That might be one for a separate ticket with regard to supporting podcasts (and videocasts). Will amend to only include the thumbnail.

comment:20 Changed 4 years ago by Ben Sturmfels

As discussed I've whittled this patch down to include only the thumbnail image, not adding the enclosure links to all available media types. I've updated the following branch and will also attach a patch.

Diff:
https://gitlab.com/BenSturmfels/mediagoblin/compare/master...328-atom-thumbnails

Check out:
git fetch ​https://gitlab.com/BenSturmfels/mediagoblin.git 328-atom-thumbnails
git checkout FETCH_HEAD

Changed 4 years ago by Ben Sturmfels

comment:21 Changed 4 years ago by ayleph

Thanks, I like the look of that much better. I ran into an issue when testing that I didn't catch before though. If a site has the blog plugin enabled, this patch causes the atom feed to crash mediagoblin because thumb isn't a member of file_urls. I know that the blog plugin isn't production ready, but I don't want to introduce a regression that affects people who are using it (myself included). Here's my proposed work around.

diff --git a/mediagoblin/listings/views.py b/mediagoblin/listings/views.py
index 0b8006d..181486c 100644
--- a/mediagoblin/listings/views.py
+++ b/mediagoblin/listings/views.py
@@ -105,8 +105,11 @@ def atom_feed(request):
     for entry in cursor:
         # Include a thumbnail image in content.
         file_urls = get_media_file_paths(entry.media_files, request.urlgen)
-        content = '<img src="{thumb}" alt='' /> {desc}'.format(
-            thumb=file_urls['thumb'], desc=entry.description_html)
+        if 'thumb' in file_urls:
+            content = '<img src="{thumb}" alt='' /> {desc}'.format(
+                thumb=file_urls['thumb'], desc=entry.description_html)
+        else:
+            content = entry.description_html
 
         feed.add(entry.get('title'),
             content,

Does that look okay to you? If so, I'll add this small change to the patch and push to master. Otherwise, let me know if you have a better way to handle this.

comment:22 Changed 4 years ago by Ben Sturmfels

Thanks Ayleph, looks good to me, please go ahead. :)

comment:23 Changed 4 years ago by ayleph

Resolution: fixed
Status: reviewclosed

Thanks! Pushed to 65d8ca7.

Note: See TracTickets for help on using tickets.