get_header_image(): may consider adding filter in 'get_header_image()'

[hztyfoon]

Hey everyone, 🙂

I think we can add a filter hook in 'header_image' function in 'wp-includes/theme.php' file.

Original Idea by: @audrasjb.

Trac ticket: 56180

[audrasjb]

Related: #56158

(see comment 3)

[hztyfoon]

I just thought I should mention my good friend @manzurahammed as he helped me with the patch. :)

[audrasjb]

Looks good to me. I added a commit to your PR to address a few WPCS issues.

[hztyfoon]

Thanks @audrasjb a lot for fixing those typos. 🤗
Looking pretty good to me.

[audrasjb]

Related: #46124

[audrasjb]

Related: #46134

[audrasjb]

Related: #38942

[mukesh27]

Hi there!

Thanks for the ticket and patch.

The approach looks good to me. Can you please add a Unit test that covers the new filter header_image?

[SergeyBiryukov]

It looks like the proposed filter would affect header_image(), but not get_header_image(), which may be confusing.

Should this be a filter on the result of get_header_image() instead? That way it would work for both functions.

See get_header_image_tag and get_header_video_url for similar examples.

[SergeyBiryukov]

I'm also not sure that "Displays" should be changed to "Prints" as part of this PR, it would be inconsistent with other similar descriptions:

 * Displays localized stylesheet link element.
 * Displays the custom header text color in 3- or 6-digit hexadecimal form (minus the hash symbol).
 * Displays the image markup for a custom header image.
 * Displays header image URL.
 * Displays header video URL.
 * Displays background image path.
 * Displays background color value.

For some of these, "Prints" might indeed be more appropriate than "Displays", but I would suggest changing that in a separate ticket.

[audrasjb]

Replying to SergeyBiryukov:

It looks like the proposed filter would affect header_image(), but not get_header_image(), which may be confusing.

Should this be a filter on the result of get_header_image() instead? That way it would work for both functions.

Great point!

[costdev]

I agree on both points @SergeyBiryukov - the filter should be in get_header_image() instead, and the "Displays" > "Prints" change is outside the scope of this ticket and should be removed from the PR.

[costdev]

Add filter to get_header_image(). Includes unit tests.

[costdev]

Testing Instructions

Steps to Test

1. Add the following code to functions.php:

   <?php
   
   add_filter( 'header_image', 'my_custom_header_image' );
   function my_custom_header_image( $header_image ) {
           return 'http://example.org/my-custom-header-image.jpg';
   }
   
   add_action( 'admin_notices', 'my_custom_header_image_notice' );
   function my_custom_header_image_notice() {
           echo '<div class="notice notice-info"><p>'. get_header_image() . '</p></div>';
   }
   

2. Navigate to the Dashboard.

Expected Results

• A notice should appear containing the following: http://example.org/my-custom-header-image.jpg

[hztyfoon]

⬆️ changing the summery from 'header_image' to 'get_header_image'.

Thanks a ton @SergeyBiryukov for those 2 points. 👍
I also agree with both of them. Will try to update the PR accordingly.

Also, thanks @costdev. I've took a look at your .diff & you've also added unit_test there. great work.

Just updated the PR.
removed the filter from header_image & added a filter in get_header_image.

Looks like it may make more sense to add the filter to get_header_image function as this way it'll work for both header_image & get_header_image (as inside header_image() there's only get_header_image() and then just printing the image url ).

Thanks @SergeyBiryukov for pointing this out.

[hztyfoon]

Updated the PR. Would you all be kind enough to check. 🙂

[audrasjb]

Thanks for updating the PR but it looks like it doesn't include the Unit Tests proposed by @costdev in 56180.1.diff. It would be useful if you can update your PR to implement those unit tests, but if you can't, it's probably better to close the PR in favor of 56180.1.diff :)

[hztyfoon]

Ok, closed the PR in favor of 56180.1.diff by @costdev as it already includes unit tests.

Just a minor change suggestion: Wouldn't the hook_name get_header_image make more sense than header_image ?

[audrasjb]

Indeed, so it's more consistent with the other get_header_image_tag_attributes filter.
PR incoming.

Trac ticket: https://core.trac.wordpress.org/ticket/56180

[audrasjb]

Tested using the following snippet, works fine:

add_filter( 'get_header_image', function() {
	return 'https://example.com/image.jpg'; // Replace this with the image of your choice.
} );

Adding needs-dev-note keyword.
Marking for commit.

[audrasjb]

In 53741:

Themes: Add a hook to filter theme header image URL.

This changeset introduces the get_header_image filter, which can be used to modify header image URL returned by get_header_image(), in themes that support the Header Image feature.

Props hztyfoon, audrasjb, mukesh27, SergeyBiryukov, costdev.
Fixes #56180.

Committed in https://core.trac.wordpress.org/changeset/53741