| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | search.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | Quick fix |
Issue Summary
The default #value for processed_keys in the 'search_form' is an array.
search.module -- line 1052:
$form['basic']['inline']['processed_keys'] = array('#type' => 'value', '#value' => array());
The validation function (search_form_validate in search.pages.inc) sets processed_keys to a trimmed version of the keys string. If the validation function fails to run (eg. the form is used without loading search.pages.inc), some code will get an empty array, when expecting a string.
I ran into this because I forgot to load search.pages.inc and I am using the apachesolr_search module. The apachesolr_search module form_alters in a new submit handler that expects processed_keys to always be sane.
I can't see a reason the default value of processed_keys is an array. Maybe I missed something here.
Comments
#1
I can't see a reason either. Should be fixed in Drupal 7 and then backported to Drupal 6.
Also, shouldn't the form function and the validate/submit functions all be together in the same file?
#2
Here's a quick patch to fix the processed keys default.
#3
#2: 556284.patch queued for re-testing.
#4
yep, that's a bug, thanks!
#5
Hm. Is it possible to follow this down and see if it breaks anything? And if so, we should probably have a test for it. If not, and it's just purely for consistency, that's fine.
#6
This won't throw an error because it's first use is in preg_match() which accepts an array(). However, we eventually return a string, so it's just confusing that the default is an array(). I don't think of any test that would catch this.
#7
still applies.
#8
#2: 556284.patch queued for re-testing.
#9
#2: 556284.patch queued for re-testing.
#10
#2: 556284.patch queued for re-testing.
#11
Could we possibly get this committed, assuming this latest retest request passes?
#12
#2: 556284.patch queued for re-testing.
#13
adding tag
#14
#2: 556284.patch queued for re-testing.
#15
Makes sense and doesn't look like this can be reasonably tested.
#16
Ok, Committed to HEAD.
Marking to 6.x for backport.
#17
From a DX point of view, i think the default form handlers should be in the same file where the form is defined.
search_formis insearch.modulesearch_form_validateis insearch.pages.incsearch_form_submitis insearch.pages.incSo when you call
drupal_get_form('search_form')in a custom page the default validate/submit handlers are not set and called unless you explicitly includesearch.pages.inc.#18
RE #17 - that is a separate issue. If you would like to advocate for this, please file it as a separate issue. Thanks!
#19
processed_keys are normalized by the default validator:search_form_validate. If the form and the validator were in the same file this issue wouldn't have appeared at all.
So, what i mentioned is the source of this issue.
#20
D6 backport.