Make WordPress Core

Opened 8 years ago

Last modified 7 years ago

#36851 new defect (bug)

Widgets don't remove hooks after being unregistered

Reported by: westonruter's profile westonruter Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8
Component: Widgets Keywords: needs-patch
Focuses: Cc:


In WP_Widget_Recent_Comments::__construct(), there is this bit of code:

if ( is_active_widget( false, false, $this->id_base ) || is_customize_preview() ) {
        add_action( 'wp_head', array( $this, 'recent_comments_style' ) );

If unregister_widget( 'WP_Widget_Recent_Comments' ) is called, this added wp_head action is still going to persist unexpectedly. At the moment, I believe the only way to remedy this inside such widgets themselves would be to check to see if the widget is still among $wp_widget_factory->widgets when the action callback is called (here the recent_comments_style method). From outside the widget, the alternative is would be to do:

$widget = $wp_widget_factory->widgets['WP_Widget_Recent_Comments'];
unregister_widget( get_class( $widget ) );
remove_action( 'wp_head', array( $widget, 'recent_comments_style' ) );

Neither of these options are great.

Perhaps there should be new widget_registered and widget_unregistered actions that widgets could listen to to do this cleanup? Or there could be a new unregister method on WP_Widget that a subclass could have this logic inside of. (We wouldn't be able to use the PHP destructor since it would never be called since the reference to the class would be still captured among the registered hooks.) Likewise, instead of adding the hooks inside of the constructor, perhaps there should also be a WP_Widget::register() method that gets called inside of the faux-private WP_Widget::_register() (and _register should be final, no?)

Change History (1)

#1 @welcher
7 years ago

@westonruter I like the idea of the unregister method coupled with a do_action( 'widget_unregistered' ) whenever a widget is unregistered.

Adding a WP_Wiget:: register() method is also a great idea, however, I don't think we can assume that hooks added in a subclass will be there moving ahead. Perhaps it's something that we add and then make recommendations to use that instead of in the constructor in the docs.

Note: See TracTickets for help on using tickets.