Closed Bug 834965 Opened 11 years ago Closed 11 years ago

Embedded Youtube videos in iframes do not play in web content

Categories

(Firefox OS Graveyard :: Gaia::Video, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::Video
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 unaffected, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 verified)

Status:
VERIFIED FIXED
Milestone:
B2G C4 (2jan on)
Project Flags:
blocking-b2g tef+
Tracking Flags:
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- unaffected
b2g18 --- verified
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- verified

People

(Reporter: Harald, Assigned: fabrice)

References

Details

(Keywords: regression)

Attachments

(1 file)

Replace NS_ERROR_NOT_IMPLEMENTED with nullptr
693 bytes, patch
jst
: review+
Details | Diff | Splinter Review 
Using the recommended youtube iframe to embed videos shows the black box with a play button. After clicking play, controls are visible but neither video or audio play.

Example iframe embed:
<iframe src="http://www.youtube.com/embed/eRsGyueVLvQ" width="320" height="215" frameborder="0" allowfullscreen></iframe>

Expected behavior: play either inline or launch video app (like the m.youtube.com does). This affects all apps with youtube videos, they currently have no way of embedded youtube videos other than launching them via Web Activity (with some custom url signature for the youtube case).
The video app is supposed to launch with the content specified. If it isn't, then this is bad bug.
blocking-b2g: --- → tef?
Component: General → Gaia::Video
QA Contact: mozillamarcia.knous
The triage call with this was that this is nice to have but can't be a blocker for v1 so I'm moving the nom to shira.

Dale, feel free to re-assign if you're not the best owner here.
Assignee: nobody → dale
blocking-b2g: tef? → shira?
Harald,

Could you explain what you mean by "recommended youtube iframe"?  Where is that recommendation and are there alternatives?  When I click on the link in firefox, I get the source I get seems to be a player that is supposed to work with flash, html5 or quicktime.  Are there other, more html5-friendly or mobile-friendly ways of embedding a youtube video on a website?
(In reply to David Flanagan [:djf] from comment #3)
> Harald,
> 
> Could you explain what you mean by "recommended youtube iframe"?  Where is
> that recommendation and are there alternatives?  When I click on the link in
> firefox, I get the source I get seems to be a player that is supposed to
> work with flash, html5 or quicktime.  Are there other, more html5-friendly
> or mobile-friendly ways of embedding a youtube video on a website?

I created a test case with the 2 ways to include HTML5-friendly youtube embeds [1]. It's suffice to say that it does not enforce HTML5 playback, but allows the player to choose flexibly.

The code for iframe is similar to what can be found in their documentation [2]. The example also includes their JS driven iframe, which also opens a message channel to control playback.

[1]: https://developers.google.com/youtube/iframe_api_reference
[2]: https://developers.google.com/youtube/iframe_api_reference#Loading_a_Video_Player
(In reply to Andrew Overholt [:overholt] from comment #2)
> The triage call with this was that this is nice to have but can't be a
> blocker for v1 so I'm moving the nom to shira.
> 
> Dale, feel free to re-assign if you're not the best owner here.

Nice to have? No. Absolutely no. No, no, no. That iframe shown above is the typical way a site decides to embed a youtube video. Playing in the app is a nice to have, but not playing at all is a blocker. Renom.
blocking-b2g: shira? → tef?
Summary: Embedded Youtube videos don't play in apps or browser → Embedded Youtube videos in iframes do not play in web content
Okay, I confirmed the behavior described by Harald. The controls appear, but nothing happens. For v1, we're supposed to firing up a web activity to the video app, but we aren't doing that.
I'd like to remind triage that sites embedding youtube videos is a very, very, very common use case on the web. Incredibly common in fact. In fact, youtube encourages heavily. Youtube is a very high up top site and top provider of web content. And something tells me we'll have at least one (if not more) top BD apps that will have embedded youtube videos.
Need info on Karen to find out what top BD apps (Preinstalled would be an example) are affected by the fact if we don't support embedded youtube videos.
Flags: needinfo?(kward)
Similar question to Tom as shown in comment 8. How bad of an impact are we going to take if embedded youtube videos don't work at all?
Flags: needinfo?(telin)
(In reply to Jason Smith [:jsmith] from comment #9)
> Similar question to Tom as shown in comment 8. How bad of an impact are we
> going to take if embedded youtube videos don't work at all?

I am working with at least 5 partners with youtube problems and currently try to get all of them to use the MozActivity with type: video/youtube. It's not a "pretty" solution (due to the vnd.youtube URL format) and I doubt that all of them will be able to implement it!
(In reply to Harald Kirschner from comment #10)
> (In reply to Jason Smith [:jsmith] from comment #9)
> > Similar question to Tom as shown in comment 8. How bad of an impact are we
> > going to take if embedded youtube videos don't work at all?
> 
> I am working with at least 5 partners with youtube problems and currently
> try to get all of them to use the MozActivity with type: video/youtube. It's
> not a "pretty" solution (due to the vnd.youtube URL format) and I doubt that
> all of them will be able to implement it!

That's not the way we should be evangelizing to partners. They shouldn't even be required to do this - the MozActivity work should automatically work with any embedded youtube content. Which is the point of this bug.
blocking-b2g: tef? → tef+
(In reply to Harald Kirschner from comment #10)
> (In reply to Jason Smith [:jsmith] from comment #9)
> > Similar question to Tom as shown in comment 8. How bad of an impact are we
> > going to take if embedded youtube videos don't work at all?
> 
> I am working with at least 5 partners with youtube problems and currently
> try to get all of them to use the MozActivity with type: video/youtube. It's
> not a "pretty" solution (due to the vnd.youtube URL format) and I doubt that
> all of them will be able to implement it!

I agree with Jason. The "video/youtube" thing is an internal hack that we should not have to expose to partners.

When we visit m.youtube.com and tap on a video, something that I do not understand happens in gecko (or in b2g chrome code maybe) that causes the video/youtube activity with the weird vnd.youtube url to happen. Ideally what we want is that when we view partner's websites, the same magic will happen in gecko and the activity will just be launched.

We need to get someone cc'ed on this bug that understands the magic that is happening in gecko.
Flags: needinfo?(telin)
Flags: needinfo?(kward)
Dale says that the firefoxOS browser sends a different user-agent string to m.youtube.com than it does to other sites.  When we visit m.youtube.com we masquerade as an android device, and get the vnd.youtube:// URL, which is an android thing. Then we have a special content handler and that video/youtube activity to handle the android URL.

But when a youtube video is embedded in some other website, we're not sending the android user agent string, so maybe youtube is sending us a different player than what we get on m.youtube.com.
Oh boy. A user agent problem. This is what happens when we use overrides to workaround problems. Can we do an override on embedded content?
Fabrice says (cc'ing him) that the UA overrides are in gaia/build/ua-override-prefs.js

If we have a test site with a youtube embed, it would be interesting to add that site to the overrides, so we send the android UA for that site.  Then see if that magically makes the embedded video play correctly.

If so, then this bug could be resolved as an evangelism issue by getting youtube to recognize the FirefoxOS UA string and treat us like android.  Fabrice says we have asked them to do this but that they have not.

But if setting the UA string for the site with the embedded player does not solve the issue, then we've got to dig in and debug their player on our device.
Evangelism is definitely the better way to solve this, but I think what this bug intends to figure out is a way to work-around the problem.
Marking status-b2g18 and status-b2g18-v1.0.0 as affected, please update the status to fixed once this is verified landed on v1-train/mozilla-b2g18 and v1.0.0/mozilla-b2g18_v_1_0_0
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → affected
AIUI, we will be sending the override to the content requested inside the <iframe> (overrides are for all requests, not just top-level ones), but if some JS is running in the main page context and doing UA detection, they will of course find the B2G UA.

Gerv
(In reply to David Flanagan [:djf] from comment #13)
> But when a youtube video is embedded in some other website, we're not
> sending the android user agent string, so maybe youtube is sending us a
> different player than what we get on m.youtube.com.

Perhaps we need to add an override to the top level domain youtube.com.

(In reply to David Flanagan [:djf] from comment #15)
> Fabrice says (cc'ing him) that the UA overrides are in
> gaia/build/ua-override-prefs.js

This is correct and adding an override takes all of 5 minutes. However, we need to ensure that an override fixes the problem before it is added. Here's a script that will add an override to B2G at runtime for testing. https://gist.github.com/4291503

> If so, then this bug could be resolved as an evangelism issue by getting
> youtube to recognize the FirefoxOS UA string and treat us like android. 
> Fabrice says we have asked them to do this but that they have not.

We have a number of requests out to Google wrt Firefox OS and Firefox for Android. AFAIK, BD is driving this work. (cc Ron)
+FF Android people - in case they have any input here

Brought this up in our compat meeting, but a proposal we have for YouTube is the following:

* On FF OS user agent, we tell YouTube to serve us a HTML 5 mobile site that when viewing a video, shows us h264 content

* On FF Android user agent (tablet & mobile), we tell YouTube to serve us a HTML 5 mobile site that when viewing a video, we jump out to the YouTube android app through the vnd protocol. 

If we get them to follow in suit with what's stated above, we can then rip out the user agent override and remove the need for the hack on FF OS.
I would much rather we just did the same thing on Android and FF OS. We need to address kicking things out to third party apps on Android better, but this hardcoded hack is not that way, and I don't want further churn with YouTube when we do put the correct pieces in.
(In reply to Wesley Johnston (:wesj) from comment #21)
> I would much rather we just did the same thing on Android and FF OS. We need
> to address kicking things out to third party apps on Android better, but
> this hardcoded hack is not that way, and I don't want further churn with
> YouTube when we do put the correct pieces in.

I would prefer that as well, but the constraint preventing us from doing that is the fact h264 support isn't implemented across all FF Android supported devices. We can't rely on kicking out to a player on FF OS without introducing the hack we already have in place.
Brad/Mark/Wes - Back in June '12 we added a hack to Firefox for Android to serve the older Fennec UA to YouTube. Was that hack removed? What UA does Firefox for Android currently send to YouTube?
That hack is not removed (http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#2643); in-fact it was just tweaked a day ago in bug 837215 to adjust for the 'tablet' token removal as the hack was failing.
The UA override for youtube.com was added a while ago

Iframe embedded youtube links work in the mozilla-central build but not mozilla-b2g18_v1_0_1-unagi/

rather confusing but at least there is a working version, attempting to bisect now
Just noting, if I go back to Jan 6th builds the situation is the same, mc works and b2g18 doesnt
For reference, there's significant divergence between the two trees in relevant files. On m-c we're load UserAgentOverride in preload.js, while in b2g18 we're watching for remote-browser-frame-shown in ProcessGlobal and loading the UAO_child.js frame script in response.
That change was uplifted, so the code should be the same.
And yes, I checked the actual hg.mozilla.org/releases/mozilla-b2g_v1_0_1 tree as well.
Except I apparently did not, since https://hg.mozilla.org/releases/mozilla-b2g18/file/b08b8af6b73e/dom/ipc/preload.js shows that it's present. I don't even know what I'm looking at any more.
The v1.0.1 tree/branch isn't live yet.  b2g18 is still 1.0.1.
Dale, what's going on here ?
Flags: needinfo?(dale)
This works on central but doesnt on b2g18, trying to figure out patch makes it work
Flags: needinfo?(dale)
This remains a blocker from the Mozilla perspective. Adding qawanted to help Dale out with comment 34.
Keywords: qawanted
While working to find the regression window, I tested the nightly builds and observed the issue reproduced all the way back to the Dec 12th build.
Keywords: qawanted
Sorry it isnt a regression window we need, it is when it started working on mozilla-central that is needed, it likely never worked on b2g18 so it isnt really a regression. I have just finished up my last patch so doing to just debug it instead of trying to bisect which is virtually impossible as far as I can tell.
Using the central build I was able to locate the build the videos started working properly. The videos issue appear to have been fixed between the builds of February 4th and 5th.
(In reply to ahubenya from comment #38)
> Using the central build I was able to locate the build the videos started
> working properly. The videos issue appear to have been fixed between the
> builds of February 4th and 5th.

Awesome find. Thanks for the help. Let me find the relevant changeset...
Keywords: regressionwindow-wanted
Ok I have no idea what I was getting wrong while attempting to bisect this before but a quick test and that looks good to me.

There are a few media related commits so will take look into them, thanks a lot that was a huge help
My attempt at building around that commit and narrowing it down to single commits hasnt helped much

Further confused by the fact that this build on the 3rd also works for me, anyone else confirm? (I am using junk.arandomurl.com/test.html to test)

https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/nightly/mozilla-central-unagi/2013/02/2013-02-03-03-10-05/
And back to square one

https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/nightly/mozilla-central-unagi/2012/12/2012-12-14-03-04-08/ 

Testing right now works for me, the code for the UA override is the same but I am going to double check for certain that the data that goes back and forward is the same.

If that is so then will have to look into what actually happens when youtube loads the video
So a not very useful update

This reproduces on b2g desktop, which makes life a little easier.

I still havent been able to bisect this, half the builds I checkout are broken, they take about an hour to build each, and as far as I go back when I do get working builds they have the same behaviour (working in mc, broken in b2g18) when I go far enough back various other bugs prevent me from being able to test it.

I have ensured that it is not a UA issue, I patched UserAgentsOverrides.jsm to send the same UA as mc for every request (and verified the network traffic) it doesnt make a difference.

I havent been able to find a way to disable iframe security so I can inspect the dom that the youtube iframe generates, that may help, both versions send the same html5 player.js and css, the fact that the UI is different for each case seems like it should point to where something is going wrong, but what I dont know

I will run debug build on each and see if that helps any, but at this point I am fairly stuck
New lesson, when debugging something use debugging builds

JavaScript error: http://s.ytimg.com/yts/jsbin/html5player-vfllMCQWM.js, line 153: NS_ERROR_NOT_IMPLEMENTED: Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIDOMWindow.crypto]

Only happens on v1.0.1, window.crypto patches are just landing, but they wouldnt have affected previous mc builds so I think something landed on central that changes the way we handle undefined access to crypto at least.

This should be much easier to bisect
and confirmed with http://junk.arandomurl.com/crypto.html which passes on mc and fails on b2g18, will definitely be able to bisect in the morning
In that case it's almost certainly bug 803964. You should also keep in mind bug 683262 which landed yesterday, and bug 673432 which is likely landing in the next couple days.
Actually, you can disregard bug 673432, as it doesn't make a difference.
Yup https://bugzilla.mozilla.org/show_bug.cgi?id=803964 is it exactly, Fabrice suggested testing a simple switch from NS_ERROR_NOT_IMPLEMENTED to a null pointer, this works great, and videos now play in content.

I would suggest that fabrices patch gets applied to b2g18 as its obviously very limited in scope and less risky than trying to rebase and backport the previous patch
Checking with fabrice, should we land this on b2g18 or backport the fix that landed on central?
Attachment #714905 - Flags: feedback?(fabrice)
If DOM Peers are ok with the nullptr patch, let's do that.
Attachment #714905 - Flags: review?(jst)
Attachment #714905 - Flags: review?(jonas)
Attachment #714905 - Flags: feedback?(fabrice)
We have two options here, either land this and make window.crypto return null, or take the fix for bug 683262 alone, which should leave b2g with a non-null window.crypto object that has no methods on it. The latter would be better if someone is doing browser detection based on window.crypto evaluating to true, but that seems pretty far stretched. Given that, I'm tempted to rake this patch for b2g18.
Attachment #714905 - Flags: review?(jst) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
This needs to be uplifted to v1.01.
status-b2g18: affected → fixed
status-b2g18-v1.0.0: affected → wontfix
The issue appears to be fixed on Unagi device
Build ID: 20130313070202
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/e74dafa6b2d9
Gaia: b34e726147f8e671ad8c538b50900ccfbffcb084

User is able to play embedded Youtube videos.
Verified fixed.
Status: RESOLVED → VERIFIED
Also issue is fixed on Unagi build ID: 20130326070204
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/28b048ffb7a7
Gaia: ace1eb32a313da1232bbdf9cff2581a4b036356d
status-b2g18: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: