WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 2 years ago

#19321 closed defect (bug) (fixed)

get_search_form() function calls an action and a filter with the same hook.

Reported by: marquex Owned by: SergeyBiryukov
Milestone: 3.6 Priority: normal
Severity: normal Version: 2.7
Component: Template Keywords: has-patch needs-codex commit
Focuses: Cc:

Description

The get_search_form() functions starts calling the action 'get_search_form', and its ends applying the filter 'get_search_form' if a searchform.php file doesn't exists.

So if a function is hooked in 'get_search_form' action, it is also called by the filter, as long as do_action and apply_filters grab the functions from the same array.

If I add an action in 'get_search_form' I would like to be called just once. Maybe the best thing to do here is to change the action hook name.

Cheers

Attachments (4)

19321.patch (1.0 KB) - added by SergeyBiryukov 2 years ago.
19321.2.patch (1.0 KB) - added by DrewAPicture 2 years ago.
'pre_get_search_form'
19321.3.patch (1.3 KB) - added by SergeyBiryukov 2 years ago.
19321.4.patch (1.3 KB) - added by SergeyBiryukov 2 years ago.
Refreshed after [23798]

Download all attachments as: .zip

Change History (15)

comment:1 @nacin3 years ago

  • Milestone changed from Awaiting Review to 2.7

Was introduced like this.

comment:2 @nacin3 years ago

  • Milestone changed from 2.7 to Awaiting Review
  • Version changed from 3.3 to 2.7

comment:3 @chipbennett3 years ago

  • Cc chip@… added

comment:4 @WraithKenny3 years ago

you could add a did_action check if you'd like it to fire only once.

@SergeyBiryukov2 years ago

comment:5 follow-ups: @SergeyBiryukov2 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.6

The get_search_form() functions starts calling the action 'get_search_form', and its ends applying the filter 'get_search_form' if a searchform.php file doesn't exists.

With the changes in #16541, the filter is now applied if searchform.php exists as well, which made the issue more prominent.

If a function is hooked to get_search_form as an action (e.g. to print something), it runs twice, so the output is printed twice. If it's hooked as a filter, it still runs twice, but at least properly returns the value.

I guess we should rename the action. I'd suggest get_search_form_before.

Last edited 2 years ago by SergeyBiryukov (previous) (diff)

comment:6 in reply to: ↑ 5 ; follow-up: @DrewAPicture2 years ago

Replying to SergeyBiryukov:

I guess we should rename the action. I'd suggest get_search_form_before.

I don't like 'before' because it implies there's a matching "after" filter, a la the theme hooks debate. What about something like 'pre_get_search_form' instead?

comment:7 in reply to: ↑ 6 @SergeyBiryukov2 years ago

Replying to DrewAPicture:

What about something like 'pre_get_search_form' instead?

Works for me. That was my initial idea as well, but it seemed that pre_* is often used in core for filters, so I thought it might be confusing as an action name.

comment:8 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov2 years ago

Replying to SergeyBiryukov:

If a function is hooked to get_search_form as an action (e.g. to print something), it runs twice, so the output is printed twice.

It also kills the search form if it doesn't return anything.

@DrewAPicture2 years ago

'pre_get_search_form'

comment:9 @DrewAPicture2 years ago

  • Keywords needs-codex added

@SergeyBiryukov2 years ago

comment:10 in reply to: ↑ 8 @SergeyBiryukov2 years ago

  • Keywords commit added

Replying to SergeyBiryukov:

It also kills the search form if it doesn't return anything.

19321.3.patch fixes that and keeps back compat for both current use cases.

@SergeyBiryukov2 years ago

Refreshed after [23798]

comment:11 @SergeyBiryukov2 years ago

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

In 23800:

Rename 'get_search_form' action to 'pre_get_search_form' to prevent collision with the filter of the same name. Make sure the filtered result is not null to prevent search form from disappearing if an action function is attached to the old hook. fixes #19321.

Note: See TracTickets for help on using tickets.