WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 12 months ago

Last modified 9 months ago

#34737 closed enhancement (fixed)

Add Facebook oEmbed support

Reported by: pento Owned by: johnbillion
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Embeds Keywords: has-patch needs-refresh
Focuses: Cc:

Description

Facebook are in the process of releasing their oEmbed support - documentation has been written, it's really just the announcement that needs to be made.

Attachments (2)

34737.diff (1.8 KB) - added by pento 21 months ago.
34737.2.diff (1.9 KB) - added by pento 21 months ago.

Download all attachments as: .zip

Change History (30)

@pento
21 months ago

#1 @pento
21 months ago

  • Keywords has-patch 2nd-opinion added
  • Owner set to pento
  • Status changed from new to assigned

This is currently blocked by #34698.

@wonderboymusic: I know it's a bit late, but assuming that bug is fixed before RC 1, how do you feel about this landing in 4.4?

#2 @pento
21 months ago

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

Let's git r done.

#3 @GaryJ
21 months ago

  • Keywords needs-patch added; has-patch removed

I think those regexes need checking. For instance, I don't think:

https?://www\.facebook\.com/.*/video(s/|.php).*

will match the last two of (from FB documentation):

https://www.facebook.com/{page-name}/videos/{video-id}/
https://www.facebook.com/{username}/videos/{video-id}/
https://www.facebook.com/video.php?id={video-id}
https://www.facebook.com/video.php?v={video-id}

...due to there being an extra / before video. i.e. I think it should be something like:

https?://www\.facebook\.com/.*video(s/|.php).*

although that would need refining further as it would also capture something like http://www.facebook.com/wp-tutorial-videos/

Last edited 21 months ago by GaryJ (previous) (diff)

#4 @swissspidy
21 months ago

Facebook is not exactly known for keeping things in a backwards-compatible way. The Graph API is always changing, the WordPress plugin isn't supported anymore, .... Will the oEmbed endpoint still be available two years from now?

Besides that, Facebook's oEmbed response is not compliant with the specification:

  • There's an additional success property
  • The height property is sometimes null
  • The width property is 100% by default instead of a unit-less number

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

#5 @DrewAPicture
21 months ago

-1 on pushing this in the back door for 4.4. I don't see why this shouldn't go through the normal paces of any other provider we add to the list.

It doesn't seem like the endpoint is well-enough established yet and @swissspidy raises some valid concerns in comment:4.

#6 @helen
21 months ago

  • Milestone changed from 4.4 to Future Release

I am not okay with doing this in 4.4 - oEmbed changes are indeed small, but they still need to be tested, vetted, and established. This is not "established", and all of @swissspidy's points are valid and important.

@pento
21 months ago

#7 @pento
21 months ago

  • Keywords has-patch added; needs-patch removed

Good catch, @GaryJ - 34737.2.diff updates the regex patterns.

@swissspidy makes some good points. Some Automattic folks are working with Facebook on this, they're going to pass this ticket along, so the problems can hopefully be addressed.

#8 @swissspidy
21 months ago

Current status:

  • It seems they don't use width: 100% anymore and instead use a default width of 500 (pixels).
  • height is still null in some cases.

I think we could integrate it once the height bug is resolved. I can live with there being an unneeded success property in the response.

#10 @swissspidy
20 months ago

height is still null in some cases.

To clarify: height is only null in the posts endpoint; videos are fine.

#11 @swissspidy
16 months ago

Still the same problem with the height property for posts.

@pento Do you know if there are still some Automattic folks working with Facebook on embeds? Do Facebook embeds work on WordPress.com?

#12 @pento
14 months ago

I don't think the height thing is an issue. It's contrary to the oembed spec, but Instagram does the same thing, and that doesn't cause problems.

Facebook embeds work correctly on WordPress.com.

#13 @pento
12 months ago

  • Keywords needs-refresh 2nd-opinion added

I think we're at a point where we can only decide if this is going in or not. The URLs that Facebook originally defined haven't changed, so I think we can call it established, or stable.

I doubt they'd be willing to change the height property that they return, their JS handles all of that magic.

The patch needs some updates:

  • the .php parts of the regexes need to be changed to \.php
  • I'm inclined to start adding unit tests for all our trusted oEmbed providers - if their output changes, we should know about it. Building those tests (with Facebook as the first provider) may be out of scope of this ticket, though.

I'm +1 on commit, though I'd like to hear arguments either way.

#14 @swissspidy
12 months ago

Yeah probably for the best to include this in 4.7. It's only a minor issue and shouldn't stop us from improving the experience for users.

There's #32360 for oEmbed unit tests. See also #28507.

Happy to work on both refreshing this patch and adding the tests.

#15 @johnbillion
12 months ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Future Release to 4.7
  • Owner changed from pento to johnbillion
  • Status changed from assigned to reviewing

#16 @johnbillion
12 months ago

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

In 38367:

Embeds: Add support for embedding Facebook posts and videos via oEmbed.

Fixes #34737
Props pento, GaryJ

#17 @johnbillion
12 months ago

In 38368:

Embeds: Add a missing regex delimiter for Facebook URLs.

See #34737

#18 @ocean90
11 months ago

In 38691:

Embeds: Add support for embedding Facebook photos of a user/page.

See #34737.

#19 @josephvb10
10 months ago

For some reason with this Facebook oEmbed is being returned in arabic language. Could this be a problem in Facebook's end (maybe by misidentifying the server's IP)?

#20 @miraclemaker
10 months ago

There's an issue with Facebook single post embeds.

The following url should embed a single post from the Wordpress account on Facebook:

https://www.facebook.com/WordPress/posts/10154412148282911

What I'm actually getting on 4.6.1 is a feed of *all* posts from that account.
You can see the correct behaviour here:

http://embed.ly/code?url=https%3A%2F%2Fwww.facebook.com%2FWordPress%2Fposts%2F10154412148282911

#21 @swissspidy
10 months ago

@miraclemaker This change here is for WordPress 4.7, not 4.6.1. In 4.6.1, auto-discovery will kick in. There, Facebook provides the embed code you're seeing now. So that's expected. If you use the latest nightly build, everything works fine.

#22 @miraclemaker
10 months ago

@swissspidy Got it, thanks for taking the time to clear that up.

#23 @pareshradadiya
10 months ago

$wp_embed->autoembed( $content ) doesn't appears to be working for the Facebook embed

#24 @swissspidy
10 months ago

@pareshradadiya Sounds unlikely. Perhaps no connection could be made to their servers or something. Could you share some code to reproduce it?

#25 @pareshradadiya
10 months ago

@swissspidy 

Add these two links in your post content from the admin dashboard

https://youtu.be/fLoRXpvEWDo
https://www.facebook.com/aheneghana/videos/10153867925655496/

Try to access this post from frontend, you will see the embed for youtube video but not for the facebook video

#26 @swissspidy
10 months ago

@pareshradadiya Embedding the video by pasting the URL works for me. Screenshot: https://cloudup.com/cVXvtICBYvI

#27 @pareshradadiya
9 months ago

@swissspidy

Embedding the video by pasting the URL does not works for me.

Here is a screenshot for reference!

http://prntscr.com/d6d0j6
http://prntscr.com/d6d1pu

#28 @swissspidy
9 months ago

@pareshradadiya That second video works well on my install as well. Which version of WordPress are you using? Do you see any errors in the browser's developer console? What meta data is stored for this post in the database?

Note: See TracTickets for help on using tickets.