TryGhost / Ghost

Independent technology for modern publishing, memberships, subscriptions and newsletters.
https://ghost.org
MIT License
47.46k stars 10.35k forks source link

Support Bluesky oEmbed auto-discovery #20028

Closed bnewbold closed 4 months ago

bnewbold commented 6 months ago

Issue Summary

Bluesky (https://bsky.app) recently added post embeds, including oEmbed discovery.

For the later, it looks like Ghost does not allow the <script> tag to be auto-included, probably as a safety/security feature.

From a quick check, it looks like Ghost uses @extractus/oembed-extractor to parse out oEmbed, and that package doesn't seem to work with the bluesky oEmbed discovery: https://extractor-demos.pages.dev/oembed-extractor

It looks like they synchronize with the "official" provider list at: https://oembed.com/providers.json

I opened an issue to get us on that provider list (https://github.com/iamcal/oembed/pull/743).

I'm opening this issue in Ghost to confirm that this is the correct way to get supported in Ghost, and to track interoperability with Ghost specifically.

For what it is worth, the source code for our embed widget script, the embed.bsky.app service, and the Bluesky Social app itself are all open source: https://github.com/bluesky-social/social-app

Thanks for all you do maintaining Ghost!

Steps to Reproduce

Paste a bsky.app post link. Expect a fully-functional post (with JS/iframe). Instead get either a "card", or the raw embed <blockquote> without javascript re-write.

Ghost Version

hosted

Node.js Version

hosted

How did you install Ghost?

hosted

Database type

MySQL 5.7

Browser & OS version

N/A

Relevant log / error output

No response

Code of Conduct

kevinansfield commented 6 months ago

Bluesky oembed should already be supported via the <link rel="alternate" type="application/json+oembed" href="..."> tag.

However looking at the oembed data returned it's missing a height property so it isn't seen as a valid oembed response. According to the oembed spec (See "2.3.4.4. The rich type") both width and height are required fields for the rich type.

bnewbold commented 6 months ago

Hi @kevinansfield, thanks for the fast reply!

Our intention is to set height to null, which is what Twitter does (https://developer.twitter.com/en/docs/twitter-for-websites/oembed-api#item1). We don't have a way to pre-compute height of an embed ahead of time, so we can't set an accurate height value.

kevinansfield commented 6 months ago

Ok, taking a look at the code it seems we don't have validation steps for "known" providers handled by the oembed-extractor library which is why Twitter works. I think we should at least be able to drop the height requirement here when checking that the rel="alternate" derived data is valid - we don't actually use the height anywhere, the check was just protection against potentially reading/storing/exposing data from non-oembed resources.

bnewbold commented 6 months ago

Great, thank you!

We have height: null shipped out in prod now, FWIW: https://embed.bsky.app/oembed?format=json&url=at%3A%2F%2Fdid%3Aplc%3Az72i7hdynmk6r22z27h6tvur%2Fapp.bsky.feed.post%2F3kq7ezofqak2f