Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.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.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | search_form_processed_keys-556284-d6-20.patch | 704 bytes | Albert Volkman |
| #2 | 556284.patch | 762 bytes | jhodgdon |
Comments
Comment #1
jhodgdonI 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?
Comment #2
jhodgdonHere's a quick patch to fix the processed keys default.
Comment #3
jhodgdon#2: 556284.patch queued for re-testing.
Comment #4
douggreen CreditAttribution: douggreen commentedyep, that's a bug, thanks!
Comment #5
webchickHm. 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.
Comment #6
douggreen CreditAttribution: douggreen commentedThis 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.
Comment #7
klausistill applies.
Comment #8
jhodgdon#2: 556284.patch queued for re-testing.
Comment #9
jhodgdon#2: 556284.patch queued for re-testing.
Comment #10
jhodgdon#2: 556284.patch queued for re-testing.
Comment #11
jhodgdonCould we possibly get this committed, assuming this latest retest request passes?
Comment #12
jhodgdon#2: 556284.patch queued for re-testing.
Comment #13
jhodgdonadding tag
Comment #14
sun#2: 556284.patch queued for re-testing.
Comment #15
sunMakes sense and doesn't look like this can be reasonably tested.
Comment #16
webchickOk, Committed to HEAD.
Marking to 6.x for backport.
Comment #17
ufku CreditAttribution: ufku commentedFrom 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.Comment #18
jhodgdonRE #17 - that is a separate issue. If you would like to advocate for this, please file it as a separate issue. Thanks!
Comment #19
ufku CreditAttribution: ufku commentedprocessed_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.
Comment #20
Albert Volkman CreditAttribution: Albert Volkman commentedD6 backport.
Comment #21
jhodgdonIn the event that we are still committing things like this to Drupal 6, this should be committed.
Comment #24
jhodgdonComment #26
jhodgdonThere's another D6 patch that is failing with these same errors. Something wrong with the bot?
Comment #27
jhodgdonTalked with Gabor (the Drupal 6 branch maintainer) and D6 issues are really not being committed unless they're really essential -- we really don't have a test system for Drupal 6 and it's too dangerous. So... putting this back to D7 / fixed, even though this is a simple one-line patch.