Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 2 years ago

#51137 closed defect (bug) (fixed)

wp_terms_checklist not checking selected taxonomy items with selected_cats option

Reported by: brianhogg's profile brianhogg Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.5.1 Priority: normal
Severity: normal Version: 5.5
Component: Taxonomy Keywords: has-patch
Focuses: template, coding-standards Cc:

Description

Since WP 5.5, using the selected_cats option with an array of taxonomy IDs as strings no longer selects those items in the checklist. It appears line 174 of wp-admin/includes/template.php is using a strict type comparison for in_array:

if ( in_array( $categories[ $k ]->term_id, $args['selected_cats'], true ) ) {

Since form results are often returned as strings, and the term ID will always be an integer in the WP_Term class, I think the strict flag should be removed. Otherwise anyone using the function would need to ensure the array elements are integers with something like array_map( 'intval', ... )

Attachments (1)

patch.diff (575 bytes) - added by brianhogg 4 years ago.

Download all attachments as: .zip

Change History (8)

@brianhogg
4 years ago

#1 @brianhogg
4 years ago

  • Keywords has-patch added; needs-patch removed

#2 @TimothyBlynJacobs
4 years ago

  • Focuses template coding-standards added
  • Milestone changed from Awaiting Review to 5.5.1

Thanks for the ticket @brianhogg!

Introduce in [47557]. I agree, I think this is worth fixing precisely because these functions are used so often in contexts where the value will be a string. Both form elements, and also as the results of get_option or get_metadata.

It looks like it also affects the following functions:

  • wp_popular_terms_checklist
  • wp_link_category_checklist
  • Walker_Category_Checklist::start_el

Looking at some other places in Core, we might instead want to array_map( 'intval' ) instead of making it a loose comparison. For instance WP_Http_Cookie::test.

Cc: @SergeyBiryukov.

#3 @SergeyBiryukov
4 years ago

  • Component changed from General to Taxonomy
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

Hi there, welcome back to WordPress Trac! Thanks for the report.

I agree this should be fixed in 5.5.1. I thought I verified that $args['selected_cats'] is an array of integers here, but apparently that's not always the case.

As noted above, array_map( 'intval', ... ) would be the way to go here, as removing the strict check would just reintroduce the coding standards issue.

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


4 years ago

#5 @SergeyBiryukov
4 years ago

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

In 48880:

Taxonomy: Make sure wp_terms_checklist() and Walker_Category_Checklist::start_el() properly handle an array of strings as selected_cats or popular_cats values.

Even with these values documented as an array of integers, they can technically also accept an array of strings, e.g. as form results.

Add a unit test.

Props brianhogg, TimothyBlynJacobs, SergeyBiryukov.
Fixes #51137.

#6 @SergeyBiryukov
4 years ago

In 48882:

Taxonomy: Make sure wp_terms_checklist() and Walker_Category_Checklist::start_el() properly handle an array of strings as selected_cats or popular_cats values.

Even with these values documented as an array of integers, they can technically also accept an array of strings, e.g. as form results.

Add a unit test.

Props brianhogg, TimothyBlynJacobs, SergeyBiryukov.
Merges [48880] to the 5.5 branch.
Fixes #51137.

#7 @SergeyBiryukov
2 years ago

In 51858:

Tests: Correct the @ticket reference in wp_terms_checklist() tests.

Follow-up to [48880].

See #53363, #51137.

Note: See TracTickets for help on using tickets.