Make WordPress Core

Opened 8 years ago

Last modified 23 hours ago

#46128 new defect (bug)

Custom header "Hide image" button not displayed when randomizing images via theme setup

Reported by: webmandesign Owned by:
Priority: normal Milestone: Awaiting Review
Component: Customize Version: 3.4
Severity: normal Keywords: has-patch has-screenshots has-unit-tests
Cc: Focuses:

Description

DESCRIPTION

When registering multiple custom header images via theme code, and setting them to randomize (by enabling random-default custom header attribute), there is no "Hide image" button displayed. But the image should be displayed even in this case.

The only way to enable "Hide image" button is to set default-image attribute during custom header registration in the theme code. But this actually effectively disables images randomization...


HOW TO TEST/REPRODUCE

Append this code to Twenty Nineteen theme's functions.php file (without opening <?php) to test - see the code comments for more details:

<?php

function test_custom_header_hide_button() {

        /**
         * Registering custom header support.
         *
         * The issue description:
         * ======================
         * When 'default-image' is set, there is "Hide image" button
         * displayed in "Header Image" section of customizer. But now
         * the 'random-default' does not work.
         * On the other side, if 'default-image' is not set, "Hide image"
         * button is not displayed in "Header Image" section of customizer
         * when we have 'random-default' enabled.
         * In both cases the "Hide image" button should be available.
         *
         * How to test:
         * ============
         * Simply (un)comment the 'default-image' to see the difference
         * in "Header Image" section of customizer.
         * But do not save/publish your customizer as we are testing
         * how the theme code is treated in customizer!
         */
        add_theme_support( 'custom-header', array(
                // 'default-image' => 'https://via.placeholder.com/1000x200/000099/ffffff?text=Image+default',
                'random-default' => true,

                // Irrelevant, but required options:
                'height' => 200,
                'width'  => 1000,
        ) );

        // Registering some images to randomize from:
        register_default_headers( array(
                'image-1' => array(
                        'url'           => 'https://via.placeholder.com/1000x200/990000/ffffff?text=Image+1',
                        'thumbnail_url' => 'https://via.placeholder.com/250x50/990000/ffffff?text=Image+1',
                        'description'   => 'Image 1',
                ),
                'image-2' => array(
                        'url'           => 'https://via.placeholder.com/1000x200/009900/ffffff?text=Image+2',
                        'thumbnail_url' => 'https://via.placeholder.com/250x50/009900/ffffff?text=Image+2',
                        'description'   => 'Image 2',
                ),
        ) );

}
add_action( 'after_setup_theme', 'test_custom_header_hide_button' );
// Display the custom header in theme footer for preview:
add_action( 'wp_footer', 'the_custom_header_markup' );

SCREENSHOT

Preview of the issue with custom header registration in customizer

Attachments (2)

custom-header-setup-via-theme.png (23.5 KB ) - added by webmandesign 8 years ago.
Preview of the issue with custom header registration in customizer
46128.diff (1.2 KB ) - added by dlh 7 years ago.

Download all attachments as: .zip

Change History (6)

@webmandesign
8 years ago

Preview of the issue with custom header registration in customizer

@dlh
7 years ago

#1 @dlh
7 years ago

  • Focuses ui removed
  • Keywords has-patch needs-testing added
  • Version 5.0.33.4

Hi @webmandesign, and (belated) thanks for this report.

The custom header API is far from my expertise. But, by my reckoning, the default for the header_image setting needs to be modified so that it returns random-default-image when random header images are going to be used. 46128.diff has that change. Would you be able to test the patch?

From my reading of the code and testing, the patch should respect themes that set random-image to false.

I don't think there needs to be a case where the default is random-uploaded-image, because is_random_header_image() won't return an uploaded image as a fallback, but my unfamiliarity could be misleading me about this point.

#2 @ozgursar
7 months ago

  • Keywords has-screenshots added; needs-testing removed

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/46128/46128.diff

Environment

  • WordPress: 7.0-alpha-61215-src
  • PHP: 8.2.29
  • Server: nginx/1.29.4
  • Database: mysqli (Server: 8.4.7 / Client: mysqlnd 8.2.29)
  • Browser: Chrome 143.0.0.0
  • OS: macOS
  • Theme: Twenty Nineteen 3.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.1

Steps to Reproduce

  1. Activate Twenty Nineteen theme
  2. Add the following code to functions.php
function test_custom_header_hide_button() {

        /**
         * Registering custom header support.
         *
         * The issue description:
         * ======================
         * When 'default-image' is set, there is "Hide image" button
         * displayed in "Header Image" section of customizer. But now
         * the 'random-default' does not work.
         * On the other side, if 'default-image' is not set, "Hide image"
         * button is not displayed in "Header Image" section of customizer
         * when we have 'random-default' enabled.
         * In both cases the "Hide image" button should be available.
         *
         * How to test:
         * ============
         * Simply (un)comment the 'default-image' to see the difference
         * in "Header Image" section of customizer.
         * But do not save/publish your customizer as we are testing
         * how the theme code is treated in customizer!
         */
        add_theme_support( 'custom-header', array(
                //'default-image' => 'https://i.imgur.com/ErERGJz.jpeg',
                'random-default' => true,

                // Irrelevant, but required options:
                'height' => 450,
                'width'  => 1000,
        ) );

        // Registering some images to randomize from:
        register_default_headers( array(
                'image-1' => array(
                        'url'           => 'https://i.imgur.com/pdH4R4c.jpeg',
                        'thumbnail_url' => 'https://i.imgur.com/pP6RGgu.jpeg',
                        'description'   => 'Image 1',
                ),
                'image-2' => array(
                        'url'           => 'https://i.imgur.com/2ZSMKRP.jpeg',
                        'thumbnail_url' => 'https://i.imgur.com/YoEVLuS.jpeg',
                        'description'   => 'Image 2',
                ),
        ) );

}
add_action( 'after_setup_theme', 'test_custom_header_hide_button' );
// Display the custom header in theme footer for preview:
add_action( 'wp_footer', 'the_custom_header_markup' );
  1. View Appearance > Customize > Header Image
  2. When default image is commented out Hide Image button is not visible
  3. Go back to functions.php and remove comment before line
//'default-image' => 'https://i.imgur.com/ErERGJz.jpeg',
  1. View Appearance > Customize > Header Image again
  2. When default image is not commented out Hide Image button is visible

Actual Results

  1. ✅ Issue resolved with patch.
  2. Hide Image button is visible when default image is commented out.

Supplemental Artifacts

Before applying patch:

With default image:
https://i.imgur.com/ggAgdKG.png

Without default image:
https://i.imgur.com/N2cQHDA.png


After applying patch:

With default image:
https://i.imgur.com/sIXyapH.png

Without default image:
https://i.imgur.com/ik1BCl0.png

This ticket was mentioned in PR #12605 on WordPress/wordpress-develop by @arkaprabhachowdhury.


4 weeks ago
#3

  • Keywords has-unit-tests added

When a theme enables random default header images without setting a fixed default image, the Customizer currently treats the header image setting as empty. This hides the Hide image button even though a randomized header is displayed.

This sets the Customizer setting default to random-default-image when the theme enables random default headers, while preserving the existing fixed default behavior otherwise. Regression coverage verifies both cases.

## Testing

  • npm run test:php -- --filter Tests_WP_Customize_Manager::test_header_image_setting_default
  • PHPCS for both changed files
  • PHP syntax checks for both changed files

#4 @arkaprabhachowdhury
23 hours ago

Opened [PR #12605](https://github.com/WordPress/wordpress-develop/pull/12605) to address this ticket. When a theme enables random default header images without setting a fixed default image, the Customizer currently treats the header image setting as empty. This hides the Hide image button even though a randomized header is displayed. This sets the Customizer setting default to random-default-image when the theme enables random default headers, while preserving the existing fixed default behavior otherwise. Regression coverage verifies both cases. Testing - npm run test:php -- --filter Tests_WP_ The PR includes focused regression coverage, and its validation details are recorded in the PR description.

I picked this up because the ticket describes a concrete core behavior or edge case that remains relevant in current WordPress. The proposed change is intentionally scoped to the ticket and is submitted here for code review, with the technical discussion remaining on this ticket.

Note: See TracTickets for help on using tickets.