wp_video_shortcode always adds controls="controls"

[paulschreiber]

wp_video_shortcode always adds controls="controls" to the <video> tags it generates.

The user should be able to pass in a value to $attr to disable output of controls="controls".

[subrataemfluence]

[video src="https://www.youtube.com/watch?v=NhheiPTdZCw" controls="false"]

The patch checks if an attribute called "controls" is set in $attr with value "false" in order to prevent output of controls="controls" attribute in the markup.

Otherwise it will add above attribute to the markup.

[psiico]

Applied your patch to wp-includes/media.php and the same problem keeps arising. Whether it's a self-hosted video or YouTube or Vimeo.

Actually, when using YT video, it seems that the YT iframe is contained inside the video (mejs-container) obj container which shows the controls from YT and mejs overlapped.

Replying to subrataemfluence:

[video src="https://www.youtube.com/watch?v=NhheiPTdZCw" controls="false"]

The patch checks if an attribute called "controls" is set in $attr with value "false" in order to prevent output of controls="controls" attribute in the markup.

Otherwise it will add above attribute to the markup.

[subrataemfluence]

@psiico First of all thank you so much for testing the patch. When we use wp_video_shortcode like this

[video src="https://www.youtube.com/watch?v=NhheiPTdZCw"]

by default WordPress adds the controls attribute as per the function in media.php (if I have understood right) without the patch implemented. The patch worked for me for the above video URL in both occasions, i.e. by passing controls="false" and not adding this attribute at all. Anything else, e.g. controls="true" adds the attribute in markup.

This is how the markup gets generated (with / without the attribute). wp_video_shortcode does not actually render a frame rather it creates a HTML5 markup.

When I pass controls="true", i.e.

[video src="https://www.youtube.com/watch?v=NhheiPTdZCw" controls="true"]

the rendered output is like this:

<video class="wp-video-shortcode" id="video-802-1" width="640" height="360" preload="metadata" controls="controls"><source type="video/youtube" src="https://www.youtube.com/watch?v=NhheiPTdZCw&#038;_=1" /><a href="https://www.youtube.com/watch?v=NhheiPTdZCw">https://www.youtube.com/watch?v=NhheiPTdZCw</a></video>

and when I pass controls="false", i.e.

[video src="https://www.youtube.com/watch?v=NhheiPTdZCw" controls="false"]

or just

[video src="https://www.youtube.com/watch?v=NhheiPTdZCw"]

the output is:

<video class="wp-video-shortcode" id="video-802-1" width="640" height="360" preload="metadata" ><source type="video/youtube" src="https://www.youtube.com/watch?v=NhheiPTdZCw&#038;_=1" /><a href="https://www.youtube.com/watch?v=NhheiPTdZCw">https://www.youtube.com/watch?v=NhheiPTdZCw</a></video>

In latter two cases the WordPress is not adding controls="controls" in the markup.

Would you please tell me how I can reproduce your way of testing?

[psiico]

@subrataemfluence Yes, of course! Before I walk you though it, I think you should change the controls value from false to off to be in tune with the rest of the parameters.

So, I'm running the trunk version. Default theme and plugins. Edited the hello world post and pasted your code after the default content.
[video src="https://www.youtube.com/watch?v=NhheiPTdZCw" controls="false"]

The hello world post renders as follows:

The generated video html is as follows:

<div id="mep_0" class="mejs-container svg wp-video-shortcode mejs-video" tabindex="0" role="application" aria-label="Video Player" style="width: 524px; height: 294px;"><div class="mejs-inner"><div class="mejs-mediaelement"><iframe class="me-plugin" id="me_youtube_0_container" frameborder="0" allowfullscreen="1" title="YouTube video player" width="524" height="294" src="https://www.youtube.com/embed/NhheiPTdZCw?controls=0&amp;wmode=transparent&amp;enablejsapi=1&amp;origin=http%3A%2F%2Flocalhost&amp;widgetid=1" style="width: 524px; height: 294px;"></iframe><video class="wp-video-shortcode" id="video-1-1" width="525" height="295" preload="metadata" style="width: 100%; height: 100%; display: none;"><source type="video/youtube" src="https://www.youtube.com/watch?v=NhheiPTdZCw&amp;_=1"><a href="https://www.youtube.com/watch?v=NhheiPTdZCw">https://www.youtube.com/watch?v=NhheiPTdZCw</a></video></div><div class="mejs-layers"><div class="mejs-poster mejs-layer" style="display: none; width: 100%; height: 100%;"></div><div class="mejs-overlay mejs-layer" style="display: none; width: 100%; height: 100%;"><div class="mejs-overlay-loading"><span></span></div></div><div class="mejs-overlay mejs-layer" style="display: none; width: 100%; height: 100%;"><div class="mejs-overlay-error"></div></div><div class="mejs-overlay mejs-layer mejs-overlay-play" style="width: 100%; height: 100%;"><div class="mejs-overlay-button"></div></div></div><div class="mejs-controls" style="display: block;"><div class="mejs-button mejs-playpause-button mejs-play"><button type="button" aria-controls="mep_0" title="Play" aria-label="Play"></button></div><div class="mejs-time mejs-currenttime-container" role="timer" aria-live="off"><span class="mejs-currenttime">00:00</span></div><div class="mejs-time-rail" style="width: 378px;"><span class="mejs-time-total mejs-time-slider" style="width: 368px;" aria-label="Time Slider" aria-valuemin="0" aria-valuemax="322" aria-valuenow="0" aria-valuetext="00:00" role="slider" tabindex="0"><span class="mejs-time-buffering" style="display: none;"></span><span class="mejs-time-loaded" style="width: 0px;"></span><span class="mejs-time-current" style="width: 0px;"></span><span class="mejs-time-handle" style="left: -5px;"></span><span class="mejs-time-float"><span class="mejs-time-float-current">00:00</span><span class="mejs-time-float-corner"></span></span></span></div><div class="mejs-time mejs-duration-container"><span class="mejs-duration">05:22</span></div><div class="mejs-button mejs-volume-button mejs-mute"><button type="button" aria-controls="mep_0" title="Mute" aria-label="Mute"></button><a href="javascript:void(0);" class="mejs-volume-slider" style="display: none;"><span class="mejs-offscreen">Use Up/Down Arrow keys to increase or decrease volume.</span><div class="mejs-volume-total"></div><div class="mejs-volume-current" style="height: 100px; top: 8px;"></div><div class="mejs-volume-handle" style="top: 5px;"></div></a></div><div class="mejs-button mejs-fullscreen-button"><button type="button" aria-controls="mep_0" title="Fullscreen" aria-label="Fullscreen"></button></div></div><div class="mejs-clear"></div></div></div>

[paulschreiber]

@psiico this code isn't in the trunk version. you'll need to manually patch your copy.

[paulschreiber]

@psiico this code isn't in the trunk version. you'll need to manually patch your copy.

[paulschreiber]

I've updated the patch to:
(a) allow more false values (false, off, 0)
(b) use WordPress coding style

[psiico]

Oh yes! You are totally right and I miss that detail. I'm sorry for wasting your time! Nonetheless I applied the patch to a 4.7.4 version I had lying around, fully updated running Twenty Fifteen, no plugins, and I have the same output as I showed on my previous trunk related post. I even tried to render the shortcode both in post and page context and both outputted the same. Having controls="on" or "off" doesn't change in the slightest the code.

[subrataemfluence]

Thank you for modifying the patch. However, it is still adding controls="controls when I don't pass the parameter i.e. simply [video src="https://www.youtube.com/watch?v=NhheiPTdZCw"].

I have made some further changes in the code. It now first checks whether controls is at all set. If yes, it looks for values "on", "true" or "1". If any of these is found controls="controls" gets added to the markup. For everything else (with controls parameter set) the attribute will be ignored while rendering the output markup.

And if the parameter is not set at all, the attribute will never be added to the markup.

Replying to paulschreiber:

I've updated the patch to:
(a) allow more false values (false, off, 0)
(b) use WordPress coding style

[paulschreiber]

Requiring a controls attribute to be present in the shortcode is a bad idea. It will break all existing uses of the shortcode.

Having controls present is a sensible default.

[birgire]

Here's a somewhat related ticket #42718 for the muted attribute for the video shortcode.

[birgire]

Here's a suggestion in 40590-4.diff​ that:

• uses wp_validate_boolean() on the controls attribute of the [video] shortcode, similar as with loop and autoplay attributes. It can validate inputs like these boolean strings: "true", "1", "false", "0" . If it's validated as true then controls="controls" is added to the <video> tag, otherwise it's not added.
• sets the controls shortcode's attribute as true by default.
• updates the existing test_wp_video_shortcode_attributes() test method, with two assertions. If more is needed, we could add new test methods for various input cases of the controls attribute.

This patch only covers adding/removing controls="controls" to the video tag, via the boolean string controls shortcode's attribute. It's another matter how MediaElement.js handles this video tag attribute. I did some quick testing and it looks like more is needed to remove the controls.

[kirasong]

A quick note that toggling video controls is a feature in the new block editor.

[desrosj]

This was discussed in today's Media chat. Here is a summary:

• This is a reasonable change to the video shortcode, even though shortcodes have been unofficially replaced by blocks.
• Adding the attribute is the preferred path forward (not adding a function argument).
• The default for the attribute should definitely remain true to maintain the current default behavior.
• This still needs more testing. @birgire's comment above makes it seem like this is an incomplete solution.

All this considered, let's move this to Future Release until it has a more solid patch everyone is confident in.