Page MenuHomePhabricator

?embedplayer=yes videos broken (again)
Closed, ResolvedPublic

Description

This seems similar to the resolved bug https://bugzilla.wikimedia.org/show_bug.cgi?id=56405 ("?embedplayer=yes broken for videos with width less than < $wgMinimumVideoPlayerSize (800px)").

Examples:

https://blog.wikimedia.org/2013/11/08/open-letter-free-access-wikipedia-south-africa/ - video is blank

https://blog.wikimedia.org/2013/10/21/scientific-multimedia-files-get-a-second-life-on-wikipedia/ (one of the cases from bug 56405) - all three videos are blank, as is the audio recording at the bottom

https://commons.wikimedia.org/wiki/File%3ACienciaaberta_gt2013_abertura.webm?embedplayer=yes (one of the cases from bug 56405) - video is blank, JavaScript console in Chromium shows the following:

Uncaught TypeError: undefined is not a function File%3ACienciaaberta_gt2013_abertura.webm?embedplayer=yes:45
(anonymous function) File%3ACienciaaberta_gt2013_abertura.webm?embedplayer=yes:45
Uncaught TypeError: Cannot read property 'profile' of undefined load.php?debug=false&lang=en&modules=startup&only=scripts&skin=vector&*:2
(anonymous function) load.php?debug=false&lang=en&modules=startup&only=scripts&skin=vector&*:2
jQuery.extend.each load.php?debug=false&lang=en&modules=jquery%2Cmediawiki%2CSpinner%7Cjquery.triggerQueueCallback%2Cl…:5
jQuery.fn.jQuery.each load.php?debug=false&lang=en&modules=jquery%2Cmediawiki%2CSpinner%7Cjquery.triggerQueueCallback%2Cl…:2
$.fn.embedPlayer load.php?debug=false&lang=en&modules=startup&only=scripts&skin=vector&*:2
(anonymous function) File%3ACienciaaberta_gt2013_abertura.webm?embedplayer=yes:62


Version: unspecified
Severity: normal

Details

Reference
bz66143

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:13 AM
bzimport set Reference to bz66143.
bzimport added a subscriber: Unknown Object (MLST).

Change 137529 had a related patch set uploaded by Brian Wolff:
Fix ?embedplayer=yes mode, which is currently totally broken

https://gerrit.wikimedia.org/r/137529

Ugh. Really need to figure out how to do automated testing of this (selenium?)

  • Bug 66409 has been marked as a duplicate of this bug. ***

Change 137529 merged by jenkins-bot:
Fix ?embedplayer=yes mode, which is currently totally broken

https://gerrit.wikimedia.org/r/137529

This should hopefully start working again come june 24.

This seems to have made things worse, the blog posts are completely unreadable now.

(In reply to Tisza Gergő from comment #6)

This seems to have made things worse, the blog posts are completely
unreadable now.

Oh wow. That's not good.


Hmm, so this doesn't happen to me locally, and I'm not sure what the cause is yet.

However, issue does not happen for videos at beta, possibly due to c469a916dadc82aa which is not on production.

Ok, what's happening is that $wgBreakFrames = true is on production. My last change that used MW's default loader, caused this code to become active for embedplayer=yes

Change 142085 had a related patch set uploaded by Brian Wolff:
Do not break iframes in the iframe output of TMH

https://gerrit.wikimedia.org/r/142085

(In reply to Bawolff (Brian Wolff) from comment #8)

Ok, what's happening is that $wgBreakFrames = true is on production. My last
change that used MW's default loader, caused this code to become active for
embedplayer=yes

However, i should note that on beta wiki, this doesn't seem to happen, and if you use ?debug=true on commons, the frame breaking also doesn't happen (So I guess somehow load order matters?). On my local wiki the frame breaking also doesn't happen (even after setting $wgBreakFrames = true). Which is odd...

Change 142085 merged by jenkins-bot:
Do not break iframes in the iframe output of TMH

https://gerrit.wikimedia.org/r/142085

Change 142132 had a related patch set uploaded by Gergő Tisza:
Do not break iframes in the iframe output of TMH

https://gerrit.wikimedia.org/r/142132

Change 142132 merged by jenkins-bot:
Do not break iframes in the iframe output of TMH

https://gerrit.wikimedia.org/r/142132

Change 142149 had a related patch set uploaded by Gergő Tisza:
Update TimedMediaHandler with embed breakage fixes

https://gerrit.wikimedia.org/r/142149

Change 142149 merged by jenkins-bot:
Update TimedMediaHandler with embed breakage fixes

https://gerrit.wikimedia.org/r/142149

Fix deployed to Commons, seems to work now. Thanks, Bawolff!

(The first video in https://blog.wikimedia.org/2013/11/08/open-letter-free-access-wikipedia-south-africa/ is still messed up, it is probably included in the blog post in a weird way. I suppose that was workaround for the original bug.)

(In reply to Tisza Gergő from comment #16)

Fix deployed to Commons, seems to work now. Thanks, Bawolff!

Great, works for me too now. Thanks a lot everyone, in particular Bawolff!

(The first video in
https://blog.wikimedia.org/2013/11/08/open-letter-free-access-wikipedia-
south-africa/ is still messed up, it is probably included in the blog post
in a weird way. I suppose that was workaround for the original bug.)

Yes, it was changed as a workaround for this bug last week. But that's actually the way we most often use for videos on the blog (e.g. https://blog.wikimedia.org/2014/05/26/happy-birthday-ward-cunningham-inventor-of-the-wiki/ ) - with basically the same HTML as MediaWiki uses on wiki pages, via a conversion script (https://meta.wikimedia.org/wiki/Wikimedia_Blog/Converting_wiki_pages_to_blog_posts ). One advantage is that this allows to select the still image to be displayed, via the thumbtime parameter, which afaik isn't possible with the iframes (is there a bug for that?).

Yes, it was changed as a workaround for this bug last week. But that's
actually the way we most often use for videos on the blog (e.g.
https://blog.wikimedia.org/2014/05/26/happy-birthday-ward-cunningham-
inventor-of-the-wiki/ ) - with basically the same HTML as MediaWiki uses on
wiki pages, via a conversion script
(https://meta.wikimedia.org/wiki/Wikimedia_Blog/
Converting_wiki_pages_to_blog_posts ). One advantage is that this allows to
select the still image to be displayed, via the thumbtime parameter, which
afaik isn't possible with the iframes (is there a bug for that?).

I don't think there's a bug for that, but its been on my mind that there should be a method of specifying the thumbtime via iframe.

Copying the html is fine, however you need to copy it when using a video of size 800px (or full size if the video < 800px). Otherwise you get the html which opens a popup dialog box, which won't work as well when just copying the html.

i.e. If the html looks like <div id="mwe_player_0" style="position:relative;display:inline-block;width:700px;height:394px" videopayload="...

Then copying the html will result in an image with a play button which links to the original file for download.

If the html looks like:

<div class="mediaContainer" style="position:relative;display:block;width:800px"><video...

Then the video will play in browser using native html5 browser support (Assuming you have a browser that supports ogg or webm).

(In reply to Bawolff (Brian Wolff) from comment #18)

I don't think there's a bug for that, but its been on my mind that there
should be a method of specifying the thumbtime via iframe.

There is now: bug 67165

(In reply to Bawolff (Brian Wolff) from comment #18)

Yes, it was changed as a workaround for this bug last week. But that's
actually the way we most often use for videos on the blog (e.g.
https://blog.wikimedia.org/2014/05/26/happy-birthday-ward-cunningham-
inventor-of-the-wiki/ ) - with basically the same HTML as MediaWiki uses on
wiki pages, via a conversion script
(https://meta.wikimedia.org/wiki/Wikimedia_Blog/
Converting_wiki_pages_to_blog_posts ). One advantage is that this allows to
select the still image to be displayed, via the thumbtime parameter, which
afaik isn't possible with the iframes (is there a bug for that?).

I don't think there's a bug for that, but its been on my mind that there
should be a method of specifying the thumbtime via iframe.

Copying the html is fine, however you need to copy it when using a video of
size 800px (or full size if the video < 800px). Otherwise you get the html
which opens a popup dialog box, which won't work as well when just copying
the html.

i.e. If the html looks like <div id="mwe_player_0"
style="position:relative;display:inline-block;width:700px;height:394px"
videopayload="...

Then copying the html will result in an image with a play button which links
to the original file for download.

If the html looks like:

<div class="mediaContainer"
style="position:relative;display:block;width:800px"><video...

Then the video will play in browser using native html5 browser support
(Assuming you have a browser that supports ogg or webm).

Thanks for these very informative comments! I added some of that to https://meta.wikimedia.org/wiki/Wikimedia_Blog/Guidelines/How_to_post#Method_1:_Reusing_the_HTML_that_embeds_a_video_on_wiki_pages .

However, when I tried this out, reusing the <div class="mediaContainer" ... HTML (from this wiki page: https://meta.wikimedia.org/w/index.php?title=Wikimedia_Blog/Drafts/A_Look_Back_at_Wikimania_2013&oldid=9276912 ) in a separate HTML page, it did not work correctly - the box displays nicely and the video starts playing in the browser (Chromium), but only in low quality
(160p, I guess) instead of the highest possible resolution for this div.

This might be more appropriate for a separate bug - in that case, sorry for the offtopic discussion.

However, when I tried this out, reusing the <div class="mediaContainer" ...
HTML (from this wiki page:
https://meta.wikimedia.org/w/index.php?title=Wikimedia_Blog/Drafts/
A_Look_Back_at_Wikimania_2013&oldid=9276912 ) in a separate HTML page, it
did not work correctly - the box displays nicely and the video starts
playing in the browser (Chromium), but only in low quality
(160p, I guess) instead of the highest possible resolution for this div.

This might be more appropriate for a separate bug - in that case, sorry for
the offtopic discussion.

Weird. That appears to be intentional:

// Sort sources by bandwidth least to greatest ( so default selection on resource constrained
// browsers ( without js? ) go with minimal source.
uasort( $mediaSources, 'TimedMediaTransformOutput::sortMediaByBandwidth' );

Seems like a bad design decesion...

This might be more appropriate for a separate bug - in that case, sorry for
the offtopic discussion.

Yeah it is kind of something that should be a separate bug.

I started on I6f3316477175634a to fix the issue. Basically currently we order the versions of the video file smallest bitrate first. My change would be to have it be smallest bitrate first only for versions with a higher resolution than the player size.

Gilles raised the priority of this task from High to Unbreak Now!.Dec 4 2014, 10:11 AM
Gilles moved this task from Untriaged to Done on the Multimedia board.
Gilles lowered the priority of this task from Unbreak Now! to High.Dec 4 2014, 11:21 AM