Opened 14 years ago
Closed 12 years ago
#16541 closed defect (bug) (fixed)
get_search_form() ignores $echo argument if searchform.php exists
Reported by: | kawauso | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 3.6 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Template | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
If a theme has searchform.php present, get_search_form()
uses require()
regardless of the $echo
value.
As per discussion with nacin in IRC, the only workaround for this would be use output buffering (which would also allow for searchform.php to be passed through the 'get_search_form' filter).
Attachments (3)
Change History (23)
#2
@
14 years ago
Does the content of the search form template (if it exists) need to pass through get_search_form
filter? Or only the default $form
?
My patch passes the content of the search form template through the filter instead of returning prematurely.
#3
@
14 years ago
I would like to disable the include of searchform.php (sitewide on a multi-site install with 100+ themes) and pass a custom $form value trough the filter with a plugin.
If this is not possible because of compatibility issues, I wish get_search_form() or locate_template() becomes pluggable...
With the latest patch provided by garyc40, at least I don't have to modify the core however searchform.php is still included and the outbut buffering is probably a waste of resources.
#4
@
13 years ago
I agree. The current design of get_search_form() is very strange. The echo parameter is present but only works with the default content. One solution is the output buffering as in the patch above, but I agree with the sentiment that it is a waste of resources. A workaround is to pass the form through the get_search_form filter but again, that is a waste of resources as get_search_form() will look for, and possibly require, a file only to have it essentially discarded in favor of the form passed through the filter. My suggestion is to drop the file search and include altogether and pass the form only via the filter. My only concern is that this method could be awkward with large and complicated search forms.
Feedback?
#6
follow-up:
↓ 7
@
13 years ago
- Cc shawnkhall added
It would be good if locate_template were hookable. Immediately inside the locate_template function it should have something like this:
$template_names = apply_filter( 'locate_template', $template_names );
This would enable plugins and themes to bypass behavior of certain core functions, such as get_search_form() without having to rewrite core.
#7
in reply to:
↑ 6
@
13 years ago
Replying to shawnkhall:
It would be good if locate_template were hookable.
See #13239. Specifically, patch 13239.c2c.4.diff. Nacin seemed close to liking an earlier version of the patch.
I certainly favor that approach over output buffering.
#8
follow-ups:
↓ 9
↓ 12
@
13 years ago
- Cc chip@… added
So, this could be a totally crazy idea (or totally stupid), but why not use file_get_contents()
, and do something like:
$search_form_template = locate_template( 'searchform.php' ); if ( '' != $search_form_template ) { $form = file_get_contents( $search_form_template ); } else { // Default form handling goes here } if ( $echo ) { echo apply_filters('get_search_form', $form); } else { return apply_filters('get_search_form', $form); }
That way, both the searchform.php form, as a string, and the default form, also as a string, get passed through the get_search_form
filter.
Is there some performance and/or security issue with using file_get_contents()
, or something else that I'm missing?
#12
in reply to:
↑ 8
@
12 years ago
Replying to chipbennett:
Is there some performance and/or security issue with using
file_get_contents()
, or something else that I'm missing?
Not necessarily, and file_get_contents()
was also my first thought. It works beautifully for pure HTML forms, but if there is any PHP involved, it's a lot riskier.
It will (1) pass PHP code as plain text, and (2) require an eval()
to run that code, all form a form where a 3rd party user can directly submit input. I can't think of a specific expliot, but it makes me nervous, especially when an object buffer is a viable alternative.
#15
@
12 years ago
- Cc mark@… added
Just wasted 30 minutes of my life because of this :( If there is no decision how to support the echo parameter then maybe it should be deprecated. Right now it is not reliable enough to be used in any case.
#16
@
12 years ago
16541.2.diff looks sane to me.
Silly output buffering