xwp / wp-core-media-widgets

Feature plugin for development of new core media widgets for image, video, and audio (and others)
https://wordpress.org/plugins/wp-core-media-widgets/
35 stars 8 forks source link

.mov Video doesn't play on frontend #171

Closed jasmussen closed 7 years ago

jasmussen commented 7 years ago

Meta boilerplate, much ❤️ for this. These widgets are ones I've been longing for for a decade. I'm not kidding.

The video widget works great in the admin:

video works great in admin

But it doesn't actually render on the frontend — or rather, it becomes a link, when I would have expected a video player:

video works great in admin

Here's a snap of the markup:

markup

I've no opinion on what type of video player it should use, whether a fancy bundled JS one, or just the stock HTML5 browser provided one that's used in the admin. I personally think the latter is fine.

timmyc commented 7 years ago

Interesting - there was some conversation in Slack yesterday around .mov files not behaving as expected:

https://wordpress.slack.com/archives/C0381N237/p1494371958914519

Apparently the mejs player doesn't support .mov files: http://stackoverflow.com/questions/14303369/how-to-play-mov-files

Perhaps this is something that could be fixed/covered in #157 - at least ensuring a warning is displayed when .mov files are uploaded.

@jasmussen curious how that file works if embeded in the editor?

jasmussen commented 7 years ago

curious how that file works if embeded in the editor?

It works fine, it's using the stock HTML5 player built into Chrome. Here's what the markup looks like:

<div class="media-widget-preview">

<div style="" class="wp-video">
<video controls="" class="wp-video-shortcode " preload="metadata">

        <source src="http://src.wordpress-develop.dev/wp-content/uploads/2017/05/click-outside.mov" type="">

</video>
</div>

        </div>

👆 that whitespace is in the markup, this is a straight copy from the inspector.

westonruter commented 7 years ago

@jasmussen it works because incidentally the MOV was created with the right combination of Codecs that make it so Chrome can play it. But MOV can be a container for many kinds of Codecs that only QuickTime can play as far as I understand. And ME.js hasn't implemented support via it's Flash fallback, apparently.

In any case, if you try embedding a MOV video into the post editor you get the same problem, where it embeds it as a link and not as a player as I tried yesterday. So I think this is out of scope for the video widget itself and should be addressed in core. Right?

westonruter commented 7 years ago

This is what I see after attempting to insert an MOV in the editor:

image

Here's a video of what I experience: https://cloudup.com/cLPvznPNyws

jasmussen commented 7 years ago

I trust your triaging judgement on this. But it does feel a bit weird that it just works in the admin, but not on the frontend. Is it possible to fall back to the stock HTML 5 player (video tag) when the mejs player can't handle?

jasmussen commented 7 years ago

Had another thought: if we can't make mov files play on the frontend, they should probably not play on the backend either. Having them play there, but not the frontend, sets expectations for the user that aren't met.

westonruter commented 7 years ago

Agreed. We should prevent MOV files from showing in the media browser.

timmyc commented 7 years ago

So I think this is out of scope for the video widget itself and should be addressed in core. Right?

I am thinking this is correct, but since we know this is an issue, and it has been brought up twice now in a day, seems like we could display an error alongside the logic in #157 for when a .mov is uploaded, or remove it from the acceptable extensions.

melchoyce commented 7 years ago

How about:

Sorry, we can't display .MOV files. Please upload an .MP4, .OGG, or [etc.] instead.

westonruter commented 7 years ago

Probably just:

Sorry, we can't display .MOV files.

It would be hard to list out all of the valid extensions since the list is variable and that makes it hard to do with translation.

If we can open the media library with a MIME type that is a comma-separated list of the embeddable video types, then the MOV files wouldn't even show in the list. (Unless the user uploaded it, but that's a separate issue: #128). So then the only place where this error message would appear is on the embed screen where the user supplies a URL to the video.

westonruter commented 7 years ago

Oh, and in that regard, the MOV file URLs are already being blocked in #157:

image

It would be nice if the list of valid video formats could be listed. There must be a way to do this properly and also be friendly to translation.

obenland commented 7 years ago

Something like this?

printf( __( 'Sorry, we can\'t display <code>.mov</code> files. Please upload a file with one of these extensions instead: %1$s' ), '<code>.' . implode( '</code>, <code>.', wp_get_video_extensions() ) . '</code>' );
westonruter commented 7 years ago

I suppose so. I guess we can assume a comma wouldn't have to be localized.