Jump to content

Better default oEmbed support


Marcher Technologies

Recommended Posts

9 minutes ago, Admonstrator said:

Would be a nice feature but I think we need more control over this. It would be nice if you can configure in ACP what embed provider is allowed or forbidden.

While I agree, I could see the complexities of doing so being large enough, that specifically would probably be saner to leave to third-party modifications. While I can add my specific suggestion as well rather easily, I noticed the fact there is no list of what works and doesn't by default is leading to confusion, which this would relieve, as well as utilizing this provider list allowing additions without updates from IPS, or any kind of mod. As a result I think this would be a good move in the software at default.

Link to comment
Share on other sites

Not every site on that list actually support the implementation of oEmbed that IPS uses - I tried adding iFixit to test it out, but it seems to do things slightly differently, so it would never actually do anything. That might be because I hadn't set it up correctly, but I think at least part of it is that the way embeds are handled in IPS4 is non-standard (though doing it that way has benefits.

However, doing it that way could potentially result in bad things happening - anyone can add their own site to that page by just forking the GitHub repo. If someone added a domain that has drive-by malware, or anything like that, bad things would happen.

Also, there are some security features to prevent people from embedding whatever they like, which wouldn't necessarily be compatible with getting the oEmbed providers from an external service.

Link to comment
Share on other sites

16 minutes ago, Colonel_mortis said:

However, doing it that way could potentially result in bad things happening - anyone can add their own site to that page by just forking the GitHub repo. If someone added a domain that has drive-by malware, or anything like that, bad things would happen.

https://github.com/iamcal/oembed/pulls

It's not like one can just commit a URL and be on that list.... you have to submit a pull request with your changes, and pull requests appear to be vetted by the maintainer of that repo.

16 minutes ago, Colonel_mortis said:

Not every site on that list actually support the implementation of oEmbed that IPS uses - I tried adding iFixit to test it out, but it seems to do things slightly differently, so it would never actually do anything. That might be because I hadn't set it up correctly, but I think at least part of it is that the way embeds are handled in IPS4 is non-standard (though doing it that way has benefits.

Also, there are some security features to prevent people from embedding whatever they like, which wouldn't necessarily be compatible with getting the oEmbed providers from an external service.

The latter is related to the former. Yes, the same list would need be utilized in \IPS\Text\Parser::allowedIFrameBases().

Link to comment
Share on other sites

3 minutes ago, Marcher Technologies said:

https://github.com/iamcal/oembed/pulls

It's not like one can just commit a URL and be on that list.... you have to submit a pull request with your changes, and pull requests appear to be vetted by the maintainer of that repo.

The latter is related to the former. Yes, the same list would need be utilized in \IPS\Text\Parser::allowedIFrameBases().

Still, I would personally rather not just blindly trust that list, though I do agree that it's probably fine. That doesn't change the fact that some endpoints might not work correctly when paired with IPS's implementation, so any links that are posted that are technically supported but don't work would then just break.

The embed domain isn't necessarily the same as the domain for the oEmbed endpoint though (although it is in most cases I think).

I am all for admins being able to add endpoints from within ACP, I just don't necessarily think it should be automatically pulled from an API (especially an API that doesn't support an encrypted connection).

Link to comment
Share on other sites

34 minutes ago, Colonel_mortis said:

That doesn't change the fact that some endpoints might not work correctly when paired with IPS's implementation, so any links that are posted that are technically supported but don't work would then just break.

I would like to know why you think this is fact? IPS' implementation supports the specification of oEmbed to a tee, even unto the rarely used 'link' response type. So long as a given provider follows the oEmbed specification, which would be a requirement to getting a pull request to be on that list accepted, it is designed in such a way it cannot possibly fail...

37 minutes ago, Colonel_mortis said:

The embed domain isn't necessarily the same as the domain for the oEmbed endpoint though (although it is in most cases I think).

I would encourage you to review the list of providers linked, it accounts for this by providing programmatic access to the embed domain/pattern list via the 'schemes' array for each provider, in essence, it is giving one all the data needed.

 

Link to comment
Share on other sites

On 06/01/2016 at 10:35 PM, Marcher Technologies said:

I would like to know why you think this is fact? IPS' implementation supports the specification of oEmbed to a tee, even unto the rarely used 'link' response type. So long as a given provider follows the oEmbed specification, which would be a requirement to getting a pull request to be on that list accepted, it is designed in such a way it cannot possibly fail...

I would encourage you to review the list of providers linked, it accounts for this by providing programmatic access to the embed domain/pattern list via the 'schemes' array for each provider, in essence, it is giving one all the data needed.

 

 

Lets take the TED endpoint as an example. The oembed providers JSON returns

    {
        "provider_name": "Ted",
        "provider_url": "http:\/\/ted.com",
        "endpoints": [
            {
                "schemes": [
                    "http:\/\/ted.com\/talks\/*"
                ],
                "url": "http:\/\/www.ted.com\/talks\/oembed.{format}"
            }
        ]
    }

However, the necessary allowed iFrame base is https://embed-ssl.ted.com, which isn't mentioned in that API response at all. There's no technical reason why it couldn't be on a separate domain all together, though none that I checked were.

In theory though, it would be possible to work around that. However, some of the APIs don't work at all - the iFixit.com endpoint provides a script tag, not an iFrame, and that script 404s. That is their fault, and they should probably be removed from the oEmbed list, but if this change was in place now, anyone pasting any links to iFixit (who are a sponsor of the company that owns the site which I am a dev for) would find that their link is replaced by a blank box (which I had mistaken for it not being compatible with IPS4). While that is absolutely that site's fault, it doesn't change the fact that the list can contain sites that don't function correctly.

Also, IPS's implementation does not support the oEmbed standard quite right - it checks the oEmbed endpoint for any url that matches a domain on their list of oEmbed domains, not just those that match the oEmbed scheme supplied by the site owner where available (in the JSON response, it's inside the endpoints and supplied for every site except YouTube). It doesn't make much practical difference (though iFixit seems to have messed theirs up, which would actually make it break less by not embedding), but it's still not full conformance.

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...