WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#29126 closed enhancement (fixed)

Add Kickstarter oEmbed support

Reported by: johnbillion Owned by:
Milestone: 4.2 Priority: normal
Severity: minor Version:
Component: Embeds Keywords: has-patch
Focuses: Cc:
PR Number:

Description (last modified by johnbillion)

I've not reviewed their implementation yet, but Kickstarter has an oEmbed endpoint for embedding a project's video. I think it's a sound candidate for inclusion.

oEmbed docs: https://make.wordpress.org/core/handbook/design-decisions/#whitelisting-oembed-providers

Example oEmbed call: https://www.kickstarter.com/services/oembed?url=https%3A%2F%2Fwww.kickstarter.com%2Fprojects%2F324283889%2Fpotato-salad

Attachments (1)

29126.diff (1.4 KB) - added by wonderboymusic 5 years ago.

Download all attachments as: .zip

Change History (21)

#1 @johnbillion
5 years ago

  • Description modified (diff)

#2 follow-up: @DrewAPicture
5 years ago

Here's some qualifying info based on the whitelisting suggestions:

Service Age Founded 2009
Service Notoriety Well-known
Endpoint notoriety Unknown
API/docs Internal/unavailable
SSL support Yes
Previously proposed? Not that I can find
Plugins No endpoint-specific plugins. See the 'Integrations' list below.
Twitter 880k followers
Facebook 950k likes
Wikipedia Page Established 2010, updated 4 days ago

Integrations:

Of the listed integrations in plugins, the download counts appear to be < 1,000 across the board, indicating some interest but not exactly explosive popularity. This could obviously be due to lack of knowledge of the endpoint's existence.

In short: I vote we hold off on integrating this as maybelater until there is more demonstrated demand.

Version 0, edited 5 years ago by DrewAPicture (next)

#3 @DrewAPicture
5 years ago

I just created the Kickstarter Video Embed plugin that directly adds the endpoint. Lets see if that picks up.

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

Replying to DrewAPicture:

In short: I vote we hold off on integrating this as maybelater until there is more demonstrated demand.

Agreed. I don't see this being that popular. Your plugin easily provides support for this and is a fine solution.

#5 follow-up: @wonderboymusic
5 years ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Awaiting Review to 4.1

Kickstarter is pretty huge, I would argue way more prominent than some of the endpoints we added in 4.0.

Let's discuss inclusion in 4.1.

#6 @nacin
5 years ago

It used to embed a project's video. Now it's a useful card. I'm up for considering it.

#7 in reply to: ↑ 5 @DrewAPicture
5 years ago

  • Keywords 2nd-opinion added

Replying to wonderboymusic:

Kickstarter is pretty huge, I would argue way more prominent than some of the endpoints we added in 4.0.

Let's discuss inclusion in 4.1.

Based on our due diligence in getting information about the endpoint, I don't think it was ever a question of whether the actual service was popular, it definitely seems to be more that people are generally unaware of the endpoint. I mean, if you look at the plugin I created just as a test case for this ticket, it's had a whopping 70 downloads since it was released the first week in August.

So I'm still -1 on adding the provider. It's pretty much up to Kickstarter if they want to promote their endpoint. I don't think it's necessarily core's job to promote it for them, just because it's a popular service. We'd be meeting a demand that doesn't seem to exist.

#8 @studionashvegas
5 years ago

Using the "documentation" on oEmbed.com, I have a concept working on a local site.

Some things I've noticed: As mentioned above, if we use the recommended API endpoint - and if no video has been uploaded to the project - it displays a 220px card instead.

The card's responsive up to 639px. At 640px and above, the project image jumps to 150px tall with the actual picture set as the background image - scaled to 100%.

View https://www.kickstarter.com/projects/324283889/potato-salad/widget/card.html?v=2 as an example.

Video is set to stretch to the width of the screen with the height adjusting proportionally.

As is, I think this may better serve as a shortcode vs a pure oEmbed, unless there's an easy way to 1) pass values through to determine whether to display a card or video and 2) pass through the content width in order to have the video be appropriately sized.

#9 follow-up: @helen
5 years ago

Observations about the endpoint:

  • Does not sanitize maxwidth. Seems to escape it, at least, but it's supposed to be an integer. Bad values set the height to 0.
  • Does not respect maxheight. Appears to ignore it completely. The "rich" type of oEmbed requires that maxwidth and maxheight be obeyed. Not a dealbreaker, but I have an easier time trusting providers who know the spec and follow it.

Would also be curious as to how well it works in responsive themes.

#10 in reply to: ↑ 9 @nacin
5 years ago

Replying to helen:

  • Does not respect maxheight. Appears to ignore it completely. The "rich" type of oEmbed requires that maxwidth and maxheight be obeyed. Not a dealbreaker, but I have an easier time trusting providers who know the spec and follow it.

It's possible this is deliberate. Years ago I recommended to Spotify to ignore it, for example, due to the nature of their tool and embed. But I don't see why they'd ignore it here. I can't remember if Twitter ignores it or not.

#11 @johnbillion
5 years ago

  • Keywords 2nd-opinion removed
  • Milestone changed from 4.1 to Future Release

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


5 years ago

@wonderboymusic
5 years ago

#13 @wonderboymusic
5 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 4.2

Patched

This ticket was mentioned in Slack in #core by drew. View the logs.


5 years ago

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


5 years ago

#17 @wonderboymusic
5 years ago

In 31671:

Add oEmbed support for Kickstarter.

See #29126.

This ticket was mentioned in Slack in #core by drew. View the logs.


5 years ago

#19 @DrewAPicture
5 years ago

@wonderboymusic: Anything left here that would prevent this being closed as fixed?

#20 @DrewAPicture
5 years ago

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

Let's call this fixed. @wonderboymusic, feel free to re-open as necessary.

Note: See TracTickets for help on using tickets.