Skip to content

Fix header image filtering and YouTube header video detection #1208

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.

Sign up for GitHub

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

Merged
westonruter merged 2 commits into from
Jun 12, 2018
Merged

Fix header image filtering and YouTube header video detection #1208

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4445c18
Fix header image filtering and YouTube header video detection
westonruter Jun 10, 2018
97620b9
Fix test_amend_header_image_with_video_header
westonruter Jun 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 27 additions & 39 deletions includes/class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Copy link
Copy Markdown
Contributor

@kienstra kienstra Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice how this allows all other filters to run first.

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.
} );
Expand Down Expand Up @@ -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'],
Expand All @@ -1405,25 +1387,31 @@ 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'] ) ) {
Copy link
Copy Markdown
Contributor

@kienstra kienstra Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice how this accounts for URLs like https://youtu.be/v5zg_Yv048s

$youtube_id = trim( $parsed_url['path'], '/' );
} elseif ( isset( $query['v'] ) ) {
$youtube_id = $query['v'];
}
}

// If the video URL is for YouTube, return an <amp-youtube> 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.
)
)
);
} else {
$video_header = AMP_HTML_Utils::build_tag(
$video_markup = AMP_HTML_Utils::build_tag(
'amp-video',
array_merge(
$video_attributes,
Expand All @@ -1434,6 +1422,6 @@ public static function output_header_video( $atts ) {
);
}

return $image_header . $video_header;
return $image_markup . $video_markup;
}
}
8 changes: 4 additions & 4 deletions tests/test-class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' ) ) );
}

/**
Expand Down Expand Up @@ -1338,15 +1338,15 @@ 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() {
Copy link
Copy Markdown
Contributor

@kienstra kienstra Jun 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an error I made in a commit, but this method conditionally_output_header() should have been test_conditionally_output_header().

But with the method renamed, this method should probably be test_amend_header_image_with_video_header().

Copy link
Copy Markdown
Member Author

@westonruter westonruter Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noting that. Fixed in 97620b9.

$mock_image = '<img src="https://example.com/flower.jpeg">';

// 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.
Expand All @@ -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-youtube width="0" height="0" layout="responsive" autoplay id="wp-custom-header-video" data-videoid="a8NScvBhVnc" data-param-rel="0" data-param-showinfo="0"></amp-youtube>',
AMP_Theme_Support::conditionally_output_header( $mock_image )
AMP_Theme_Support::amend_header_image_with_video_header( $mock_image )
);
}
}