Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editor: Resolve more oembed URLs (e.g. Facebook posts) #9786

Closed
1 of 2 tasks
kirrg001 opened this issue Aug 15, 2018 · 11 comments · Fixed by #9827
Closed
1 of 2 tasks

Editor: Resolve more oembed URLs (e.g. Facebook posts) #9786

kirrg001 opened this issue Aug 15, 2018 · 11 comments · Fixed by #9827
Assignees
Labels
affects:admin Anything relating to Ghost Admin affects:editor Work relating to the Koenig Editor server / core Issues relating to the server or core of Ghost

Comments

@kirrg001
Copy link
Contributor

kirrg001 commented Aug 15, 2018

There are some embed links which are not supported yet e.g. Facebook posts.

Note: Facebook post embeds were enabled in #9803, we still want to enable the <link rel="alternate"> lookup mentioned below to support other providers that may not be listed on https://oembed.com

Steps to reproduce

  1. Open the Koenig Editor
  2. Click on the card menu "+"
  3. Select Facebook embed
  4. Paste a link to a public Facebook post
  5. See that it errors

Plan

There's a two prong approach to this issue:

  1. Add missing oembed providers to the oembed.com providers list then update the oembed-parser library that Ghost uses
  2. Update our oembed endpoint to fetch the URL and look for a <link rel="alternate" type="application/json+oembed" href="..."> tag when no local lookup is possible
@kirrg001 kirrg001 added affects:admin Anything relating to Ghost Admin later [triage] Things we intend to work but are not immediate priority labels Aug 15, 2018
@kevinansfield kevinansfield changed the title Koenig Editor: Resolve more embed links (e.g. facebook posts, twitter status) Koenig Editor: Resolve more embed links (e.g. facebook posts) Aug 15, 2018
@lunaticmonk
Copy link
Contributor

@kirrg001 I will take a look into it.

@kevinansfield
Copy link
Contributor

@lunaticmonk the opengraph work is already planned. What could be useful is a PR to https://github.com/iamcal/oembed that adds a Facebook (Post) provider

@kevinansfield kevinansfield self-assigned this Aug 16, 2018
@lunaticmonk
Copy link
Contributor

@kevinansfield Gotcha! I will take a look into it and do the needful. Seems the https://github.com/iamcal/oembed has Facebook (Video) and not the Facebook (Post). I will try to add it. Will let you know here when done.

@lunaticmonk
Copy link
Contributor

@kevinansfield What all providers do we need? I am making PR for the Facebook (Post) provider. Should I also include Facebook (Photo) provider?

@kevinansfield
Copy link
Contributor

@lunaticmonk Facebook only has post and video oembed endpoints, there's no separate one for photos - https://developers.facebook.com/docs/plugins/oembed-endpoints/

@lunaticmonk
Copy link
Contributor

@kevinansfield Created a pull request here: iamcal/oembed#305

@kevinansfield
Copy link
Contributor

Thanks @lunaticmonk 👍 I've added a comment over on the PR, let's keep discussion there for now 😄

@kevinansfield kevinansfield added server / core Issues relating to the server or core of Ghost affects:editor Work relating to the Koenig Editor and removed later [triage] Things we intend to work but are not immediate priority labels Aug 17, 2018
@kevinansfield kevinansfield changed the title Koenig Editor: Resolve more embed links (e.g. facebook posts) Editor: Resolve more oembed URLs (e.g. Facebook posts) Aug 17, 2018
kevinansfield added a commit that referenced this issue Aug 20, 2018
refs #9786
- bumped `oembed-parser` dependency to a forked version
  - contains fix for oembed.com providers that include `{format}` in the `url`
  - contains updated `providers.json` file including the `Facebook (Post)` provider (thanks @lunaticmonk)
@lunaticmonk
Copy link
Contributor

@kevinansfield @kirrg001 Can we close this issue if this is resolved?

@kevinansfield
Copy link
Contributor

kevinansfield commented Aug 25, 2018

@lunaticmonk the second part of this issue is still open - we’d like to look for the oembed meta tag if there’s no known provider. This would have avoided the need to manually edit the providers list

@kirrg001 kirrg001 reopened this Aug 25, 2018
@lunaticmonk
Copy link
Contributor

@kevinansfield I understand. Currently, the providers that are added to the providers return a html in the response from the oembed endpoint. Consider a case when no provider is found, do we want to have our own custom card for it and fetch the meta tags from the input url?

@kevinansfield
Copy link
Contributor

@lunaticmonk that’s a different issue :) we already have designs for that. This issue is about loading oembed data for providers that we don’t have explicit oembed endpoints for

@allouis allouis mentioned this issue Aug 27, 2018
3 tasks
kevinansfield pushed a commit that referenced this issue Aug 27, 2018
closes #9786

- Make GET request when url has no provider match
  - The HEAD request was made in order to send less data over the wire when
checking for redirects for urls that do not have an oembed provider
match. We are now going to look for provider metatags withing the
response of the request - rather than making a HEAD followed by a GET if
no redirect is found, this condenses that to a single request.

- Try to get OEmbed data from tag if no provider
  - Here we parse the HTML response of the resource and look for a link tag
that will give us the oembed resource url which we can use to fetch the
embed html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:admin Anything relating to Ghost Admin affects:editor Work relating to the Koenig Editor server / core Issues relating to the server or core of Ghost
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants