Make WordPress Core

Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#64186 closed defect (bug) (fixed)

has_filter()/has_action() do not support an expected $priority param

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 6.9 Priority: normal
Severity: normal Version: 2.5
Component: Plugins Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

This is split out from #64178 for visibility. Initially explained in PR.

The has_action() and has_filter() functions are lacking an expected $priority parameter to be able to check whether a given callback is added at a specific priority. This is something I've missed in the past. It also "feels right" given that add_action()/remove_action()/add_filter()/remove_filter() all have an optional $priority parameter. See examples on GitHub and on WP Directory where ecosystem code assumed there was such a $priority parameter. This is important because the same callback can be added multiple times on the same hook with different priorities.

This is needed for #64178 to be able to move up the priority of wp_oembed_add_discovery_links() from 10 to 4. For back-compt, it cannot be unhooked at priority 10 for the sake of the many plugins that remove the action via:

<?php
remove_action( 'wp_head', 'wp_oembed_add_discovery_links' );

See GitHub Search (1.7k files) and WPDirectory (145 plugins).

So the action needs to be hooked twice:

<?php
add_action( 'wp_head', 'wp_oembed_add_discovery_links', 4 ); // Printed after feed_links() and feed_links_extra().
add_action( 'wp_head', 'wp_oembed_add_discovery_links' ); // Unhooked the first time that wp_oembed_add_discovery_links() runs for back-compat.

The wp_oembed_add_discovery_links() function can then make use of the new $priority parameter for has_action() to check whether a plugin has unhooked the function at the original priority:

<?php
// For back-compat, short-circuit if a plugin has removed the action at the original priority.
if ( ! has_action( 'wp_head', 'wp_oembed_add_discovery_links', 10 ) ) {
        return;
}

// Prevent running again at the original priority.
remove_action( 'wp_head', 'wp_oembed_add_discovery_links' );

This was accounting for back-compat was similarly in r60910 with the difference there being a new function was introduced as opposed to reusing the same function.

Change History (4)

This ticket was mentioned in PR #10456 on WordPress/wordpress-develop by @westonruter.


3 months ago
#1

  • Keywords has-patch has-unit-tests added

This cherry picks the has_filter()/has_action() changes from https://github.com/WordPress/wordpress-develop/pull/10449 to isolate them from the changes to wp_oembed_add_discovery_links().

Trac ticket: https://core.trac.wordpress.org/ticket/64186

@westonruter commented on PR #10456:


3 months ago
#2

Review from Gemini:

The changes introduce a new $priority parameter to the has_filter method in WP_Hook class and the has_filter and has_action functions in plugin.php. This allows checking for the existence of a filter or action at a specific priority.

Review Summary:

  • src/wp-includes/class-wp-hook.php:
    • The has_filter method now accepts an optional $priority parameter.
    • The docblock for has_filter has been updated with @since 6.9.0 Added the $priority parameter. and the @param and @return descriptions are correctly updated to reflect the new parameter and its behavior.
    • The internal logic correctly handles the new $priority parameter, including a check for is_int( $priority ) and changing the foreach loop variable to $callback_priority to avoid conflicts.
  • src/wp-includes/plugin.php:
    • The has_filter and has_action functions now accept an optional $priority parameter and pass it to the WP_Hook class's has_filter method.
    • Docblocks for both functions are updated with @since 6.9.0 Added the $priority parameter. and the @param and @return descriptions are correctly updated.
  • Test Files (tests/phpunit/tests/actions.php, tests/phpunit/tests/filters.php, tests/phpunit/tests/hooks/hasFilter.php):
    • New tests and modifications to existing tests have been added to cover the new $priority parameter functionality for has_action and has_filter.
    • @ticket 64186 has been added to the relevant test methods, which is appropriate for test files.

Overall Assessment:

The changes are well-implemented and adhere to WordPress coding standards. The docblocks are correctly updated, and the new functionality is covered by unit tests. The code remains compatible with PHP 7.2.

#3 @westonruter
3 months ago

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

In 61118:

Plugins: Add missing $priority parameter to has_filter() and has_action().

This brings has_filter()/has_action() in parity with add_filter()/add_action() and remove_filter()/remove_action(), all of which support a $priority parameter.

Props westonruter, swissspidy.
Fixes #64186.
See #64178.

#4 @westonruter
3 months ago

In 61119:

Embeds: Add wp_oembed_add_discovery_links() to run earlier at wp_head priority 4.

This results in the oEmbed discovery links being printed before scripts and styles are printed. This helps ensure they appear within the first 150K bytes of the HTML response, which is required by WP_oEmbed::discover(). With increasing the styles_inline_size_limit from 20K to 40K in #63018, it becomes more probable that the oEmbed discovery links will be pushed out of range.

For backwards compatibility with themes and plugins that disable the oEmbed discovery links by unhooking wp_oembed_add_discovery_links() from running at wp_head priority 10 (even though the oembed_discovery_links filter is available to disable such links), this callback is added a second time to run earlier at priority 4. The first time the function runs, it checks to see if the callback is still hooked at priority 10. If not, the function short circuits. If it is still hooked at priority 10, however, the function then unhooks itself at priority 10 so that it won't run a second time later during the wp_head action, before proceeding with printing the discovery links. A similar back-compat approach was taken previously in [60910]. The back-compat checks are only performed if the function is invoked during the wp_head action, allowing the function to be called "idempotently" elsewhere.

Developed in https://github.com/WordPress/wordpress-develop/pull/10449

Follow-up to [61118], [61117].

Props westonruter, swissspidy.
See #64186, #63018.
Fixes #64178.

Note: See TracTickets for help on using tickets.