It's used to set the #action of the form, which is great. Unfortunately it then does a hardcoded redirect, so there is pretty much no value at all to this argument. I'd actually consider this an API bug worth backporting to D6.

CommentFileSizeAuthor
#6 search-redirect-bug.patch1.99 KBmerlinofchaos
#5 drupal.search-form-submit.patch727 bytessun
#3 search-redirect-bug.patch651 bytesmerlinofchaos
#1 search-redirect-bug.patch651 bytesmerlinofchaos

Comments

merlinofchaos’s picture

merlinofchaos commented
StatusFileSize
new search-redirect-bug.patch651 bytes

Status: Needs review » Needs work

The last submitted patch failed testing.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs work » Needs review
StatusFileSize
new search-redirect-bug.patch651 bytes

Weird. THe tests pass for me when I run them. Let's try again.

Status: Needs review » Needs work

The last submitted patch failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review
StatusFileSize
new drupal.search-form-submit.patch727 bytes

How about that?

merlinofchaos’s picture

merlinofchaos commented
StatusFileSize
new search-redirect-bug.patch1.99 KB

sun's patch fails without clean urls, which is why mine failed testing but only for the bot.

This one corrects for that and stores the pre-url() called value. However, this means that the actual argument to search_form() is changed slightly. Except that nothing in core calls search_form itself.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

#6: search-redirect-bug.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ modules/search/search.module	9 Oct 2009 20:53:52 -0000
@@ -861,13 +861,15 @@ function search_form($form, &$form_state
+  // Record the $action for later use in redirecting.
+  $form_state['action'] = $action;

+++ modules/search/search.pages.inc	9 Oct 2009 20:53:52 -0000
@@ -138,6 +138,6 @@ function search_form_submit($form, &$for
-  $form_state['redirect'] = 'search/' . $type . '/' . $keys;
+  $form_state['redirect'] = $form_state['action'] . '/' . $keys;
   return;

I wonder whether we cannot simply set $form_state['redirect'] in the form builder already? Or do we nuke that property for no good reason?

Powered by Dreditor.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

RE #8 sun: Because they keys may have been altered by other modules, so this uses the "processed keys" and adds them in at the end of the submit. Or so I gather from other comments in the code.

I think we should get this one in, so after requesting a test again, I'll probably set it to RTBC unless sun objects?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

#6: search-redirect-bug.patch queued for re-testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Wait for the testing bot though, just in case...

Status: Reviewed & tested by the community » Needs work

The last submitted patch, search-redirect-bug.patch, failed testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Hmmm. Those test failures are all related to some JavaScript thing in locale.module. I think I'll retest because I don't believe this patch caused those failures.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs work » Needs review

#6: search-redirect-bug.patch queued for re-testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Looks like the test bot was happier this time. Let's get this simple fix in.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.