From 4445c1868c5ff63123a7c136171a706d6df29859 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 9 Jun 2018 23:39:27 -0700 Subject: [PATCH] Fix header image filtering and YouTube header video detection * Make sure that any get_header_image_tag filters from theme get applied. Fixes quality issue in Twenty Seventeen. * Fix detection of YouTube short URLs. * Dequeue wp-custom-header instead of deregistering to prevent Query Monitor complaining. --- includes/class-amp-theme-support.php | 66 +++++++++++--------------- tests/test-class-amp-theme-support.php | 8 ++-- 2 files changed, 31 insertions(+), 43 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 20174c7c062..d33e362945e 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -369,7 +369,10 @@ public static function add_hooks() { add_action( 'comment_form', array( __CLASS__, 'amend_comment_form' ), 100 ); remove_action( 'comment_form', 'wp_comment_form_unfiltered_html_nonce' ); add_filter( 'wp_kses_allowed_html', array( __CLASS__, 'whitelist_layout_in_wp_kses_allowed_html' ), 10 ); - add_filter( 'get_header_image_tag', array( __CLASS__, 'conditionally_output_header' ), 10, 3 ); + add_filter( 'get_header_image_tag', array( __CLASS__, 'amend_header_image_with_video_header' ), PHP_INT_MAX ); + add_action( 'wp_print_footer_scripts', function() { + wp_dequeue_script( 'wp-custom-header' ); + }, 0 ); add_action( 'wp_enqueue_scripts', function() { wp_dequeue_script( 'comment-reply' ); // Handled largely by AMP_Comments_Sanitizer and *reply* methods in this class. } ); @@ -1356,46 +1359,25 @@ public static function enqueue_assets() { * Conditionally replace the header image markup with a header video or image. * * This is JS-driven in Core themes like Twenty Sixteen and Twenty Seventeen. - * So in order for the header video to display, - * this replaces the markup of the header image. - * - * @since 1.0 - * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 - * @param string $html The image markup to filter. - * @param array $header The header config array. - * @param array $atts The image markup attributes. - * @return string $html Filtered markup. - */ - public static function conditionally_output_header( $html, $header, $atts ) { - unset( $header ); - if ( ! is_header_video_active() ) { - return $html; - }; - - if ( ! has_header_video() ) { - return AMP_HTML_Utils::build_tag( 'amp-img', $atts ); - } - - return self::output_header_video( $atts ); - } - - /** - * Replace the header image markup with a header video. + * So in order for the header video to display, this replaces the markup of the header image. * * @since 1.0 * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L78 * - * @param array $atts The header tag attributes array. + * @param string $image_markup The image markup to filter. * @return string $html Filtered markup. */ - public static function output_header_video( $atts ) { - // Remove the script for video. - wp_deregister_script( 'wp-custom-header' ); - $video_settings = get_header_video_settings(); + public static function amend_header_image_with_video_header( $image_markup ) { + // If there is no video, just pass the image through. + if ( ! has_header_video() || ! is_header_video_active() ) { + return $image_markup; + }; + + $video_settings = get_header_video_settings(); $parsed_url = wp_parse_url( $video_settings['videoUrl'] ); - $query = isset( $parsed_url['query'] ) ? wp_parse_args( $parsed_url['query'] ) : null; + $query = isset( $parsed_url['query'] ) ? wp_parse_args( $parsed_url['query'] ) : array(); $video_attributes = array( 'media' => '(min-width: ' . $video_settings['minWidth'] . 'px)', 'width' => $video_settings['width'], @@ -1405,17 +1387,23 @@ public static function output_header_video( $atts ) { 'id' => 'wp-custom-header-video', ); - // Create image banner to stay behind the video. - $image_header = AMP_HTML_Utils::build_tag( 'amp-img', $atts ); + $youtube_id = null; + if ( isset( $parsed_url['host'] ) && preg_match( '/(^|\.)(youtube\.com|youtu\.be)$/', $parsed_url['host'] ) ) { + if ( 'youtu.be' === $parsed_url['host'] && ! empty( $parsed_url['path'] ) ) { + $youtube_id = trim( $parsed_url['path'], '/' ); + } elseif ( isset( $query['v'] ) ) { + $youtube_id = $query['v']; + } + } // If the video URL is for YouTube, return an element. - if ( isset( $parsed_url['host'], $query['v'] ) && ( false !== strpos( $parsed_url['host'], 'youtube' ) ) ) { - $video_header = AMP_HTML_Utils::build_tag( + if ( ! empty( $youtube_id ) ) { + $video_markup = AMP_HTML_Utils::build_tag( 'amp-youtube', array_merge( $video_attributes, array( - 'data-videoid' => $query['v'], + '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. @@ -1423,7 +1411,7 @@ public static function output_header_video( $atts ) { ) ); } else { - $video_header = AMP_HTML_Utils::build_tag( + $video_markup = AMP_HTML_Utils::build_tag( 'amp-video', array_merge( $video_attributes, @@ -1434,6 +1422,6 @@ public static function output_header_video( $atts ) { ); } - return $image_header . $video_header; + return $image_markup . $video_markup; } } diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 577f3645cbe..11c63d12452 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -386,7 +386,7 @@ public function test_add_hooks() { $this->assertEquals( 10, has_filter( 'cancel_comment_reply_link', array( self::TESTED_CLASS, 'filter_cancel_comment_reply_link' ) ) ); $this->assertEquals( 100, has_action( 'comment_form', array( self::TESTED_CLASS, 'amend_comment_form' ) ) ); $this->assertFalse( has_action( 'comment_form', 'wp_comment_form_unfiltered_html_nonce' ) ); - $this->assertEquals( 10, has_filter( 'get_header_image_tag', array( self::TESTED_CLASS, 'conditionally_output_header' ) ) ); + $this->assertEquals( PHP_INT_MAX, has_filter( 'get_header_image_tag', array( self::TESTED_CLASS, 'amend_header_image_with_video_header' ) ) ); } /** @@ -1338,7 +1338,7 @@ public function test_whitelist_layout_in_wp_kses_allowed_html() { /** * Test AMP_Theme_Support::conditionally_output_header(). * - * @see AMP_Theme_Support::conditionally_output_header() + * @see AMP_Theme_Support::amend_header_image_with_video_header() */ public function conditionally_output_header() { $mock_image = ''; @@ -1346,7 +1346,7 @@ public function conditionally_output_header() { // If there's no theme support for 'custom-header', the callback should simply return the image. $this->assertEquals( $mock_image, - AMP_Theme_Support::conditionally_output_header( $mock_image ) + AMP_Theme_Support::amend_header_image_with_video_header( $mock_image ) ); // If theme support is present, but there isn't a header video selected, the callback should again return the image. @@ -1358,7 +1358,7 @@ public function conditionally_output_header() { set_theme_mod( 'external_header_video', 'https://www.youtube.com/watch?v=a8NScvBhVnc' ); $this->assertEquals( '', - AMP_Theme_Support::conditionally_output_header( $mock_image ) + AMP_Theme_Support::amend_header_image_with_video_header( $mock_image ) ); } }