Customizer: current header image does not show when it has the same name as another header image

[sirbrillig]

I noticed that during heavy testing (this probably won't affect most people) of the Customizer's Header Image Controls, occasionally after cropping an image, saving the change, and reloading the Customizer, the current header image would display "no image set", despite there actually being an image.

The reason for this is twofold. One is that the Backbone View that displays the current image has no fallback if it can't find the current header image in its "Previously Uploaded" array (or the defaults). I've created a patch to address that here: #31742.

But the main reason for this bug is that the "Previously Uploaded" array can sometimes not include certain images. This is because the get_uploaded_header_images function in https://core.trac.wordpress.org/browser/trunk/src/wp-includes/theme.php#L1181 returns the list of images as an associative array keyed by the *basename* of the image.

This is a problem because multiple images uploaded on different days can have the same name, and in my testing this happened a lot when I was cropping images several times. In most cases this is no problem because the image paths are different, and the image attachment IDs are different. Here, however, it means that in the Customizer you'll only see one image if there are several with the same filename and if the current image is not in that list, it will display as though no image is set.

Again, this is a relatively hidden bug, but I think it's probably a good idea if it is fixed sooner rather than later in case more code uses get_uploaded_header_images and expects it to be keyed by filename.

[sirbrillig]

Modifying get_uploaded_header_images to use the attachment ID as the array key successfully fixes the issue in the Customizer, but using the header image basename as the key is done in several other places that rely on that function. Let's examine them.

1. Custom_Image_Header->show_header_selector

This outputs HTML of available headers, using the keys of the array returned by get_uploaded_header_images as the radio button values, which are then used to save the header image elsewhere (WP_Customize_Header_Image_Setting->update which then calls Custom_Image_Header->set_header_image). This should be fine if we switch to using the attachment ID as the key because set_header_image uses the same data (see below).

2. Custom_Image_Header->set_header_image

...which reads in its description that it accepts as its parameter:

the key of an image uploaded for that theme (the basename of the URL).

And indeed it compares the parameter to the keys of the array returned by get_uploaded_header_images. This would work fine if the keys were changed to use the attachment ID, I think, as long as the docs were changed to reflect that, because this is called only by WP_Customize_Header_Image_Setting->update which gets its values from get_uploaded_header_images as well.

3. Custom_Image_Header->get_uploaded_header_images

This also returns the results of get_uploaded_header_images (although it is quite happy to return them with different keys). The results of that function then get bootstrapped into the Javascript of the Customizer as _wpCustomizeHeader.uploads, which is used to construct the model api.HeaderTool.ChoiceList. There, the key (still currently the header image basename) is used as the attribute header.defaultName and is used for saving, but only if there is no attachment ID, so it should not be affected by changing the keys of attachments to their ID.

Since I don't immediately see a downside, I'll attach a patch below that makes this change.

[sirbrillig]

To be clear on how to reproduce this bug:

Upload two images with the same name on two different months (since images are organized by YYYY/MM) and then open the Customizer's Header Image Control. You would expect to see both images, but only one will be shown.

For example, if one image has the URL 2015/02/my-image.png and a second has the URL 2015/03/my-image.png only the second image will be shown.

[DrewAPicture]

Seems to have been introduced in 3.4.

[ocean90]

Replying to sirbrillig:

This is because the get_uploaded_header_images function in https://core.trac.wordpress.org/browser/trunk/src/wp-includes/theme.php#L1181 returns the list of images as an associative array keyed by the *basename* of the image.

Introduced in [17757], changed in [19815].

[ocean90]

In 32091:

Customize Headers: Improve handling of the initial header model.

Headers are currently keyed by the basename of the image, see #31786. When two images have the same key only one image will be listed and the current image can be empty in the header control.
To prevent this we now fall back to the raw current header image if the image isn't in _wpCustomizeHeader.uploads.

props sirbrillig.
fixes #31742.

[ocean90]

Are there any plugins and themes which are using get_uploaded_header_images()? If so, will the change break them? This needs to be investigated first.

[sirbrillig]

Are there any plugins and themes which are using get_uploaded_header_images()? If so, will the change break them?

Hm... I'm not sure how best to find out. Is there an easy way to search the wp.org plugin/theme repos for a function call?

Alternatively, would it be better to do something like add a new function that organizes the headers by ID and deprecate the old function? That's probably the safer method, I'd guess, but I'm not sure what sort of precedents may have been set around this sort of thing in the past.

[ocean90]

In 36539:

Themes: Use the attachment ID as the key in get_uploaded_header_images().

Prevents missing header images when an image has the same name as another header image.

Props sirbrillig.
Fixes #31786.