WordPress.org

Make WordPress Core

#20929 closed defect (bug) (fixed)

wp_list_filter Notice

Reported by: wpsmith Owned by: nacin
Priority: normal Milestone: 3.5
Component: Taxonomy Version:
Severity: normal Keywords: has-patch needs-refresh
Cc: bpetty, kpayne@…

Description

If someone wanted to add an argument for easy filtering on a taxonomy and then used get_taxonomies to query that non-standard key, it generates notices.

E.g., Given labels,

$args = array( 'mycustomkey' => true, 'hierarchical' => 0, 'rewrite' => array( 'slug' ), );
register_taxonomy( 'my_taxonomy', array( 'my_post_type' ), $args);

Then later, if I used:

 get_taxonomies( array( 'mycustomkey' => true ) );

it generates:

Notice: Undefined index: mycustomkey in C:\xampp\htdocs\www\sandbox\wp-includes\functions.php on line 2499.

This is a simple fix:

if ( isset($to_match[ $m_key ]) && $m_value == $to_match[ $m_key ] )

Attachments (2)

functions.wp_list_filter.patch (435 bytes) - added by wpsmith 12 months ago.
20929-unit-test.patch (1.7 KB) - added by kurtpayne 12 months ago.
First pass unit test

Download all attachments as: .zip

Change History (8)

comment:1 bpetty12 months ago

  • Cc bpetty added

I was able to reproduce this bug very easily.

I have also reviewed and tested this patch with 3.4, and this looks good.

comment:2 follow-up: nacin12 months ago

  • Keywords needs-refresh needs-unit-tests added; dev-feedback removed
  • Milestone changed from Awaiting Review to 3.5

Looks good at a glance. Some thoughts and housekeeping:

  • Let's use array_key_exists() here instead. isset() will return false if the value is null, and that may not be expected behavior here.
  • If we had kept isset(), isset($to_match[ $m_key ]) && would be isset( $to_match[ $m_key ] ) && per our coding standards.
  • Please diff from the root of WordPress, rather than from within wp-includes. This will provide for a patch path of wp-includes/functions.php, rather than just functions.php. There happens to be only one functions.php file in core (outside of default themes), so when prompted someone applying the patch could guess, but there are three post.php files.
  • Some tests here would be cool. They could go into TestListFilter. If you run the test suite with the -d flag, it forces WP_DEBUG, and thus you can write a test that could normally generate a notice. Be sure to test the NULL case :-)

comment:3 in reply to: ↑ 2 ; follow-up: wpsmith12 months ago

Replying to nacin:

  • Some tests here would be cool. They could go into TestListFilter. If you run the test suite with the -d flag, it forces WP_DEBUG, and thus you can write a test that could normally generate a notice. Be sure to test the NULL case :-)

Thanks Nacin! Any tips/links on how to do the test or run the test suite?

comment:4 in reply to: ↑ 3 SergeyBiryukov12 months ago

Replying to wpsmith:

Any tips/links on how to do the test or run the test suite?

http://codex.wordpress.org/Automated_Testing

kurtpayne12 months ago

First pass unit test

comment:5 kurtpayne12 months ago

  • Cc kpayne@… added
  • Keywords needs-unit-tests removed

Attached the first attempt at a unit test. I'm sure it could be improved upon.

It doesn't rely on the user passing "-d" in, and will force E_NOTICE on to capture the notice message, since that's the symptom we're testing.

comment:6 nacin12 months ago

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

In [21184]:

Check if array key exists in wp_list_filter(). props wpsmith. fixes #20929.

Note: See TracTickets for help on using tickets.