Make WordPress Core

Opened 13 months ago

Last modified 3 months ago

#58183 new defect (bug)

PHP notice when accessing Customizer with a classic theme without widgets

Reported by: kraftner's profile kraftner Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch needs-testing has-testing-info
Focuses: Cc:

Description (last modified by SergeyBiryukov)

If you open the Customizer with a classic theme that doesn't have any widget areas registered the following PHP warning is emitted:

PHP Warning:  Attempt to read property "title" on null in /var/www/html/wp-includes/class-wp-customize-widgets.php on line 905
PHP Stack trace:
PHP   1. {main}() /var/www/html/wp-admin/customize.php:0
PHP   2. do_action($hook_name = 'customize_controls_print_footer_scripts') /var/www/html/wp-admin/customize.php:302
PHP   3. WP_Hook->do_action($args = [0 => '']) /var/www/html/wp-includes/plugin.php:517
PHP   4. WP_Hook->apply_filters($value = '', $args = [0 => '']) /var/www/html/wp-includes/class-wp-hook.php:332
PHP   5. WP_Customize_Widgets->output_widget_control_templates('') /var/www/html/wp-includes/class-wp-hook.php:308

This seems to be similar to #54905.

It seems that this is a similar issue, also mentioned by #56142, but not included in the fix deployed with [54419] since @westonruter wasn't able to reproduce it: https://core.trac.wordpress.org/ticket/54905#comment:38

This tickets adds the steps to reproduce it in the hope to then be able to find a fix.

Steps to reproduce

  1. Create/Use a classic theme that does not have Widgets.
  2. Open the customizer
  3. Find the PHP warning in the logs.

Versions

WordPress 6.2
PHP 8.2

(But also appears with other, older combinations, e.g. WP 5.9 and PHP 8.0)

Change History (16)

#1 @SergeyBiryukov
13 months ago

  • Description modified (diff)

#2 @westonruter
13 months ago

Looking at this briefly, I can see that the Widgets panel is being registered with an active_callback set to WP_Customize_Widgets::is_panel_active(), which is defined as:

<?php
        public function is_panel_active() {
                global $wp_registered_sidebars;
                return ! empty( $wp_registered_sidebars );
        }

This is different to how the Menus panel was being registered conditionally, in that instead of an active callback the WP_Customize_Nav_Menus::check_capabilities() method was utilized prior to [54419]. When check_capabilities is used, it will actually cause the panel to be removed entirely in WP_Customize_Manager::prepare_controls() runs. This is why, as I recall, the notice was happening for menus, because the Panel got removed.

I remain unsure why this is happening for Widgets since check_capbilities is not being used, but rather the active_callback. So the panel shouldn't be getting removed.

#3 @westonruter
13 months ago

Seeing as the issue is specifically with the WP_Customize_Widgets::output_widget_control_templates() method, this is getting added to the customize_controls_print_footer_scripts action in the class's constructor:

<?php
add_action( 'customize_controls_print_footer_scripts', array( $this, 'output_widget_control_templates' ) );

This method is expecting for the widgets panel to have been created, and this is done by another method that is registered previously in the constructor:

<?php
add_action( 'customize_register', array( $this, 'schedule_customize_register' ), 1 );

In the admin, this schedule_customize_register calls the customize_register method right away. This method is what registers the panel.

So the panel should be registered. My guess is it may have something to do with the migration of widgets to blocks, and/or the incorporation of the Site Editor. But since happens in classic themes, I'm not sure.

It seems this will take some deeper step debugging to identify what is going on. Nevertheless, right now I don't have this queued up to look at further.

#4 @kraftner
13 months ago

Thanks for the further investigation. Today I took some time to look into this and actually ended up with a related other change, namely [52622]. Removing this line again makes the issue go away.

This is of course no solution because then #54888 which [52622] was supposed to fix reappears.

So I assume the fix is to go with the approach [54419] has and use customize_panel_active to deactivate the Widgets panel.

Last edited 13 months ago by kraftner (previous) (diff)

#5 @kraftner
13 months ago

I think the initial plan in #54888 was to remove the Widget and Menu panels when the theme doesn't support them.

Due to #54905 this was then changed in [54419] to not remove it, but only hide it.

The approach for the Widget panel in [52622] also was removing, not hiding. Ironically this changeset mentions consistency with the menu panel, which after [54419] actually isn't true any more. So for the widgets panel it was still removed instead of just hidden like the menu panel.
This then results in a similar issue as #54905, namely this ticket.

So, since just like with the menu panel removing instead of just deactivating has proven problematic, the same applies to the widget panel. So I think what actually needs to be done here, also in the name of consistency with the menu panel is to revert [52622].

As I understand it, the widget panel still wouldn't be shown for a theme without widgets areas since the active_callback still deactivates the panel without registered widget areas.

Version 0, edited 13 months ago by kraftner (next)

#6 @kraftner
13 months ago

Okay, now I get it. It would indeed be shown for a block theme.

So the right approach here probably is to do the same as was done for the navigation panel. Revert [52622] but then deactivate it for block themes by adding something similar to the menu panel removal.

This ticket was mentioned in PR #4375 on WordPress/wordpress-develop by kraftner.


13 months ago
#7

  • Keywords has-patch added

Hide instead of remove the widget panel to prevent PHP warning in classic themes.

Trac ticket: https://core.trac.wordpress.org/ticket/58183#comment:5

#8 @kraftner
13 months ago

  • Keywords has-patch removed

I just added a PR that seems to fix the issue here as far as I could tell. Maybe someone can have a look at it to confirm.

Last edited 13 months ago by kraftner (previous) (diff)

#9 @costdev
13 months ago

  • Keywords has-patch needs-testing added

#10 @costdev
13 months ago

  • Keywords has-testing-info added

This ticket was mentioned in Slack in #core by kraftner. View the logs.


12 months ago

#12 @SergeyBiryukov
6 months ago

  • Milestone changed from Awaiting Review to 6.5

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 months ago

#14 @audrasjb
3 months ago

As per today's bugscrub: we'll keep it in the milestone for now as the PR still applies cleanly against trunk. Let's revisit the ticket in a week to move it to Future release if necessary.

This ticket was mentioned in Slack in #core by rajinsharwar. View the logs.


3 months ago

#16 @rajinsharwar
3 months ago

  • Milestone changed from 6.5 to Future Release

Moving this ticket to "Future Release" as per @westonruter, this might need some invetsigation: https://core.trac.wordpress.org/ticket/58183#comment:3

If we have some activity, feel free to put this back in the milestone.

Note: See TracTickets for help on using tickets.