WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 13 months 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 13 months ago.
19321.2.patch (1.0 KB) - added by DrewAPicture 13 months ago.
'pre_get_search_form'
19321.3.patch (1.3 KB) - added by SergeyBiryukov 13 months ago.
19321.4.patch (1.3 KB) - added by SergeyBiryukov 13 months ago.
Refreshed after [23798]

Download all attachments as: .zip

Change History (15)

comment:1 nacin2 years ago

  • Milestone changed from Awaiting Review to 2.7

Was introduced like this.

comment:2 nacin2 years ago

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

comment:3 chipbennett2 years ago

  • Cc chip@… added

comment:4 WraithKenny17 months ago

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

SergeyBiryukov13 months ago

comment:5 follow-ups: SergeyBiryukov13 months 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.

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.

Version 0, edited 13 months ago by SergeyBiryukov (next)

comment:6 in reply to: ↑ 5 ; follow-up: DrewAPicture13 months 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 SergeyBiryukov13 months 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: SergeyBiryukov13 months 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.

DrewAPicture13 months ago

'pre_get_search_form'

comment:9 DrewAPicture13 months ago

  • Keywords needs-codex added

SergeyBiryukov13 months ago

comment:10 in reply to: ↑ 8 SergeyBiryukov13 months 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.

SergeyBiryukov13 months ago

Refreshed after [23798]

comment:11 SergeyBiryukov13 months 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.