Make WordPress Core

Opened 12 years ago

Closed 11 years ago

#19321 closed defect (bug) (fixed)

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

Reported by: marquex's profile marquex Owned by: sergeybiryukov's profile 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 11 years ago.
19321.2.patch (1.0 KB) - added by DrewAPicture 11 years ago.
'pre_get_search_form'
19321.3.patch (1.3 KB) - added by SergeyBiryukov 11 years ago.
19321.4.patch (1.3 KB) - added by SergeyBiryukov 11 years ago.
Refreshed after [23798]

Download all attachments as: .zip

Change History (15)

#1 @nacin
12 years ago

  • Milestone changed from Awaiting Review to 2.7

Was introduced like this.

#2 @nacin
12 years ago

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

#3 @chipbennett
12 years ago

  • Cc chip@… added

#4 @WraithKenny
11 years ago

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

#5 follow-ups: @SergeyBiryukov
11 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 11 years ago by SergeyBiryukov (previous) (diff)

#6 in reply to: ↑ 5 ; follow-up: @DrewAPicture
11 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?

#7 in reply to: ↑ 6 @SergeyBiryukov
11 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.

#8 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov
11 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.

@DrewAPicture
11 years ago

'pre_get_search_form'

#9 @DrewAPicture
11 years ago

  • Keywords needs-codex added

#10 in reply to: ↑ 8 @SergeyBiryukov
11 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.

@SergeyBiryukov
11 years ago

Refreshed after [23798]

#11 @SergeyBiryukov
11 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.