Adding forms in the search result won't work. When submitting a form, the search result will be empty, so the forms won't be validated/processed.
A fixed that worked for me was to adjust the search.pages.inc file (see attachment).

What are the steps required to reproduce the bug?

Include forms in the search results.

What behavior were you expecting?

Working forms. :-)

What happened instead?

Forms do not work and the search keywords are reset.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Island Usurper’s picture

Version: 6.12 » 7.x-dev
Status: Active » Needs review

I can confirm that this does the job in D6. I imagine that it should work the same for D7. What say you, testbot?

Status: Needs review » Needs work

The last submitted patch failed testing.

Island Usurper’s picture

Status: Needs work » Needs review
FileSize
612 bytes

Ah, I didn't notice that the patch wasn't against the Drupal root. Rerolled.

hobbes’s picture

Island Usurper’s picture

That was me who said that the patch was tested successfully. It would be good to have someone else confirm the fix before marking it RTBC.

Just had a thought, though. Would it be better to make sure that $_POST['keys'] doesn't belong to the search form, and not some other form that has been displayed on the page?

Status: Needs review » Needs work

The last submitted patch failed testing.

netsensei’s picture

Tried the fix. Didn't work for me.

Context:

I'm trying to integrate the ubercart 'add-to-cart' form in my search results. In my template.php I did something like this:

phptemplate_preprocess_search_result(&$variables) {
  $product = node_load($result['node']->nid);
  $variables['addtocart']           = theme('uc_product_add_to_cart', $product);
}

Okay, ignore the node_load: there are prettier ways to get a node loaded then in a preprocess function. The second line is relevant. theme_uc_product_add_to_cart() returns a rendered form (HTML). The $addtocart variable gets printed in the template.

Thus, in my search results, I get a nice 'add to cart' button for every product in the search results.

The problem

When I press 'add to cart', the search results page goes blank and the product didn't get added to the cart. The project I'm working on required me to perform a hook_form_alter on the cart form but that didn't seem to make a big difference. With or without the alterations: the issue remains. Even so, extra custom submit/validation handlers just don't get executed either.

Analysis

I went digging in forms.inc. I noticed that theme_uc_product_add_to_cart() is called twice for each result. I dumped the output of the drupal_get_form which renders the form and noticed an issue with the form id's.

i.e. let's say we have product with a node ID of 304.

First pass returns a form with form id uc-product-add-to-cart-form-304
Second pass returns a form with form id uc-product-add-to-cart-form-304-1

The form returned by the second pass is used in the preprocess function and embedded in the page. I don't have time to delve deeper and get to the bottom of this. Maybe the form register gets confused and only registers the first form? The evidence point to this as the form doesn't get executed in the first place.

The form action also points to the search result page. So, I'd guess the fix above is more a patch against a symptom of the problem then a fix for the root cause.

So, this needs more looking into by someone with a better understanding of the inner guts of Drupal.

netsensei’s picture

Okay,

This morning, Occam's razor proved right once more: although there is most def a - albeit separate - problem with the theming layer in search, it wasn't causing my problem.

Apparently, the project I'm working on runs Apachesolr. It overrides search_view with it's own apachesolr_search_view() function which does basically the same: taking the data from $_POST and pushing it through drupal_goto.

I'm going to create an issue in the Apachesolr queue with a patch against this one.

Status: Needs work » Needs review

robertDouglass requested that failed test be re-tested.

jhodgdon’s picture

Status: Needs review » Needs work

It is hard to review this patch/issue without some more information --- how exactly are you trying to insert a form in the search results?

A test on the patch (i.e. a test that fails without the patch and passes with the patch) would help illuminate the problem and also help ensure that it doesn't break again.

jhodgdon’s picture

Another D6 report of this issue: #503022: Incomplete test in search_view function -- I just closed that one as a duplicate of this one...

jhodgdon’s picture

See also #600424: search.module pulls arguments directly from $_GET rather than using the menu system, which is somewhat related. These two issues might both need to be fixed together.

netsensei’s picture

After giving it a bit of thought, I'm starting to think the issue at hand doesn't stop at the Search module. Similar issues rise at the ApacheSolr module (#614644: Forms attached to Apachesolr search results won't work) and even custom developers will be hindered by this (http://drupal.org/node/643744)

First some (horribly simplified) pseudo-code and context:

function mymodule_view() {

   if (!$_POST) {
      <execute stuff like process data from arguments which shouldn't execute if there was a $_POST>
   }

   $output .= drupal_get_form('mymodule_form');

   return $output;
}

function mymodule_form(&$form_state) {
  ....
  return $form;
}

function mymodule_form_submit($form, &$form_state) {
  .... <do stuff like map $_POST to $_GET arguments>
  drupal_goto('page-with-callback-mymodule_view()');
}

The issue being that we 'embed' a form in a separate page callback through drupal_get_form and we add logic which should be executed unless the form was actually submitted.

Yet, there are no provisions to make a page callback aware if a form was just submitted or not. The only places I know of, where Drupal provides the right context are FAPI callbacks (form_builder, validate and submit handlers) which are executed through drupal_get_form (which provides us with a nice, helpful $form_state array).

To make this work, I'd assume we would at least want to be able to check this in our page callback in an easy fashion:

* Was a form submitted? Yes or no?
* If yes: which form was submitted?

Making the entire $form_state available to page callbacks would be overkill, I guess :-) Maybe we should think about what information about submitted forms should be available.

I suppose this is something which affects the way core works when it searches for a page callback mapped to a path to execute. Would be great to know if this is something which can be implemented and what objections can be raised against such a change.

Alternative way to deal with this from the current context: instead of a 'regular' page callback, use drupal_get_form as a page callback, pass the search form as a 'page arguments' and render the entire result set from within the form builder. At least we'll have $form_state to check if the form was submitted.

jhodgdon’s picture

Status: Needs work » Postponed (maintainer needs more info)

So...

I was confused about this, so I did a bit of digging. Some observations:

- When you build a form as the main form for a page, normally drupal_get_form() is the page callback, and the action of the form is the same as the action for the page. Then after submitting the form via POST from a browser, drupal_get_form() when it builds the page/form again, notices the POST information and processes it, calling the submit handlers, validation, etc.
- If you have a form in a block (or any other form on a page that the main page handler shouldn't handle), one way to build it is with an action attribute on the form, so that when that form submits, it will go to a different page that knows how to handle that form submission.
- Alternatively, you can leave the action blank, and submit to the same page the block/section is displayed on. In this case, when the block/section gets rebuilt, it will do a drupal_get_form() again, which will realize it has POST data specific to that form, and process the POST data.
- The search page function search_view() is bypassing, at least somewhat, this process. It doesn't want to process its own submissions via drupal_get_form(), because it has its own way of processing things and because it could have been submitted via the search block or the search form on the page, which, for whatever reason, are two different forms.
- Apparently this bypassing that search_view() is doing is affecting Ubercart "add to cart" button forms.
- The bypassing that search_view() is doing isn't affecting all forms whose action is the current page, however. For instance, the user login block works fine on the search page.

So... what I would like is some more information about these particular forms that are NOT working when embedded into the page. I tried to look at the issue on the ubercart web site referenced in #4, but the URL didn't work (access denied). Here's a corrected URL:
http://www.ubercart.org/issue/5089/search_result_teaser_add_cart_button_...
(or go to Issues and search for "search result teaser add cart button" without the quotes)
But it didn't give me any information about what kind of form was being added, or why it wouldn't have been rebuilt and submitted properly in a drupal_get_form(), the way the login form is rebuilt and submitted properly. So I'm confused. Also, comment #7 above indicates that the proposed fix didn't work for that person.

Can you shed any light on this, perhaps Island Usurper (or anyone else)?

Carlitus’s picture

#3 It works for me

jhodgdon’s picture

Carlitus: Can you give me some more information about what form you were trying to add, how you did it, etc.? (see #14)

Carlitus’s picture

I added the add_to_cart in the search results in the preprocess_search_result

  if (isset($result['node']->content['add_to_cart'])) {
    $info['add_to_cart'] = $result['node']->content['add_to_cart']['#value'];
  } 

And in the search-result.tpl.php

  <div class="search-snippet">
      <span class="field-content"><?php print $info_split['add_to_cart']; 

?>

jhodgdon’s picture

Can you also post the resulting HTML form that gets added to the search results? Thanks.

jhodgdon’s picture

Just to clarify, see #14 above. Some other forms that could be on the search page, such as the user login form, work just fine. So I need to see a specific form (the full HTML code for the form) in order to figure out why. I guess I could also install Ubercart and test using your code snippet form #17 (thanks for that by the way!), but as I am sure you know, configuring it even for testing takes a certain amount of time, so I was hoping to bypass that a bit.

Thanks!

netsensei’s picture

@carlitus: that's basically the same thing I did in #7

@jhodgdon: You can find a live example of an add-to-cart form integrated with a search results page on http://www.airmilesshop.nl. Note: we use ApacheSolr but the problem still stands because the Solr module uses a slightly altered copy of search_view() of the original search module and mimics the same concept.

We used the patch in #3 to fix this (we ported it to the Solr module). The problem is that it doesn't root out the original problem as described in #13.

jhodgdon’s picture

Thanks netsense! I went to your site and did a search for "air miles", and found forms like this being added to the search results (somewhat simplified - removed all the extra divs and labels):

<form action="/search/apachesolr_search/air%20miles"  accept-charset="UTF-8" method="post" id="uc-product-add-to-cart-form-4544">
    <input type="radio" id="edit-airpoints-0" name="airpoints" value="0"  checked="checked"  class="form-radio attribute-default active" />
    <input type="radio" id="edit-airpoints-1" name="airpoints" value="1"   class="form-radio attribute-default active" /> 
    <input type="hidden" name="qty" id="edit-qty" value="1"  />
    <input type="submit" name="op" id="edit-submit-4544" value="Voeg toe"  class="form-submit node-add-to-cart" />
    <input type="hidden" name="form_build_id" id="form-3fdcad978b648dfc592e8265d87b2361" value="form-3fdcad978b648dfc592e8265d87b2361"  />
    <input type="hidden" name="form_id" id="edit-uc-product-add-to-cart-form-4544" value="uc_product_add_to_cart_form_4544"  />
</form>

Note that /search/apachesolr_search/air%20miles is the URL I was on, so this is a form that submits to the same page.

So what I still don't understand is why this form should have problems when it gets submitted, but the form in (for example) a user login block (which also submits to the same page) would not have problems when it gets submitted on the search results page.

And I don't understand why the patch in #3 would fix the problem either.

This is sounding more and more like it is an Ubercart bug, especially given that in #7 you are saying that your theme function for the ubercart form is getting called twice per page, and so far the only forms I have heard of that don't work when they are on the search results page are ubercart forms.

Any ideas?

didlix’s picture

I am attempting the same thing with ubercart / add to cart on search page, once it submits the post data the search page no longer renders it's search results (despite the search url being the same).

Also - people are testing on D6 (version I have the issue with) but the version in the ticket is Version: 7.x-dev

jhodgdon’s picture

The version on this issue is Drupal 7 because we are fairly certain that the problem (if it is a problem in the Drupal Search module and not a problem in Ubercart) also exists in Drupal 7. The policy around here is that we fix issues on the in-development version of Drupal first, and then if necessary port the fix back to earlier versions of Drupal. This prevents situations where we have a bug that has been fixed in v. 6.x that comes back in v. 7.x.

didlix’s picture

I can also confirm that the patch in #3 works with d6.

netsensei’s picture

@jhodgdon: Okay. So it's possibly a D7 issue yet definitely a D6 issue.

I've looked diagonally at the D7 code and I see several noteworthy differences between D6 and D7. übercart for D7 (or drupal commerce) are still under heavy development so I don't think there are already developers who hit this problem from that angle under D7.

I wonder if there are already developers out there who hit this problem in their - custom or contrib - D7 projects?

Regardless, unless we actually try to integrate some forms through a custom module in D7's search results and see what happens there, we can't really rule out that there is or isn't a problem.

The user login block form puzzles me, though. One theory might be that the order of execution determined in the system table where the user login form submit handler is executed before the search page callback. I've given it a quick look but so far a simple var_dump() test showed me that the user_block_view() function (which contains a drupal_get_form) gets executed after the search_view callback function.

jhodgdon’s picture

Until we understand what is really causing the problem, I don't think we can get a good fix for it, in Drupal 7 or Drupal 6. And I can tell you right now that if you put the version back to Drupal 6.x, the interest in fixing it among the Drupal core developers will drop to zero. Which is why I changed it to Drupal 7.

Carlitus’s picture

jhodhdon, here is my html code:

Without patch (doesn't work)

<form action="/search/node/raquetas" accept-charset="UTF-8" method="post" id="uc-product-add-to-cart-form-96">
  <input name="qty" id="edit-qty" value="1" type="hidden">
  <input name="op" id="edit-submit-96" value="Add to cart" class="form-submit node-add-to-cart" type="submit">
  <input name="form_build_id" id="form-88c020428a7cee8e3b4a159ad76cd071" value="form-88c020428a7cee8e3b4a159ad76cd071" type="hidden">
  <input name="form_id" id="edit-uc-product-add-to-cart-form-96" value="uc_product_add_to_cart_form_96" type="hidden">
</form>

With patch (it works)

<form action="/search/node/raquetas" accept-charset="UTF-8" method="post" id="uc-product-add-to-cart-form-96">
  <input name="qty" id="edit-qty" value="1" type="hidden">
  <input name="op" id="edit-submit-96" value="Add to cart" class="form-submit node-add-to-cart" type="submit">
  <input name="form_build_id" id="form-50c9dd9f45b3a03ba58e4328ed1d9fe3" value="form-50c9dd9f45b3a03ba58e4328ed1d9fe3" type="hidden">
  <input name="form_id" id="edit-uc-product-add-to-cart-form-96" value="uc_product_add_to_cart_form_96" type="hidden">
</form>

I don't see any diference...only the form_build_id

jhodgdon’s picture

The fix up there is not affecting your HTML code. It is affecting how it is being processed.

And if you want HTML to appear in a comment on an issue, put it inside a CODE HTML tag. i.e. <code> </code>

Carlitus’s picture

Uppss...edited, thanks.

netsensei’s picture

Okay,
.
I've dug a bit deeper into it. I've created a small form module for D7 which does these things:

1. Create a form with 1 textfield and 1 submit button
2. Create a page callback which returns the form
3. On submit drupal_set_message() is called to show the message "This will not be executed!"

If you surf to http://localhost/prototype, You'll see the form and it will work right out of the box. No problems there.

Okay.

Now I want to embed that same form in my search results.

First up:
1. Create a few articles
2. Run cron to index the articles.

Now for the actual embedding. I chose to do this:

function MYTHEME_preprocess_search_result(&$variables) {
  $variables['snippet'] .= drupal_render(drupal_get_form('prototype_form'));
}

Given that I'm building a new theme called MYTHEME, I override the preprocessor for the search_result.tpl.php template and concatenate the form to the $snippet variable. It's basically the same principle we've been doing in D6.

3. Now perform a search. You'll notice that the form will be attached to each search result. I had just one article in my installation so just 1 form came out in the search results listing.
4. Just enter some input in the embedded form and press "Send away"
5. Observe how you lose all search results and the submit handler is not being executed.

Here's the module code:

(Create a new module under sites/all/modules/custom/prototype, put the code in your prototype.module file and create a prototype.info file. Enable the module afterward.)

<?php
// $Id: $

/**
 * @file
 * Creates a new form and embeds it into a search result
 */
 
 /**
  * Implements hook_menu().
  */
function prototype_menu() {
  $items['prototype'] = array(
    'title' => 'Prototype',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('prototype_form'),
    'access arguments' => array('search content'),
    'type' => MENU_CALLBACK,
  );
  
  return $items;
}
 
/**
 * The form builder function
 */
function prototype_form($form, &$form_state) {
  $form['name'] = array(
    '#type' => 'textfield',
    '#title' => t('Your name'),
    '#maxlength' => 255,
    '#default_value' => '',
    '#required' => TRUE,
  );
  
  $form['actions'] = array('#type' => 'actions');
  $form['actions']['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Send away'),
  );
  
  $form['#submit'][] = 'prototype_form_submit';
  
  return $form;
}

/**
 * The corresponding submit handler callback
 */
function prototype_form_submit($form, &$form_state) {
  drupal_set_message("This will not be executed!");
}

So. To me this mimics exactly the same problem as I had when I embedded my form in D6 on the airmilesshop site.

To make matters even more difficult, I stumbled upon this while implementing my module:

drupal_get_form() returns a render array instead of a string

(issue) drupal_get_form() no longer renders your form to an HTML string. Instead, it returns a render array as expected by drupal_render(). Your module should strive to keep this array structure, and return an array in your menu callback. You are welcome to add/modify the array as you wish. Only under severe distress should you call drupal_render(drupal_get_form()) in order to mimic previous versions of Drupal. Returning an array lets developers make easy changes during hook_page_alter().

For related details see Menu "page callbacks" and blocks should return an array and hook_page_alter().

http://drupal.org/node/224333#unrendered

If I understand correctly, what we do here is actually a big no-no in D7. Using drupal_render in our template preprocessors like this is a last resort strategy. We shouldn't be doing that, yet the use case put forth here actually forces us to do just that.

If any, to me this looks like good enough a reason to get this sorted out before D7 ships.

jhodgdon’s picture

Status: Postponed (maintainer needs more info) » Active

Thanks for working out an example that fails! This looks like it will be very helpful.

I will take a look at this and see if I can work it into a test, so that we can fix the bug and have it stay fixed.

And by the way, I think it's fine to call drupal_render() in a theme. At that point, the page is already being rendered.

jhodgdon’s picture

Assigned: Unassigned » jhodgdon

I plan to work on this today, or early next week.

jhodgdon’s picture

Status: Active » Needs review
FileSize
5.66 KB

Here's the first step. It's a test that fails, illustrating this issue, based on #30 by netsensei above (thanks!). So at least we have an illustration of the problem.

Next step: fixing the problem (without breaking anything else).

Note: I expect this patch to fail tests.

jhodgdon’s picture

Note: in the .info file in the previous patch, it should have
hidden=TRUE

I had set it to FALSE for manual testing purposes.

jhodgdon’s picture

So. Just as a point of interest, I put in a debug statement at the top of search_view(), which is the page callback for searching. I printed out $_POST.

The search module in Drupal 7 is not ever having $_POST set to anything at all. So that line at the top that says:

  if (!isset($_POST['form_id'])) {

which was changed to

  if (!isset($_POST['keys'])) {

in the proposed patch in #3 above... well, it's basically irrelevant for the search module, and probably is just left over from Drupal 6, or 5, or something.

Status: Needs review » Needs work

The last submitted patch, 497206-failing-test.patch, failed testing.

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
8.62 KB

This patch appears to fix the problem. On my test box, all the search tests pass.

It also has the advantage of making search_view() be less weird and more standard Drupal (i.e. use drupal_get_form() to process the form, as it should).

pwolanin’s picture

I feel like I'm missing something in how this patch works. Are you sure the search is not running twice?

Or does the redirect on form submission happen within drupal_get_form() and thus bypass the rest of the function?

If this is working correctly, we should backport to Drupal 6.

pwolanin’s picture

Here's a backport to apachesolr 6.x: http://drupal.org/node/614644#comment-3084090 and indeed this seems to work as desired for at least the search form.

Patch needs a better code comment, like:

  // Construct the search form.  If the user submits POST data, this will
  // redirect to a GET request before the search actually runs.

Also - I don't think we don't need to return the form in 2 different ways - why not always put in the $build array?

jhodgdon’s picture

RE: return value:
drupal_get_form() is returning a $form array. So we can either return that as the $build for the page, or if there are search results, then there are other components to the page that need to be added to the $build. I don't see much value in putting the $form under another level of array if there's nothing else to be added to the page? It's already a renderable array.

I agree about on the comment to add, which should probably point the reader to drupal_form_submit(). But I think the main point of the patch is to make sure that drupal_get_form() is being done, so that the page is being processed in the standard Drupal way, and the search form wont' think it's submitted if a different form is actually the one being submitted.

As far as the search being done twice... If someone submits a different form on the page, then a complete page load happens and the same search as the last page load will be done again, yes. If someone submits the search form, then the first time through drupal_get_form it will just redirect to a the url search/[module path]/[keywords] and not do the search (from search_form_submit), and then that URL will get through to search_view() and do the search. I think.

pwolanin’s picture

My suggestion re: putting the form another level down in all cases is simply for consistency. E.g. in extreme cases for hook_page_alter.

jhodgdon’s picture

FileSize
8.98 KB

That is reasonable.

OK, here's a new patch that (a) adds more comments and (b) always returns a similar page array for rendering.

pwolanin’s picture

Status: Needs review » Postponed
FileSize
14.14 KB

Patch here is really stale - looks like it incorporates a change in the return of search_data() that didn't happen?

Here's a new patch - it's stacked on top of the patch from http://drupal.org/node/567100#comment-3178320

All test for search pass locally, but marking as postponed until the other patch is committed.

pwolanin’s picture

Status: Postponed » Needs work
pwolanin’s picture

Assigned: jhodgdon » Unassigned
Status: Needs work » Needs review
FileSize
14.44 KB

Here's a new patch - the strategy is changed a little since based on experience with apachesolr, running a query may have side effects in terms of form_alter, so you don't want to build the search form twice, and don't want to build it until after the query runs, if it is called for.

Status: Needs review » Needs work

The last submitted patch, 497206-search-form-45.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
14.07 KB

Hmm, looks like I used a stale variable name - trying again.

I hate having to keep $_POST in here, but I don't see any other way to satisfy the use case of modules that form-alter the search form if a search has actually executed.

jhodgdon’s picture

Status: Needs review » Needs work

Ummm... There seems to be stuff from an unrelated issue in this patch. What's all this stuff about conditions callbacks?

pwolanin’s picture

Title: Search conflicts with other forms » Avoid search conflicts with other forms, use menu API instead of search_get_keys()
Status: Needs work » Needs review

@jhogdon - remember we discussed that fix in IRC? In part it's in the same patch since the changed lines overlap.

The goal was to use the menu API instead of search_get_keys(), and also allow non-core modules to supply additional conditions (e.g. filters not jsut keywords) to the search. This is essential to avoid having to duplicate this entire function when writing a contrib search module.

jhodgdon’s picture

OK...
OK. I'm giving this a proper review. First just looking at the patch, starting from the top:

a) Needs , at end of line:

+    'conditions_callback' => 'search_example_conditions_callback'

Also, we don't normally put sample callback functions into the *.api.php files, but usually instead put them into the hook doc as @code segments. I don't mind this, but it isn't what we normally do. If you do want to keep it this way: (a) put an @see in the hook docblock to this function, and (b) maybe name it something else, because the Examples module kind of owns the namespace *_example. ?

b) \ No newline at end of file (in the search.test chunk)

c) Should we have a test for the new conditions stuff?

I'm going to add these comments now, and then actually try applying the patch and testing a few things...

jhodgdon’s picture

FileSize
14.07 KB

Here's a new patch fixing (a) and (b) to my satisfaction at least. How about a test for the new conditions stuff? I am not clear on how it is used.

Other than that, as the tests still pass and the code is clean and clear (IMO), I think it's ready to go...

jhodgdon’s picture

pwolanin pointed out I forgot to fix the missing , by mistake. Duh.

jhodgdon’s picture

FileSize
15.34 KB

Actually, I uploaded the wrong patch there.

jhodgdon’s picture

FileSize
15.34 KB

oh, and here's the comma

pwolanin’s picture

Here's a patch with an added test. Adding the test part of the patch alone leaves 3 fails, that the patch in #54 turns into all pass.

Also attached is the test portion as an isolated patch (as a txt) for the curious.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Assuming that the test bot agrees, I think the patch in #55 is ready to go.

Dries’s picture

Status: Reviewed & tested by the community » Needs work
+++ modules/search/search.api.php
@@ -30,9 +30,15 @@
+ *   - 'conditions_callback': Name of a callback function that is invoked by
+ *     search_view() to get an array of additional search conditions to pass to
+ *     search_data(). Sample callback function:
+ *     sample_search_conditions_callback().

Would be good to hint at why this is useful. The documentation is still very abstract.

+++ modules/search/search.api.php
@@ -40,10 +46,28 @@ function hook_search_info() {
+ * An example conditions callback function for search.

Conditions should be singular; condition.

+++ modules/search/search.api.php
@@ -40,10 +46,28 @@ function hook_search_info() {
+ * Pulls additional search keywords out of the $_REQUEST variable.

Instead of pulling it out $_POST that is?

+++ modules/search/search.pages.inc
@@ -12,9 +12,15 @@
  * @param $module
  *   Search module to use for the search.
  */
-function search_view($module = NULL) {
+function search_view($module = NULL, $keys = '') {

The new parameter $keys is not documented.

+++ modules/search/search.test
--- /dev/null
+++ modules/search/tests/search_embed_form.info

We shouldn't abbreviate 'embedded' to 'embed'.

+++ modules/search/tests/search_embed_form.module
@@ -0,0 +1,66 @@
+ * Test module implementing a form that can be embedded in search results.

It would be useful to explain (somewhere) why that is useful.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
19.01 KB

Rerolled based on feedback above. However, I think "conditions" (plural) is correct in all uses of "conditions_callback" and "conditions callback", since the callback may return an array of multiple conditions. I have tried to make that more clear in the example.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

I agree with pwolanin on conditions vs. condition for the callback.

I also think "embed" was fine in the name of the support module for testing. I was thinking of it in the verb form: a module that embeds a form, not as a short form of embedded. But I have no problem with the change, either.

Assuming the test bot agrees...

Dries’s picture

Status: Reviewed & tested by the community » Fixed

This looks great now. Thanks for incorporating my recommended changes. You were obviously right about 'conditions'; I misread the sentence.

Committed to CVS HEAD. :)

Status: Fixed » Closed (fixed)

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

chaps2’s picture

There is a work around if you don't want to patch core:

The forms that are handled correctly on the search results page tend to be those embedded in blocks where drupal_get_form($form_id) is called before the search_view() function is executed.

Taking a similar approach for any other forms embedded anywhere in the page (including search results), it may be possible to call drupal_get_form() in a hook such as hook_init(). e.g.:

function your_module_init() {  
  if(!empty($_POST['form_id']) && $_POST['form_id'] == 'your_module_form') {
    drupal_get_form('your_module_form');
  }
}

I suspect it depends on what you need to process the form submission as to whether (or where) you force it to be handled in a hook in this way.

Agileware’s picture

Version: 7.x-dev » 6.x-dev
Status: Closed (fixed) » Patch (to be ported)

What about a drupal 6 port of this (or else the early version like in #3)?

Agileware’s picture

For anyone who wants it here is the patch in #3 made with git against current 6.x-dev.

Albert Volkman’s picture

Status: Patch (to be ported) » Needs review
apaderno’s picture

Status: Needs review » Closed (fixed)

It's too late to port this to Drupal 6.

Albert Volkman’s picture

Status: Closed (fixed) » Needs review

This would only not be acceptable as a backport if it was an API change, correct? #3 doesn't affect the API.

karthikmca1987’s picture

#3: search_results_forms.patch queued for re-testing.

jhodgdon’s picture

Version: 6.x-dev » 7.x-dev
Status: Needs review » Fixed

I think that the patch that was committed to 7.x is too complex for us to port to 6.x at this point. We don't have a testing system really in 6 to make sure we don't break something, and this issue is kind of obscure anyway. I'm just going to set it back to 7.x/fixed.

Status: Fixed » Closed (fixed)

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