Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#31752 closed enhancement (fixed)

Add support for Reddit comment thread embeds?

Reported by: bananastalktome's profile bananastalktome Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.4 Priority: normal
Severity: minor Version: 4.2.1
Component: Embeds Keywords: has-patch
Focuses: Cc:

Description

Reddit has recently enabled embeddable comment threads for public comments (http://www.redditblog.com/2015/03/announcing-embeddable-comment-threads.html), and I wanted to know if it would be a candidate to add to core. They don't have an oEmbed endpoint, so I believe it would need to be added with a custom handler (similar to googlevideo or youtube_embed_url).

Looking for feedback to get a sense of whether this should be added into core as an WordPress embed type or if it should be left to plugin territory.

Attachments (2)

31752.diff (12.2 KB) - added by swissspidy 8 years ago.
31752.2.diff (1.2 KB) - added by swissspidy 8 years ago.

Download all attachments as: .zip

Change History (14)

#1 @bananastalktome
8 years ago

  • Keywords dev-feedback 2nd-opinion added
  • Severity changed from normal to minor

#2 follow-ups: @helen
8 years ago

  • Keywords dev-feedback 2nd-opinion removed
  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed

If it's not exposed via oEmbed, let's leave this to plugins right now.

#3 in reply to: ↑ 2 @bananastalktome
8 years ago

Replying to helen:

If it's not exposed via oEmbed, let's leave this to plugins right now.

Sounds good to me, thanks for the feedback!

#4 in reply to: ↑ 2 @Viper007Bond
8 years ago

Replying to helen:

If it's not exposed via oEmbed, let's leave this to plugins right now.

I've been in contact with Reddit about getting them to implement an oEmbed endpoint for this and they're currently working on it. :)

#5 @Viper007Bond
8 years ago

One of their developers contacted me to let me know that oEmbed support was added today: https://github.com/reddit/reddit/wiki/oEmbed

However for now they only support embedding comments, i.e. a URL that looks like this:

https://www.reddit.com/r/Showerthoughts/comments/2safxv/we_should_start_keeping_giraffes_a_secret_from/cno7zic

Should we use regex to only match those supported URLs? Something like this:

#https?://(www\.)?reddit\.com/r/[^/]+/comments/[^/]+/[^/]+/.*#i

Or should we just send all Reddit URLs to them and let unsupported URLs get back a 404? The advantage of that is when they roll out support for more embed types, we aren't required to update things on our end. The drawback is we'd be making unneeded HTTP requests.

#6 @Viper007Bond
8 years ago

  • Resolution maybelater deleted
  • Status changed from closed to reopened
  • Version set to 4.2.1

#7 @SergeyBiryukov
8 years ago

  • Milestone set to Awaiting Review
  • Type changed from feature request to enhancement

@swissspidy
8 years ago

#8 @swissspidy
8 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to Future Release

31752.diff uses the forwards compatible #https?://(www\.)?reddit\.com/r/[^/]+/comments/[^/]+/.*#i regex to enable oEmbed support for Reddit.

Their oEmbed docs are on GitHub.

The service definitely meets the criteria for new oEmbed providers. Perhaps this is already something for 4.4.

#9 @wonderboymusic
8 years ago

why can't this be #https?://(www\.)?reddit\.com/r/[^/]+/comments/.*#i? the bit at the end in the patch isn't captured

@swissspidy
8 years ago

#10 @swissspidy
8 years ago

31752.2.diff is a new patch with a simpler regex as suggested.

#11 @swissspidy
8 years ago

  • Milestone changed from Future Release to 4.4
  • Owner set to wonderboymusic
  • Status changed from reopened to assigned

#12 @wonderboymusic
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 35356:

oEmbed: add Reddit Comments as a provider

Docs: https://github.com/reddit/reddit/wiki/oEmbed

Props swissspidy.
Fixes #31752.

Note: See TracTickets for help on using tickets.