Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Embedded youtube videos over https #4743

Merged
merged 1 commit into from Feb 9, 2014

Conversation

Projects
None yet
8 participants
Contributor

arlogn commented Jan 25, 2014

This seems a way to solve #4264. Iframe is forced on https.

Owner

jhass commented Jan 25, 2014

This should ultimately be fixed upstream (ruby-oembed), either by fixing the existing provider or at least by adding a new one. Would be great if you could look into that.

However I'm willing to accept this as a workaround until there's a fixed upstream.

Owner

jhass commented Jan 25, 2014

Oh, and the test suite needs updating :)

https://travis-ci.org/diaspora/diaspora/jobs/17620026#L308

Contributor

arlogn commented Jan 26, 2014

Failed again, have to update more ... tomorrow :)

@Flaburgan Flaburgan referenced this pull request in ruby-oembed/ruby-oembed Jan 26, 2014

Closed

Not working with HTTPS #33

Contributor

Flaburgan commented Jan 26, 2014

There is an issue open about that in the ruby-oembed project: judofyr/ruby-oembed#33
I just commented it, it would be awesome to see that fixed inside the gem itself :)

Contributor

Flaburgan commented Jan 26, 2014

Would adding ?scheme=https to https://github.com/judofyr/ruby-oembed/blob/master/lib/oembed/providers.rb#L139 be enough to solve the problem?

Owner

jhass commented Jan 26, 2014

Probably, not sure if the author will accept (and release) that though.

Contributor

Flaburgan commented Jan 26, 2014

PR submitted, we will see what the repo owner thinks about it.

Contributor

arlogn commented Jan 27, 2014

Well, in the meantime o_emebed_helper_spec fails even if I update the oembed request:

OEmbedHelper o_embed_html for type "secure_video"
Failure/Error: formatted.should =~ /#{data['oembed_data']['html']}/
expected: an object element
got: an anchor element

I'm a ruby noob and I could be wrong but seems to me that the type "secure_video" should have an iframe as html value if the oembed request contains iframe=1. And this had to be even before this workaround. Same for type "secure_rich".
For the same reason I don't understand why now gets an anchor instead an iframe.
Seems wrong also formatted.should =~ /#{data['oembed_data']['url']} for types "unsecure_video/unsecure_rich". Please, take a look.

Contributor

arlogn commented Jan 28, 2014

Thanks, now is more clear. I thought that the html value were to be compared with the same value in the json obtained by the oembed request.

Contributor

Ruxton commented Jan 30, 2014

Can be fixed simply by doing this in config/initializers/oembed.rb

oembed_provider_list = [
  OEmbed::Providers::YouTube
  OEmbed::Providers::Vimeo,
  OEmbed::Providers::SoundCloud,
  OEmbed::Providers::Instagram,
  OEmbed::Providers::Flickr
]

OEmbed::Providers::Youtube.endpoint += "?scheme=https"

As shown in the library ruby-oembed library https://github.com/judofyr/ruby-oembed/blob/master/lib/oembed/providers.rb#L138

Contributor

Ruxton commented Jan 30, 2014

or to go one step further

OEmbed::Providers::Youtube.endpoint += "?scheme=https" if AppConfig.environment.require_ssl?
Contributor

Flaburgan commented Jan 30, 2014

Do we need to test if require_ssl? What's the downside of requesting https content even for an http page?

Contributor

arlogn commented Jan 30, 2014

Thanks @Ruxton. Does not seems there are problems to embed https content inside http pages.

Contributor

Flaburgan commented Jan 30, 2014

No news about the PR I opened in the gem itself, so I'm in favor of merging that inside diaspora*.

Contributor

Flaburgan commented Feb 8, 2014

Is that ready to be merged?

Contributor

arlogn commented Feb 9, 2014

It's ok for me. Works good.

@Raven24 Raven24 merged commit 1563d08 into diaspora:develop Feb 9, 2014

1 check passed

default The Travis CI build passed
Details
Contributor

Flaburgan commented Feb 9, 2014

Thank you!

Contributor

fabianrbz commented Feb 18, 2014

is this still an issue?

Contributor

Flaburgan commented Feb 18, 2014

You can close #4264 ;)

Contributor

polsvoice commented Jun 25, 2014

I'm still having this problem on JoinDiaspora (but not diasp.org). It's the same with Firefox and Chromium. YouTube videos just sit there with a spinning wheel and never load. The console says

[blocked] The page at 'https://joindiaspora.com/stream' was loaded over HTTPS, but ran insecure content from 'http://www.youtube.com/embed/YVCifm2UPC0?feature=oembed&autoplay=1&wmode=opaque': this content should also be loaded over HTTPS.

I used both http:// and https:// for the video URI and got the same result.

Contributor

jaywink commented Jun 25, 2014

Not sure when joindiaspora.com was updated - it's on 0.4.0.1 now but it definitely was not approx 10 hours ago when I checked. Mind trying once more @polsvoice ?

Contributor

polsvoice commented Jun 25, 2014

@jaywink Just checked. It still doesn't work.

Contributor

Flaburgan commented Jun 25, 2014

@polsvoice did you Ctrl + Shift + R to refresh the cache? This should have been fixed in 0.3.0.2. Joindiaspora was using 0.3.0.1 so not surprised that before the upgrade it didn't work.

Contributor

polsvoice commented Jun 25, 2014

@Flaburgan Yeah, I just cleared my cache and tried again. Same result.

Contributor

Flaburgan commented Jun 25, 2014

I just tested with the diaspora HQ account and I'm able to play a youtube video on joindiaspora.
Can you try to clear your cache / cookies, we never know...

Contributor

arlogn commented Jun 25, 2014

@polsvoice can you try to embed a new video? It seems that problem persist only on old embedded videos.

Contributor

polsvoice commented Jun 25, 2014

@arlogn Okay, I tried posting three videos. Two of them were new, and one was the old one that didn't work. I posted them as both http:// and https:// links. The first one worked, with both links. However, the other two (including the old one) still don't work. (I turned off all Chromium extensions, too, in case that was causing a problem.)

Contributor

polsvoice commented Jun 25, 2014

Note that when I switch to my other account on diasp.org, all of the posted videos (except one) play.

Contributor

arlogn commented Jun 26, 2014

@polsvoice can you post the link of videos that don't work? I want to try on my old JD account.

Contributor

Flaburgan commented Jun 27, 2014

It looks like video embedded before the fix doesn't work when those embedded after work. Maybe we need a db task?

Contributor

arlogn commented Jun 27, 2014

Well, I can confirm, the videos posted by @polsvoice work good on my pod and not on JoinDiaspora.
@Flaburgan I tried some videos posted on my pod before the fix and they work good. This seem a problem restricted to JD. I dunno, JD until a few days ago was still at 0.3.0.0. Unlike all other pods they have updated directly to 0.4.0.1. Could this have something to do with the problem?

Contributor

Flaburgan commented Jun 28, 2014

@arlogn yes, because the fix for the embedded videos from youtube was in 0.3.0.2, so your pod was fixed a long time ago when jd was stuck on 0.3.0.1. If you want to test an old video on your pod you have to find one before January.

Contributor

arlogn commented Jun 28, 2014

@Flaburgan you're right, I thought it was past a lot less time. Now I tried some videos embedded seven months ago and in fact they don't work. So the problem is the oembed cache and if I'm not mistaken the only solution is to clear it. It seems risky. In any case the important thing is that the new embedded videos now work also on joindiaspora.

Contributor

polsvoice commented Jul 1, 2014

@arlogn I just checked by re-posting those videos, and they still don't work.

Contributor

arlogn commented Jul 2, 2014

@polsvoice unfortunately, all videos embedded before the fix don't work. The oEmbed responses for these videos have been cached with wrong protocol (http) for the source. If they are re-posted this code is embedded and the browser freezes the content as insecure. How to solve this? Maybe as @Flaburgan says in a comment above, with a db task to clear the cache forcing oEmbed to fetch new embed code.

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