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

Add oEmbed support for mobile URL structures like mobile.twitter.com #4992

Closed
rachelmcr opened this issue Jan 5, 2017 · 2 comments
Closed

Comments

@rachelmcr
Copy link
Member

Expected behavior

I expect our oEmbeds to support the URL structures that these services use in mobile browsers. For example, Twitter uses mobile.twitter.com instead of twitter.com or www.twitter.com in mobile browsers.

Actual behavior

We only support Twitter embeds for twitter.com or www.twitter.com, not mobile.twitter.com:

https://github.com/WordPress/WordPress/blob/5eb2277dce2ee8a388cf86d112c5ae93809479a4/wp-includes/class-oembed.php#L58-L110

Tested on Nexus 9, Android 7.1.1, WPAndroid alpha-32
@maxme
Copy link
Contributor

maxme commented Jan 5, 2017

Looks like twitter supports it:

$ curl https://publish.twitter.com/oembed\?url=https://mobile.twitter.com/maximeb/status/793003939973267456
{"url":"https:\/\/twitter.com\/maximeb\/status\/793003939973267456","author_name":"Maxime","author_url":"https:\/\/twitter.com\/maximeb","html":"\u003Cblockquote class=\"twitter-tweet\"\u003E\u003Cp lang=\"en\" dir=\"ltr\"\u003EGreat post about \u003Ca href=\"https:\/\/twitter.com\/hashtag\/bitcoin?src=hash\"\u003E#bitcoin\u003C\/a\u003E and \u003Ca href=\"https:\/\/twitter.com\/hashtag\/ethereum?src=hash\"\u003E#ethereum\u003C\/a\u003E mining differences and how they affect hard forks \u003Ca href=\"https:\/\/t.co\/Qi1jGNNClA\"\u003Ehttps:\/\/t.co\/Qi1jGNNClA\u003C\/a\u003E \u003Ca href=\"https:\/\/t.co\/hchwRVRZ1m\"\u003Epic.twitter.com\/hchwRVRZ1m\u003C\/a\u003E\u003C\/p\u003E— Maxime (@maximeb) \u003Ca href=\"https:\/\/twitter.com\/maximeb\/status\/793003939973267456\"\u003EOctober 31, 2016\u003C\/a\u003E\u003C\/blockquote\u003E\n\u003Cscript async src=\"\/\/platform.twitter.com\/widgets.js\" charset=\"utf-8\"\u003E\u003C\/script\u003E","width":550,"height":null,"type":"rich","cache_age":"3153600000","provider_name":"Twitter","provider_url":"https:\/\/twitter.com","version":"1.0"}

But this must be fixed in WordPress before we can add the exception in the app.

@rachelmcr
Copy link
Member Author

Ah, makes sense. I have a note for myself to test other oEmbed services to see if there are other similar URL structures that should be supported, and I'll open a core ticket to suggest a fix there first.

I'll close this issue for now, and we can reopen when WordPress supports it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants