Make WordPress Core

Opened 5 months ago

Last modified 5 months ago

#59507 new defect (bug)

global-styles-and-settings.php in wp_theme_has_theme_json

Reported by: cocasio45's profile cocasio45 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.3.1
Component: Themes Keywords: has-patch php81 needs-unit-tests close
Focuses: Cc:

Description

Wordpress 6.3.1 , php 8.1.23

<?php
function wp_theme_has_theme_json() {
        static $theme_has_support = array();

        $stylesheet = get_stylesheet();

        if (
                isset( $theme_has_support[ $stylesheet ] ) &&
                /*
                 * Ignore static cache when the development mode is set to 'theme', to avoid interfering with
                 * the theme developer's workflow.
                 */
                ! wp_is_development_mode( 'theme' )
        ) {
                return $theme_has_support[ $stylesheet ];
        }

        $stylesheet_directory = get_stylesheet_directory();
        $template_directory   = get_template_directory();

        // This is the same as get_theme_file_path(), which isn't available in load-styles.php context
        if ( $stylesheet_directory !== $template_directory && file_exists( $stylesheet_directory . '/theme.json' ) ) {
                $path = $stylesheet_directory . '/theme.json';
        } else {
                $path = $template_directory . '/theme.json';
        }

        /** This filter is documented in wp-includes/link-template.php */
        $path = apply_filters( 'theme_file_path', $path, 'theme.json' );
        
        /* ADD THIS SECTION */
        if( ! $path ) {
                $theme_has_support[ $stylesheet ] = false;
                return false;
        }
         /* END OF SECTION */

        $theme_has_support[ $stylesheet ] = file_exists( $path );
        /* file_exists THROW DEPRECATION ERROR WHEN RECIEVE NULL */

        return $theme_has_support[ $stylesheet ];
}

Attachments (1)

59507.patch (1.2 KB) - added by abditsori 5 months ago.

Download all attachments as: .zip

Change History (12)

This ticket was mentioned in PR #5357 on WordPress/wordpress-develop by AbdiTolesa.


5 months ago
#1

  • Keywords has-patch added; needs-patch removed

@abditsori
5 months ago

@jrf commented on PR #5357:


5 months ago
#2

Please add a test to proof the issue and safeguard the fix.

#3 @jrf
5 months ago

  • Focuses php-compatibility removed
  • Keywords php81 needs-unit-tests added

#4 @spacedmonkey
5 months ago

@cocasio45 Can you explain the reason for this ticket and the benefit of it. Thanks.

AbdiTolesa commented on PR #5357:


5 months ago
#5

Please add a test to proof the issue and safeguard the fix.

@jrfnl The issue could only happen if $path was set to null using the theme_file_path filter. Can I just use the hook in the unit test and assert false is returned instead?

@jrf commented on PR #5357:


5 months ago
#6

@AbdiTolesa You can.

#7 follow-up: @cocasio45
5 months ago

@spacedmonkey it's written in the code i forget to write it in the ticket but file_exists, when recieving the null value throw a DEPRECATION ERROR that's why

#8 in reply to: ↑ 7 ; follow-up: @jrf
5 months ago

  • Keywords close added

Replying to cocasio45:

@spacedmonkey it's written in the code i forget to write it in the ticket but file_exists, when recieving the null value throw a DEPRECATION ERROR that's why

To be honest, the only time that could happen is if a plugin/theme would hook into that filter and return a value of null, which is not a valid return type for the filter (it expects type string and only string).

In my opinion, this deprecation should not be hidden away. The plugin/theme doing it wrong should get fixed instead.

#9 in reply to: ↑ 8 @cocasio45
5 months ago

Replying to jrf:

Replying to cocasio45:

@spacedmonkey it's written in the code i forget to write it in the ticket but file_exists, when recieving the null value throw a DEPRECATION ERROR that's why

To be honest, the only time that could happen is if a plugin/theme would hook into that filter and return a value of null, which is not a valid return type for the filter (it expects type string and only string).

In my opinion, this deprecation should not be hidden away. The plugin/theme doing it wrong should get fixed instead.

I'm ok with that point of view, in this case we need to make the exception more visible because the only way that I could get the error is when I run xdebug

#10 follow-up: @spacedmonkey
5 months ago

The only time this happens is from developer error, when someone misuses theme_file_path filter. This DEPRECATION ERROR seems valid to me.

@cocasio45 In the future, please write a description. Not everyone on trac can read code. Even me, who can, I didn't read it.

I would vote for #wontfix on this ticket.

#11 in reply to: ↑ 10 @cocasio45
5 months ago

Replying to spacedmonkey:

The only time this happens is from developer error, when someone misuses theme_file_path filter. This DEPRECATION ERROR seems valid to me.

@cocasio45 In the future, please write a description. Not everyone on trac can read code. Even me, who can, I didn't read it.

I would vote for #wontfix on this ticket.

As told it will not appen again

Also I am not using a theme.json and this hook is executed so you sure the error the error is comming from me ?

Note: See TracTickets for help on using tickets.