1237963 - Embedded YouTube video fails to play HTML5 video or Flash: http://tomclancy-thedivision.ubi.com/game/en-US/beta/

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[mkdante381]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160104162232

Steps to reproduce:

Tested on fresh profile Firefox beta, developer and Nightly. No problem on Google Chrome
1. Go to http://tomclancy-thedivision.ubi.com/game/en-US/beta/index.aspx
2. FF have a problem with load YouTube embed movie on this site. This movie is in html5 default, but FF displays This site require plugin

Comment 1

[mkdante381]

[Tracking Requested - why for this release]:

Comment 2

[Paul Pasca[Away. Please needinfo? Paul Oiegas [:pauloiegasSV]]]

I've managed to reproduce this on the latest release(43.0.4) and latest Nightly(46.0a1). On Chrome and IE the video plays as expected. On both Firefox versions, regardless if Flash is enabled or disabled, the page requires to activate a plugin in order to play the video.

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20160105164030

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160113030208

Considering this, I will mark this issue as New and assign the appropriate component.
If anyone considers that the component is not the right one, please change it to a more appropriate one.

Comment 3

[mkdante381]

This is problem with Firefox Core. It has no connection with Shumway. Shumway is only in Nightly and as well as an Add-on. 
Temporary solution:
Install Greasemonkey
Install scripts
https://greasyfork.org/pl/scripts/13174-replace-old-flash-player-based-youtube-embeds-by-their-new-html5-counterparts
and
https://greasyfork.org/pl/scripts/3435-youtube-embed-tweak-html5

Execution order script, 1st is "Replace old Flash Player-based YouTube embeds by their new HTML5 counterparts" and 2nd Youtube Embed Tweak HTML5

Comment 4

[YF (Yang)]

Attachment: testcase

Comment 5

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Chris, would you be able to help me understand the extent of this issue? Do they need to install a Flash plugin to get this video to playback? Thanks!

Comment 6

[Chris Peterson [:cpeterson]]

@ Ritu: this video does not play, even if Flash is installed. I can reproduce this on OS X and Windows 10. The video plays correctly on Chrome, Safari, IE, and Edge. Only Firefox is broken, suggesting this is a Firefox problem.

btw, this bug does not need to be marked as "Security-Sensitive Core Bug", but I don't seem to have Bugzilla permission to unset the "Security-Sensitive Core Bug" flag.

@ Kyle: can you tell why this embedded YouTube video won't play as HTML5 video or Flash? The video is broken even if I fake Chrome's UA, suggesting this is not a UA detection problem in the video player. The <object> element's "movie" param (SWF URL) is odd because it includes a URL parameter "html5=1"?!

The video is located half-way down the page, behind a birthdate block.

<object data-agegate="true" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;" width="640" height="360">
<param name="movie" value="http://www.youtube.com/embed/yPq_NVi-TC4?html5=1&amp;rel=0&amp;hl=en_US&amp;version=3">
<param name="allowFullScreen" value="true">
<param name="allowscriptaccess" value="always">
<embed src="http://www.youtube.com/embed/E2jtOoOKuKs?wmode=transparent?html5=1&amp;rel=0&amp;hl=en_US&amp;version=3" class="youtube-player" type="text/html" allowscriptaccess="always" allowfullscreen="true" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;" width="640" height="360">
</object>

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

The problem here is that we don't normally have eSupportDocuments as a capability for embed tags that aren't youtube rewrites (see bug 769117 for more info on that), so when we get a text/html type, we just go into plugin fallback mode in nsObjectLoadingContent. If I make embed tags support eSupportDocuments, this works for me.

ni?'ing bz on this, since I discussed the special casing of this with him in Bug 769117 Comment 88.

Comment 8

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

BTW, no reason I can see that this bug need to be sec-sensitive, we're just taking the wrong rendering path.

Comment 9

[Liz Henry (:lizzard)]

Just came here to say there is no reason to file this as security-sensitive. 
Tracking for 44+ , while I'm not sure this is a regression, or how widespread it is, we want to support video playback correctly.

Comment 10

[Boris Zbarsky [:bzbarsky]]

<embed> is not supposed to support documents, and doesn't in any browser.  Pointing an <embed> at a URL that returns text/html, as here, is just cargo-culting that won't do anything.

<object> does support documents, but this particular <object> doesn't have a "data" attribute pointing at the thing to load.  Instead it has a <param name="movie"> which per spec doesn't do anything at all.  So per spec, this site is just sending bogus markup.  We've never done anything interesting with this bogus markup, so it's not like it's a regression.  That said, I think more and more people are taking their existing YouTube markup that relied on Chrome loading the <object> due to the <param> and us loading the <embed>, swapping out the Flash URL for the HTML5 one in a cargo-cult way, and ending up in this broken situation.

As for this working in other UAs, note bug 517440 and the spec issue at https://github.com/whatwg/html/issues/387 that I filed about a month ago precisely because Chrome _does_ do something with this <param> and we might need to clone the something.  And perhaps do it somewhat urgently.  I'm quite surprised that the example in comment 6 works in Chrome, because my reading of their code suggested the weird was limited to plug-ins...

At the time I saw IE _not_ doing anything with the param if the <object> didn't have a "type" attribute, but it sounds like at least on the markup from comment 6 it does work, which brings me right back to wanting to have some idea of what other browsers actually do here before we start randomly trying to implement things.

Johnny, who has the cycles to take this on?  If no one else does, I can do it, I guess; if we're going to make changes like this in time for any of the current non-m-c branches we need to move _quickly_.

Comment 11

[Boris Zbarsky [:bzbarsky]]

OK, the <param name="movie"> thing is not in fact relevant here, at least for Chrome.  I just checked, and Chrome is rendering the <embed> in this case, not the <object>.

So I guess the question is what UAs generally do here.  I tried a testcase at http://jsfiddle.net/y0v6q1uz/ which just has:

  <embed type="text/html" src="http://web.mit.edu">

and it looks to me like Safari, Chrome, IE all render it.  In fact they all render http://jsfiddle.net/y0v6q1uz/1/ which just has:

  <embed src="http://web.mit.edu">

So maybe we should just add the document capability to embed and file a bug on the HTML spec to spec this as needed for web compat.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Oh, and if that _does_ make both those testcases work, that seems like a pretty backportable change to me.  Risky, as is anything in object/embed land, but less risky than most things there.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Going to drop the needinfo on jst, because we don't need to deal with the <param> mess here.

Comment 14

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Add documents to embed element capabilities

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8707754 [details] [diff] [review]
Patch 1 (v1) - Add documents to embed element capabilities

r=me though as a followup (to reduce risk for backporting) we could get rid of the concept of eSupportSVG altogether, right?  At this point eSupportDocuments is set any time eSupportSVG is set and the codepaths in nsObjectLoadingContent that handle both are identical.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Oh, and also we could use tests.  Perhaps as simple as <embed> loading a page that runs script sets parent.something, and then checking onload that .something got set.  Again, followup ok.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc2fc3ae3bd2

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bc2fc3ae3bd2

Comment 19

[mkdante381]

Now must be fixed on 44 and 45...

Comment 20

[Boris Zbarsky [:bzbarsky]]

Not sure about "must", but I do think we should request branch uplifts here.

Comment 21

[mkdante381]

Yes must be fixed. FF46 now is only in Nightly channel, but in stable channel 46 version is in 2016-04-19. 3 months for fixed problem? By such errors, people choose another browser... Next bug reported by me in 2015-09-15 13:05 PDT is with steam store and loading recommendations bug 1205027 Not fixed and Why is not fixed? Who tested on Firefox the errors in pages?

Comment 22

[Boris Zbarsky [:bzbarsky]]

> 3 months for fixed problem?

Depends on how big a problem and how risky the fix.

Comment 23

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8707754 [details] [diff] [review]
Patch 1 (v1) - Add documents to embed element capabilities

Approval Request Comment
[Feature/regressing bug #]: None
[User impact if declined]: Document embeds won't work (they work in all other browsers)
[Describe test coverage new/current, TreeHerder]: Tests from bug 769117 partially cover this, as will tests from bug 1239586
[Risks and why]: Changes how an HTML element works, but it's a fairly small change, with tests
[String/UUID change made/needed]: None

Comment 24

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

I'll probably need to provide a new patch if Bug 769117 doesn't also get uplifted.

Comment 25

[Boris Zbarsky [:bzbarsky]]

I'm not sure this patch depends on bug 769117 in any way...  Why would it?

Comment 26

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

The diff here removes the mRewrittenYoutubeEmbed stuff, so it may conflict. That's it.

Comment 29

[mkdante381]

I see new preference enabled in Nightly:
plugins.rewrite_youtube_embeds;true
This preference fix problem on site tom clancys, but not on other sites completely, with default flash youtube embed. Example on http://www.stalker.pl/forum/viewtopic.php?f=9&t=134&start=6276 FF convert this to html5, but window this video have other size. 
Look on solution:
1. Install Greasemonkey

2. Install scripts
https://greasyfork.org/pl/scripts/13174-replace-old-flash-player-based-youtube-embeds-by-their-new-html5-counterparts
and
https://greasyfork.org/pl/scripts/3435-youtube-embed-tweak-html5

3. Execution order script, 1st is "Replace old Flash Player-based YouTube embeds by their new HTML5 counterparts" and 2nd Youtube Embed Tweak HTML5. 

4.In script "Youtube Embed Tweak HTML5" change var videosize to 100%
I changed to default size and settings:
// Set Video Size, with a 16:9 preset, Large (1024x576) medium (720x405) or Set size in percentage, for a video size as a percantage of the container size. 
var videosize = '100%';
// or
//var videosize = '50%';
// nochangeurl must be set to no for player settings to work. yes = default url and no = modified url, size is always modified.
var nochangeurl = 'no';
// Show youtube logo, yes or no
var ytlogo = 'yes';
// Show annoations yes or no. 
var annotation = 'yes';
// Show Related videos at end of playback, yes or no.
var related = 'yes';
// Force https option, yes or no
var ssl = 'yes';

Comment 30

[Boris Zbarsky [:bzbarsky]]

I filed a spec issue for this at https://github.com/whatwg/html/issues/513

Comment 31

[Masatoshi Kimura [:emk]]

(In reply to mkdante381 from comment #29)
> I see new preference enabled in Nightly:
> plugins.rewrite_youtube_embeds;true
> This preference fix problem on site tom clancys, but not on other sites
> completely, with default flash youtube embed. Example on
> http://www.stalker.pl/forum/viewtopic.php?f=9&t=134&start=6276 FF convert
> this to html5, but window this video have other size. 

Please file a new bug to track this.

Comment 32

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8707754 [details] [diff] [review]
Patch 1 (v1) - Add documents to embed element capabilities

Firefox 44 release enters RC mode this week and the update bar continues to be high. We are only taking fixes for critical (recent) regressions, top crashes and sec-crit fixes. I am rejecting the beta uplift for the following reasons:
1. This seems to have been an issue in Fx43 and as such not widespread enough to warrant a dot release. 
2. The high risk associated with changing how an HTML element during RC week with very little time left in the cycle to stabilize.

Comment 33

[mkdante381]

(In reply to Masatoshi Kimura [:emk] from comment #31)
> (In reply to mkdante381 from comment #29)
> > I see new preference enabled in Nightly:
> > plugins.rewrite_youtube_embeds;true
> > This preference fix problem on site tom clancys, but not on other sites
> > completely, with default flash youtube embed. Example on
> > http://www.stalker.pl/forum/viewtopic.php?f=9&t=134&start=6276 FF convert
> > this to html5, but window this video have other size. 
> 
> Please file a new bug to track this.

OK new bug for fixed bug 1240471

Comment 34

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8707754 [details] [diff] [review]
Patch 1 (v1) - Add documents to embed element capabilities

Taking it in 45. Improve the video playback.

Comment 35

[Carsten Book [:Tomcat]]

does not apply to aurora cleanly:

grafting 323720:bc2fc3ae3bd2 "Bug 1237963 - Add documents to embed element capabilities; r=bz"
merging dom/html/HTMLSharedObjectElement.cpp
warning: conflicts while merging dom/html/HTMLSharedObjectElement.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)

Comment 36

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1 AURORA) - Add documents to embed element capabilities; r=bz, a=sledru

Ok here's an aurora version of the patch

Comment 37

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/31eb8acb454f