Merge pull request #2642 from ampproject/fix/video-header

Add looping for video headers; extend tree-shaking for amp-video-* class names
ampproject · Jul 16, 2019 · d283e52 · d283e52
2 parents 715ecb2 + 2bdbc10
commit d283e52
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 5 deletions.
diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -2375,6 +2375,7 @@ public static function amend_header_image_with_video_header( $image_markup ) {
 			'height'   => $video_settings['height'],
 			'layout'   => 'responsive',
 			'autoplay' => '',
+			'loop'     => '',
 			'id'       => 'wp-custom-header-video',
 		];
 
@@ -2394,13 +2395,23 @@ public static function amend_header_image_with_video_header( $image_markup ) {
 				array_merge(
 					$video_attributes,
 					[
-						'data-videoid'        => $youtube_id,
-						'data-param-rel'      => '0', // Don't show related videos.
-						'data-param-showinfo' => '0', // Don't show video title at the top.
-						'data-param-controls' => '0', // Don't show video controls.
+						'data-videoid'              => $youtube_id,
+
+						// For documentation on the params, see <https://developers.google.com/youtube/player_parameters>.
+						'data-param-rel'            => '0', // Don't show related videos.
+						'data-param-showinfo'       => '0', // Don't show video title at the top.
+						'data-param-controls'       => '0', // Don't show video controls.
+						'data-param-iv_load_policy' => '3', // Suppress annotations.
+						'data-param-modestbranding' => '1', // Show modest branding.
+						'data-param-playsinline'    => '1', // Prevent fullscreen playback on iOS.
+						'data-param-disablekb'      => '1', // Disable keyboard conttrols.
+						'data-param-fs'             => '0', // Suppress full screen button.
 					]
 				)
 			);
+
+			// Hide equalizer video animation.
+			$video_markup .= '<style>#wp-custom-header-video .amp-video-eq { display:none; }</style>';
 		} else {
 			$video_markup = AMP_HTML_Utils::build_tag(
 				'amp-video',

diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php
@@ -272,6 +272,32 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
 	 */
 	private $selector_mappings = [];
 
+	/**
+	 * Elements in extensions which use the video-manager, and thus the video-autoplay.css.
+	 *
+	 * @var array
+	 */
+	private $video_autoplay_elements = [
+		'amp-3q-player',
+		'amp-brid-player',
+		'amp-brightcove',
+		'amp-dailymotion',
+		'amp-delight-player',
+		'amp-gfycat',
+		'amp-ima-video',
+		'amp-mowplayer',
+		'amp-nexxtv-player',
+		'amp-ooyala-player',
+		'amp-powr-player',
+		'amp-story-auto-ads',
+		'amp-video',
+		'amp-video-iframe',
+		'amp-vimeo',
+		'amp-viqeo-player',
+		'amp-wistia-player',
+		'amp-youtube',
+	];
+
 	/**
 	 * Get error codes that can be raised during parsing of CSS.
 	 *
@@ -510,6 +536,16 @@ private function has_used_class_name( $class_names ) {
 				continue;
 			}
 
+			// Class names for extensions which use the video-manager, and thus video-autoplay.css.
+			if ( 'amp-video-' === substr( $class_name, 0, 10 ) ) {
+				foreach ( $this->video_autoplay_elements as $video_autoplay_element ) {
+					if ( $this->has_used_tag_names( [ $video_autoplay_element ] ) ) {
+						continue 2;
+					}
+				}
+				return false;
+			}
+
 			/*
 			 * Class names for amp-access and amp-access-laterpay.
 			 * See <https://www.ampproject.org/docs/reference/components/amp-access>.

diff --git a/tests/php/test-amp-style-sanitizer.php b/tests/php/test-amp-style-sanitizer.php
@@ -448,6 +448,7 @@ public function get_link_and_style_test_data() {
 					<style> .amp-geo-group-foo { color: peru; } </style>
 					<style> .amp-iso-country-us { color: oldlace; } </style>
 					<style> .non-existent { color: black; } </style>
+					<style> .amp-video-eq { display: none; } </style>
 					</head><body><p>Hello!</p></body></html>
 				',
 				[
@@ -476,6 +477,7 @@ public function get_link_and_style_test_data() {
 					<style> .amp-geo-no-group { color: ghostwhite; } </style>
 					<style> .amp-geo-group-foo { color: peru; } </style>
 					<style> .amp-iso-country-us { color: oldlace; } </style>
+					<style> .amp-video-eq { display: none; } </style>
 					<style> .non-existent { color: black; } </style>
 					</head>
 					<body>
@@ -510,6 +512,7 @@ public function get_link_and_style_test_data() {
 					'.amp-geo-no-group{color:ghostwhite}',
 					'.amp-geo-group-foo{color:peru}',
 					'.amp-iso-country-us{color:oldlace}',
+					'.amp-video-eq{display:none}',
 					'', // Because no non-existent.
 				],
 				[],

diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php
@@ -1970,7 +1970,7 @@ public function test_amend_header_image_with_video_header() {
 		// There's a YouTube URL as the header video.
 		set_theme_mod( 'external_header_video', 'https://www.youtube.com/watch?v=a8NScvBhVnc' );
 		$this->assertEquals(
-			$mock_image . '<amp-youtube media="(min-width: 900px)" width="0" height="0" layout="responsive" autoplay id="wp-custom-header-video" data-videoid="a8NScvBhVnc" data-param-rel="0" data-param-showinfo="0" data-param-controls="0"></amp-youtube>',
+			$mock_image . '<amp-youtube media="(min-width: 900px)" width="0" height="0" layout="responsive" autoplay loop id="wp-custom-header-video" data-videoid="a8NScvBhVnc" data-param-rel="0" data-param-showinfo="0" data-param-controls="0" data-param-iv_load_policy="3" data-param-modestbranding="1" data-param-playsinline="1" data-param-disablekb="1" data-param-fs="0"></amp-youtube><style>#wp-custom-header-video .amp-video-eq { display:none; }</style>',
 			AMP_Theme_Support::amend_header_image_with_video_header( $mock_image )
 		);
 	}