WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 3 weeks ago

#50568 new defect (bug)

Improve WP_Term's sanitization calls

Reported by: Cybr Owned by:
Milestone: 5.7 Priority: normal
Severity: normal Version: 5.5
Component: Taxonomy Keywords: has-patch needs-unit-tests early
Focuses: performance Cc:

Description

Akin to WP_Post::filter(), I think WP_Term::filter() should have a bypass, too. This will significantly improve performance wherever terms are used (list edit screens, category widgets, blocks) because the bypass prevents redundant re-sanitization.

The attached patch will shave off a 30% load time at wp-admin/edit.php after #50567's implementation. This patch tests for the WP_Term object's ::$filter state, and only re-sanitizes the term when the state differs from the input argument.

You'll also find that sanitize_term() now looks a lot like sanitize_post()--the same goes for get_term()'s resemblance to get_post(). Overall, it seems posts have received a lot more love over the years, and this patch steals some of that.

There are a few issues with terms, however. For example, update_term_cache() caches terms regardless of being shared, while WP_Term::get_instance() tries to prevent that. The $filter = 'display' argument is used in contexts where raw would do fine, too (e.g. @ WP_Terms_List_Table::single_row()). If we iron those issues out, we can fully phase out the re-sanitization of terms.

Attachments (1)

wp-includes.patch (5.1 KB) - added by Cybr 6 months ago.

Download all attachments as: .zip

Change History (7)

@Cybr
6 months ago

#1 @SergeyBiryukov
6 months ago

  • Milestone changed from Awaiting Review to 5.6

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


3 months ago

#3 @hellofromTonya
3 months ago

  • Keywords needs-unit-tests early added
  • Milestone changed from 5.6 to 5.7

From today's 5.6 core scrub, Sergey:

This could probably use some unit tests. Both #50567 and this one are nice performance enhancements, but also seem like early tickets, should probably be moved to 5.7.

Moving this ticket to early 5.7 milestone along with #50567.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

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


5 weeks ago

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


4 weeks ago

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


3 weeks ago

Note: See TracTickets for help on using tickets.