WordPress.org

Make WordPress Core

#41743 closed defect (bug) (fixed)

Using the_widget on a widget that has not been registered results in an undefined index notice.

Reported by: SeBsZ Owned by: obenland
Milestone: 4.9 Priority: normal
Severity: normal Version: 2.8
Component: Widgets Keywords: good-first-bug has-patch
Focuses: Cc:

Description

First of all, this may be by design to notify the developer that they've probably made a mistake.

However, I'd like to suggest a small check that prevents this notice.

In Wordpress 4.8.1 wp-includes\widgets.php on line 1039 (or the second line inside the the_widget() function) there is:

$widget_obj = $wp_widget_factory->widgets[$widget];
if ( ! ( $widget_obj instanceof WP_Widget ) ) {
  return;
}

This means that calling the_widget with a $widget name that has not been registered, instantly results in an undefined index notice.

A simple check would solve this:

if ( ! ( isset($wp_widget_factory->widgets[$widget]) ) ) {
  return;
}

$widget_obj = $wp_widget_factory->widgets[$widget];
if ( ! ( $widget_obj instanceof WP_Widget ) ) {
  return;
}

Apologies if this is not a bug but by design.

Attachments (4)

41743.diff (938 bytes) - added by SeBsZ 11 months ago.
41743.2.diff (506 bytes) - added by mrasharirfan 11 months ago.
Cleaned up the patch according to WP Coding Standards.
41743.3.diff (1.0 KB) - added by mrasharirfan 11 months ago.
41743.patch (650 bytes) - added by ramiy 10 months ago.
Use informative message in _doing_it_wrong() function

Download all attachments as: .zip

Change History (20)

#1 @obenland
11 months ago

  • Keywords needs-patch good-first-bug added
  • Version changed from 4.8.1 to 2.8

Introduced in [11317].

Would you want to write a patch for that?

#2 @SeBsZ
11 months ago

Sure. This is my first patch here, let's hope I did it correctly.

@SeBsZ
11 months ago

#3 @obenland
11 months ago

  • Milestone changed from Awaiting Review to 4.9

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


11 months ago

@mrasharirfan
11 months ago

Cleaned up the patch according to WP Coding Standards.

#5 @mrasharirfan
11 months ago

  • Keywords has-patch added; needs-patch removed

Hi @obenland and @SeBsZ

I have tested the patch. And it is working fine.

Also, I have cleaned up the code according to WP Coding Standards given here: https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/

Cheers!

#6 @SeBsZ
11 months ago

Great, glad to have helped.

BTW, you cleaned up my code (added some spaces), however the line of code below mine does not use spaces inside the square brackets yet. Might need to clean that up as well? I actually copied that part for my patch.

#7 @mrasharirfan
11 months ago

Here you go. Cleaned up the line below as well.

#8 @westonruter
11 months ago

To me it seems having a notice/warning is better than failing silently. If you have a theme/plugin that is calling the_widget() then you should know when it fails.

The other option would be to return a WP_Error on failure, and true on success.

#9 @obenland
11 months ago

Agreed, I was about to suggest adding a _doing_it_wrong() call here to tell developers that widgets need to be registered first

#10 @SeBsZ
11 months ago

I also agree, hence the reason my very first line in this ticket was:

First of all, this may be by design to notify the developer that they've probably made a mistake.

The reason I came across these notifications is my plugin was correctly registering and using, but another plugin (Yoast SEO) uses this call in certain situations:

<?php
remove_all_actions( 'widgets_init' );

Which led to my widgets not being registered when I tried to use them and that's how I first encountered the undefined index notice. I'm only saying this to explain that sometimes this isn't a mistake by a developer per se, but could be because of another plugin's incorrect behavior.

I do agree a _doing_it_wrong() is better than failing silently, so at least a developer will know something is wrong.

#11 @obenland
11 months ago

  • Owner set to obenland
  • Resolution set to fixed
  • Status changed from new to closed

In 41327:

Widgets: Add nudge for registered widgets

Informs developers that widgets need to be registered before they can be
displayed through the_widget(). Previously it would fail with an ambiguous
undefined index notice.

Props SeBsZ, mrasharirfan.
Fixes #41743.

#12 @ramiy
11 months ago

@obenland,

The text can be more informative. We can explain how to register widgets. It is done in other messages for developers.

Just replace:

_doing_it_wrong( __FUNCTION__, __( 'Widgets need to be registered before they can be displayed.' ), '4.9.0' );

With:

/* translators: %s: register_widget() */
_doing_it_wrong( __FUNCTION__, sprintf( __( 'Widgets need to be registered using %s, before they can be displayed.' ), '<code>register_widget()</code>' ), '4.9.0' );

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


10 months ago

#14 @SergeyBiryukov
10 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to address comment:12.

@ramiy
10 months ago

Use informative message in _doing_it_wrong() function

#15 @ramiy
10 months ago

@obenland, @SergeyBiryukov

You can use the attached patch and close this ticket.

#16 @obenland
10 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 41340:

Widgets: Improve _doing_it_wrong message.

Adds more context to help sending developers on the path to success.

Props ramiy. Fixes 41743.

Note: See TracTickets for help on using tickets.