has_header_video() should be filtered

[wpweaver]

function has_header_video() is totally dependent on having a video URL set by the Header Media menu.

The returned value of the function should be filtered. It is easily possible to use 'header_video_settings' to replace the 'videoUrl' setting. But the fact that has_header_video() is used various places by the header video code, it makes it difficult to supply an alternate video if there is no get_theme_mod( 'external_header_video' ) set.

It is perfectly reasonable to want to use the Header Video script with 'header_video_settings', but having has_video_header() fail makes it really hard to do. If the result was filtered, then the plugin/theme could provide an alternative videoURL even when the 'external_header_video' has not been set.

All of the other critical elements have filters, but not having one for has_header_video() makes all of the other filters somewhat incomplete. The filter should obviously be named 'has_header_video'.

[celloexpressions]

Makes sense; moving to 4.7.2 for investigation since this in introduced in 4.7. Are you able to create a patch to add the filter @wpweaver?

[sanket.parmar]

Adding a has_header_video filter.

[wpweaver]

Sorry, the fix would be totally trivial, but I'm not really familiar enough with the system to provide patches.

If I understand correctly, the has-patch tag means someone else did it. Thank you.

[SergeyBiryukov]

Maybe get_header_video_url() should be filterable instead?

[SergeyBiryukov]

But the fact that has_header_video() is used various places by the header video code, it makes it difficult to supply an alternate video if there is no get_theme_mod( 'external_header_video' ) set.

FWIW, get_theme_mod() is filterable, so it should be possible to set an alternative video URL using theme_mod_external_header_video filter.

[celloexpressions]

Replying to SergeyBiryukov:

But the fact that has_header_video() is used various places by the header video code, it makes it difficult to supply an alternate video if there is no get_theme_mod( 'external_header_video' ) set.

FWIW, get_theme_mod() is filterable, so it should be possible to set an alternative video URL using theme_mod_external_header_video filter.

While it's possible to filter from the get_theme_mod, I think it makes sense to introduce another filter here. However, I actually agree that a filter on get_header_video_url() would be better than on has_header_video, since that offers the most flexibility.

[wpweaver]

I only read code as deep as has_header_video. Filtering get_header_video_url could work, but the logic would be a bit more complicated. The current source of the url is either an internal attachment ID or a url to an external link. Currently, either one of these could generate the ultimate url, but the filter needs to be able to provide a url even if the theme_mod values are null.

The logic of just where to apply the filter, and on which elements is fairly non-trivial in get_header_video_url, but certainly solvable. How to do it in has_header_video is simple.

But putting the filter in get_header_video_url would also make the_header_video_url work as expected, so the filter really does belong on get_header_video_url, but it will require new logic for the attachment vs external video source to make a filter work.

Maybe like this?

function get_header_video_url() {
	$id = absint( get_theme_mod( 'header_video' ) );
	$url = esc_url( get_theme_mod( 'external_header_video' ) );

	//if ( ! $id && ! $url ) {
	//	return false;
	//}

	if ( $id ) {
		// Get the file URL from the attachment ID.
		$url = wp_get_attachment_url( $id );
	}
	$url = apply_filters('get_header_video_url', $url);
	if ( ! $url )
		return false;
	return esc_url_raw( set_url_scheme( $url ) );
}

[celloexpressions]

Filter get_header_video_url().

[celloexpressions]

I think rearranging the conditional placement after the filter should work. Does 39512.2.diff​ look right?

[SergeyBiryukov]

In 40045:

Customize: Introduce get_header_video_url filter for the return value of get_header_video_url().

Props sanket.parmar, celloexpressions.
Fixes #39512.

[SergeyBiryukov]

Reopening for 4.7.3 consideration.

I used @since 4.8.0 in [40045], just in case it doesn't make it into 4.7.3 (we haven't had enhancements in minor releases since 3.0 I think, but that's going to change now).

If it does make it into 4.7.3, we can change the @since entry in trunk as well.

[celloexpressions]

This seems like an obvious 4.7.x candidate to me since it adds a filter on a function added in 4.7 that should have been filtered from the beginning.

[dd32]

In 40086:

Customize: Update the introduced version in the docs for the get_header_video_url filter to 4.7.3.

See #39512.

[dd32]

In 40087:

Customize: Introduce get_header_video_url filter for the return value of get_header_video_url().

Props sanket.parmar, celloexpressions, SergeyBiryukov.
Merges [40045], [40086] to the 4.7 branch.
Fixes #39512.