oscarotero / Embed

Get info from any web service or page
MIT License
2.09k stars 310 forks source link

Add Twitter pattern without subdomains #362

Open rsanzante opened 4 years ago

rsanzante commented 4 years ago

I have a strange issue. I'm using Embed URL, a Drupal module that requires another Drupal module, Embed, that uses this library.

The URL Embed module is supposed to allow embedding content using oEmbed. It works ok with several providers but not for Twitter's tweets. After a debugging sessions, it seems the URL is not detected properly because Embed\Providers\OEmbed\Twitter::getPatterns() returns just 'https?://*.twitter.com/*'. This pattern doesn't match the standard tweet URL like https://twitter.com/USER/status/TWEET_ID.

The module works if I add a new pattern: 'https?://twitter.com/*'.

However, if you go to the test URL (https://oscarotero.com/embed/demo/index.php) and type a tweet URL it seems to work. For example:

https://oscarotero.com/embed/demo/index.php?url=https%3A%2F%2Ftwitter.com%2Fmetadrop%2Fstatus%2F1267734586525659138

This is the tweet I was having problems to display.

So, I don't understand what is happening. It seems tweets should not be displayed because match fails, as it happening in my Durpal installation, but the test URL indeed shows the tweet. If I 'fix' the patterns, the Drupal module works ok. I'm crating a PR, but I'm not really sure If this a proper fix.

rsanzante commented 4 years ago

Ok, it seems it may be ok because the code in v3 branch is similar to the fix: https://github.com/oscarotero/Embed/blob/v3.x/src/Providers/OEmbed/Twitter.php

class Twitter extends EndPoint implements EndPointInterface
{
    protected static $pattern = ['*.twitter.com/*', 'twitter.com/*'];
    protected static $endPoint = 'https://publish.twitter.com/oembed';
}
oscarotero commented 4 years ago

Is there any plans to upgrade embed to, at least, v3 sometime? (v4 would be amazing) All issues I have for v2 are from that drupal module :)

Seems like the url_embed module has not been updated in years.

rsanzante commented 4 years ago

I don't know but I can try to move this forward.

Thanks!