search

ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

Support for Facebook post embed #979

Closed rudygalfi closed 8 years ago

rudygalfi commented 9 years ago

Per publisher request. More info: https://developers.facebook.com/docs/plugins/embedded-posts

Idea would be for it to be similar to amp-twitter or amp-pinterest.

systemaddict commented 8 years ago

Facebook will probably never help develop this. So is there an alternative route?

cramforce commented 8 years ago

@systemaddict I don't actually agree that they'd never do it, but such an embed doesn't necessarily need to be made by Facebook. The same implementation strategy we use for tweets should be fine.

adewale commented 8 years ago

Please prioritise embedded posts: https://developers.facebook.com/docs/plugins/embedded-posts and embedded videos: https://developers.facebook.com/docs/plugins/embedded-video-player since they tend to be essential to articles like: http://www.n-tv.de/wirtschaft/Mark-Zuckerberg-ist-jetzt-ein-besserer-Mensch-article16485106.html and http://www.buzzfeed.com/assmamaad/la-fille-de-mark-zuckerberg-est-une-fan-de-star-wars

cramforce commented 8 years ago

Use case: Support news articles about what Mark Zuckerberg posted on Facebook :)

We should definitely do this and it should be easy enough. Probably similar to amp-twitter, but it is worth investigating whether a pure iframe integration (like instagram) based on whatever postMessage they send for iframe resizing is possible.

asap commented 8 years ago

Here's some examples of a pages where we use Facebook embeds at The Huffington Post:

http://www.huffingtonpost.com/entry/woman-redefines-bikini-body-in-perfectly-honest-facebook-post_us_5693c1f0e4b0a2b6fb70cc29?cps=gravity_2246_-4637688767927315695&kvcommref=mostpopular

http://www.huffingtonpost.com/entry/the-viral-facebook-post-struggling-moms-will-relate-to-instantly_us_569d03b8e4b0b4eb759f1689?cps=gravity_2692_3287072325818751354&kvcommref=mostpopular&

Generally, we grab the url of the post itself and wrap it in some embed code. Ideally, like everyone has mentioned, it should work like amp-twitter or other tags.

mkhatib commented 8 years ago

I have a change that's almost ready. Just fixing up some tests to send with it for a PR.

I went the amp-twitter approach few exceptions. One is that the only way I got FB SDK to render posts correctly when there are multiple amp-facebook elements on the page is to actually load the SDK script in every amp-facebook iframe.

Before that, following amp-twitter approach and only loading it once FB would only render the post that included the SDK correctly and the rest failed to fetch or render at all until I started to manually calling parse FB.XFBML.parse(parentElement) which would successfully fetch the posts but fails to render the post with the correct height and width and instead renders a 0px by 0px posts and throws an error fb-post failed to resize in 45s.

As I mentioned the only way I got this to work and render sizes correctly is by including the sdk and calling FB.init in every amp-facebook iframe.

Another issue that I bumped into early on was the inability to access contentWindow for the FB post iframe to listen to resize event, I am not entirely sure but I think that's the expected behavior for iframes that are loaded from a different domain (In comparison, twitter's iframe doesn't actually have a src).

Luckily FB SDK has an alternative for contentWindow.resize. FB.Event.subscribe allows us to listen to xfbml.resize event that the SDK fires which also sends the height and width of the resize event - I use that to fire the embed-size event to allow the parent iframe to resize itself. The only concern I have here is that FB doesn't seem to document xfbml.resize event anywhere in their documentation, I had to do some debugging to see what kind of events FB is firing. So not sure if this is a safe approach for now.

Over all, FB SDK lack of a twitter-like programatic way to create posts createTweet was a bit painful. And I am hoping once we get FB devs input on this we can improve the implementation by removing the multiple inclusions of the SDK.

I'll send the PR once I get the tests to pass and I can address any concerns needed.

cramforce commented 8 years ago

Thanks @mkhatib. This is now only pending inclusion in validator. Tracked in #1486