Feature/oembed support #1984
Feature/oembed support #1984
Conversation
| oembed.embed = function(req, res) { | ||
| var url = req.query.url; | ||
|
|
||
| if (!url || !url.match(/https?:\/\/jsbin.com/)) return res.send(400); |
There was a problem hiding this comment.
jsbin.com shouldn't be hard coded here, I guess we're saying you can't embed the root (or non-bin documents).
But there is a helper to get this. @allouis is digging it out...
There was a problem hiding this comment.
Is this piece of code to check that the url they're requesting is an actual jsbin url?
There was a problem hiding this comment.
@allouis yes, exactly, if there's a helper to validate jsbin URL format would be great 👍
There was a problem hiding this comment.
@malditogeek We haven't got a helper to validate, but to ensure this runs just as well on local installs could you check against app.get('url host') this will return "jsbin.com" or "localhost:3000" etc...
|
This is awesome, thank you. There's a couple of tweaks required (inline comments), but if you can get this changed today/this morning, I'll merge it in today. |
| oembed.embed = function(req, res) { | ||
| var url = req.query.url; | ||
|
|
||
| if (!url || !url.match(config.url.host)) { return res.send(400); } |
There was a problem hiding this comment.
@allouis I've seen that config is being used in the bin.js handler and has a url attribute, looks better?
There was a problem hiding this comment.
The config is actually loaded on to app.settings, and since app is available on every request object, you don't need the require. So the coe is just .match(req.app.settings.url.host) and one less require (which actually does count towards boot time!)
|
Going live shortly! (though, it's 5pm which is past the deadly hour, but WE CAN DO IT!) |
|
Sweet! Thanks for your help guys! |
|
Live. Though, shouldn't there be a meta tag in the header say what the embed url is? |
|
Awesome, dat continuous deployment :D It's optional, it's used for auto-discovery but will add it Cheers! On Tue, Oct 7, 2014 at 6:08 PM, Remy Sharp notifications@github.com wrote:
|
Hi @remy!
This is a first attempt to implement oEmbed support for JSBin. oEmbed allows you to embed rich content without running arbitrary code (unlike the current embed method) and therefore is the prefer method for a lot of people including Gitter, here some more info: http://oembed.com/ and some examples: https://embed.spotify.com/oembed/?url=http://open.spotify.com/track/4fyHcNgv4PizcRMXUGPFQ2
Here's how they would look:
What do you think?