Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable/hide muted toggle in Video block when autoplay is enabled #4635

Closed
westonruter opened this issue Apr 29, 2020 · 5 comments
Closed

Disable/hide muted toggle in Video block when autoplay is enabled #4635

westonruter opened this issue Apr 29, 2020 · 5 comments
Assignees
Labels
P2 Low priority WS:Core Work stream for Plugin core

Comments

@westonruter
Copy link
Member

Bug Description

As discussed in ampproject/amphtml#28041, when an amp-video has the autoplay attribute the video will forcibly be muted. As such, the muted attribute has no effect and it is deprecated. Nevertheless, the Video block provides toggles for both autoplay and muted:

image

When autoplay is toggled-on, there is actually an warning currently added and yet the Muted toggle is still displayed:

image

This is misleading on sites with the AMP plugin because even though the Muted toggle is not enabled, the video will still be muted.

Expected Behaviour

The Muted toggle should be either hidden, or a notice should be added to indicate that autoplay videos will be muted on AMP pages regardless of the Muted toggle state.

The muted attribute should he then omitted from the amp-video elements created from the Video block.

Steps to reproduce

  1. Add a video block.
  2. Enable the autoplay toggle.

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@westonruter
Copy link
Member Author

The “plays inline” toggle probably doesn't make sense either when “autoplay” is enabled.

@swissspidy
Copy link
Collaborator

Just ran into the muted is deprecated warning for a video with autoplay off.

The plugin's validation tool did not warn me about this deprecation message, so I only learned it through Search Console. It would be great if the plugin would warn me about this.

In ampproject/amphtml#28041 (comment) it was mentioned that it's difficult or even impossible to remove the "Muted" UI control in Gutenberg. If that's still the case, perhaps that can be reported upstream?

@westonruter
Copy link
Member Author

It's been awhile since I looked at this, so I'm confused. You're seeing the muted is deprecated notice when autoplay is off? I'm not seeing that warning in the the validator. I'm not seeing it when autoplay is absent (see playground) or present (playground).

@swissspidy
Copy link
Collaborator

swissspidy commented Aug 9, 2021

Yes. It's absent, to be precise. But in the editor it's "off" like this:

Screenshot 2021-08-09 at 12 51 07

The markup is as follows:

<figure class="wp-block-video"><amp-video width="1920" height="1080" controls="" muted="" poster="https://pascalbirchler.com/wp-content/uploads/2021/08/Web-Stories-Client-Side-Video-Optimization-poster.jpeg" src="https://pascalbirchler.com/wp-content/uploads/2021/08/Web-Stories-Client-Side-Video-Optimization.mp4" layout="responsive" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive"><i-amphtml-sizer style="display:block;padding-top:56.25%"></i-amphtml-sizer><a href="https://pascalbirchler.com/wp-content/uploads/2021/08/Web-Stories-Client-Side-Video-Optimization.mp4" fallback="">https://pascalbirchler.com/wp-content/uploads/2021/08/Web-Stories-Client-Side-Video-Optimization.mp4</a><noscript><video width="1920" height="1080" controls muted poster="https://pascalbirchler.com/wp-content/uploads/2021/08/Web-Stories-Client-Side-Video-Optimization-poster.jpeg" src="https://pascalbirchler.com/wp-content/uploads/2021/08/Web-Stories-Client-Side-Video-Optimization.mp4" playsinline></video></noscript></amp-video><figcaption>Client-side video optimization in the Web Stories editor in action</figcaption></figure>

Search Console complains:

The attribute 'muted' in tag 'video' is deprecated - use 'autoplay' instead.

@westonruter
Copy link
Member Author

Ah, it's specifically flagging the presence of muted on the noscript > video fallback element.

The muted attribute on amp-video itself is not deprecated: https://github.com/ampproject/amphtml/blob/530fc13c795099f03ac043739eac0aa22c7372e6/extensions/amp-video/validator-amp-video.protoascii#L75-L78

It is, however, on amp-video > noscript > video: https://github.com/ampproject/amphtml/blob/530fc13c795099f03ac043739eac0aa22c7372e6/validator/validator-main.protoascii#L1836-L1840

This was done 5 years ago: ampproject/amphtml@7cb88f7#diff-b9292e22ae04c8d88da089f65bb8e7c7a00d53cb971335179b9cfa303752c82a

But the muted attribute on amp-video never seems to have been deprecated in the validator.

Therefore, it seems the quick fix here is just to omit the muted attribute from the noscript > video fallback, as simple as the following:

diff --git a/includes/sanitizers/class-amp-video-sanitizer.php b/includes/sanitizers/class-amp-video-sanitizer.php
index f929566b5..bbf4797cb 100644
--- a/includes/sanitizers/class-amp-video-sanitizer.php
+++ b/includes/sanitizers/class-amp-video-sanitizer.php
@@ -62,6 +62,9 @@ public function sanitize() {
 
 		if ( $this->args['add_noscript_fallback'] ) {
 			$this->initialize_noscript_allowed_attributes( self::$tag );
+
+			// Omit muted from noscript>video since it causes deprecation warnings in validator.
+			unset( $this->noscript_fallback_allowed_attributes['muted'] );
 		}
 
 		for ( $i = $num_nodes - 1; $i >= 0; $i-- ) {

@westonruter westonruter added this to the v2.1.4 milestone Aug 9, 2021
@dhaval-parekh dhaval-parekh self-assigned this Aug 17, 2021
@westonruter westonruter modified the milestones: v2.1.4, v2.2 Aug 30, 2021
@delawski delawski self-assigned this Sep 1, 2021
@westonruter westonruter modified the milestones: v2.2, v2.3 Sep 4, 2021
@westonruter westonruter modified the milestones: v2.3, v2.4 Dec 23, 2021
@westonruter westonruter removed this from the v2.4 milestone Apr 14, 2022
@westonruter westonruter closed this as not planned Won't fix, can't repro, duplicate, stale Nov 30, 2023
@github-project-automation github-project-automation bot moved this to Ready for Review in Ongoing Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Low priority WS:Core Work stream for Plugin core
Projects
Archived in project
6 participants