Hi,

I noticed something weird with Drupal 6.
If I have a form function -- say custom_search() -- attached to an URL, and this function has both "$page .= drupal_get_form('custom_search_form'); and, at the bottom, the query results, theme_pager will return a working pager... but with a LOT of stuff at the end of each URL. Basically, it dumps in the query string the _whole_ form:

/custom_search?name=u&op=Search&form_build_id=form-db3e01abb6baf06b6402ddb33ee72fad&form_token=f536fb6851c1c8b6e9a9f8575ddee1c8&form_id=custom_search_form" title="Go to page 1" class="active">1

Is this expected? How can I prevent it? (It makes URLs very messy, and it just "feels" wrong).

Bye,

Merc.

Comments

mercmobily’s picture

Hi,

Well, sort of.
I added:

    $old_REQUEST=$_REQUEST;
    unset($_REQUEST['form_build_id']);
    unset($_REQUEST['form_id']);
    unset($_REQUEST['form_token']);
    unset($_REQUEST['op']);
    $page .= theme('pager', NULL, 10,0);
    $_REQUEST=$old_REQUEST;

Now... _surely_ we are not meant to do this...?!?
This only happens when the pager is called in a page which is the direct result of a form. Maybe the pager should be smart enough to filter that stuff out...?

Merc.

pobster’s picture

Well I think that's unexpected and I do quite a lot of work with paging, why don't you post up the function and I'll have a poke at it.

Pobster

mercmobily’s picture

Hi,

Thank you for your answer!
I hope I'll manage to make your life as easy as possible on this one...
Just make a module called "custom". this is "custom.module":

BTW, if I *did* happen to do things right (you never know), this is a working snipset to create a multipage filtered form...

function custom_menu() {

  $items['custom_search'] = array(
      'title' => t('Easy search'),
      'page callback' => 'custom_search',
      'access arguments' => array('access content'),
      'type' => MENU_CALLBACK,
    );

  return $items;
}

function custom_search(){

  // NOTE 3: This is ALSO ugly. This is because I don't actually know
  // where the 'name' variable is coming from. It could be GET (from the
  // pager) or POST (from the form)

  // Sets 'name'
  if($_POST['name'] ){
    $name=$_POST['name'];
  } else {
    $name=$_GET['name'];
  }

  $page .= drupal_get_form('custom_search_form');

 $result = pager_query("SELECT * FROM {variable} WHERE name like '%%%s%%'", 10, 0,NULL, $name);
  #drupal_set_message("Filtered by name: $name");
  if($result){
    while ($item = db_fetch_object($result)) {
    $page .= "<li>$item->name  -- $item->value</li>";
    }

    // NOTE 2: UGLY BIT. Without this, this module *works*. However,
    // the pager will have the whole form's worth of crap in the
    // query string...
    $old_REQUEST=$_REQUEST;
    unset($_REQUEST['form_build_id']);
    unset($_REQUEST['form_id']);
    unset($_REQUEST['form_token']);
    unset($_REQUEST['op']);
    $page .= theme('pager', NULL, 10,0);

   $_REQUEST=$old_REQUEST;

  }
  return $page;

}

function custom_search_form(){

  #drupal_set_message("Test: ".$_GET['name'].", ".$_POST['name']);

  // NOTE 4: Another source of ugliness. If the page is rendered from the
  // pager, I want the fields to be set...
  if($_GET['name']){
    $default['name']=$_GET['name'];
  }

 $form['name'] = array(
    '#type' => 'textfield',
    '#size' => 35,
   '#default_value' => $default['name'],
    '#maxlength' => 128,
    '#title' => t('The text on the user\'s tab.'),
    );

  $form['submit'] = array(
    '#type' => 'button',
    '#value' => t('Search'),
  );

  // NOTE 1
  // This needs to stay here so that if you go to page 2, for example,
  // and then start a new search from the form, you don't keep all of
  // the rubbish in the query string
  $form['#action'] = url('custom_search');

  return $form;
}

pobster’s picture

Hmmm... Well I can't install it myself just yet as I'm working, but I can see where you've gone wrong in places (with regard to the 'ugliness'), try to follow more the examples given on the developers site; http://cvs.drupal.org/viewvc.py/drupal/contributions/docs/developer/exam...

And I reckon that you're getting the full form variables in the URL because you're calling pagination AFTER you've called drupal_get_form. The pagination should be done IN the form not in a separate place (it should make more sense if you look at the multipage example, even if it is vastly different from what you're trying to achieve).

...Incidentally... The code is a bit of a security risk... You're taking a GET variable and directly using it in a db query - a malicious user could easily craft a url which could say, close that db query early and then do *anything* to your db... Drop it, alter it... Yikes, anything... So please be careful!

If you do need help with rewriting, let me know and I'll help out when I get a minute. Couple of things to note; there's no need to use either GET or POST, the form API allows you to access $form in the same way when you declare it in your function. Also, as I mentioned above - don't forget that each form has a form id attached to it so Drupal can securely match it between submits (to check that they match and no tomfoolery is afoot) hence you can only have one form per page. Realising this will make it easier to understand visibilities between multiform pages.

edit: Just thought, I've written a property (real estate) module before which does exactly what you're trying to achieve, I'll see if I still have a copy of it anywhere...
edit #2: Me again ;o) This is perhaps a better example; http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/tablemanage... (than the multipage form example I mean).

Pobster

mercmobily’s picture

Hi,

I can see what you mean by saying "the paging should be *within* the form. It's interesting!

About the security risk: I am not actually using the "name" variable directly. I used the mysql escape system -- which should mean that nothing nasty can actually happen...

I would be _immensely_ grateful if you could find the time to dump a (correct) rewrite of the module above here -- for me, and anybody else reading...

THANK YOU!

Merc.

pobster’s picture

Oops, apologies I forgot all about this... I'll have a go right now...

Edit... Okay, I've bad news! The form I used in my real estate module works a lot differently to how I remembered... (It's Drupal 4.7 as well, so it's vastly different anyway) So I tried writing my own code which... Suffers from the same problem! I'll get back to you on this...

Pobster

mercmobily’s picture

Hi,

Thanks a million.
At this point, I will make absolute sure I will write a handbook page about this when it's resolved properly. That way, many will benefit from your answer!

Bye,

Merc.

pobster’s picture

Well I can't believe that I've never come across the need to do this before! This is proving to be quite a challenge, and I've already found one Drupal issue page which says it's not possible... http://drupal.org/node/5371 but I'm sure there's a way round it...

I'm going to need to be working for a while, but I *will* get back to you on this - I love a challenge!

Pobster

mercmobily’s picture

Hi,

Well, it's _definitely_ possible, since I did it :-D
But a neater way would be indeed grand!

Thanks a lot,

Merc.

pobster’s picture

Well yes agreed, you did do it... But you've only really created a way round it rather than a solution - obviously it's better to not have the problem in the first place! I'm afraid I haven't done anything new with my code yet, but I thought I'd post it here in case anyone else is following this thread?

function custom_menu() {
  $items['custom_search'] = array(
    'title' => t('Easy search'),
    'page callback' => 'drupal_get_form',
    'page arguments' => array('custom_search'),
    'access arguments' => array('access content'),
    'type' => MENU_CALLBACK,
  );
  return $items;
}

function custom_search($form_values = NULL) {
  $form['name'] = array(
    '#type' => 'textfield',
    '#size' => 35,
    '#default_value' => $form_values['post']['name'] ? $form_values['post']['name'] : $_GET['name'],
    '#title' => t('The text on the user\'s tab'),
  );
  $form['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Search'),
  );

  $form['#redirect'] = FALSE;
  return $form;
}

function theme_custom_search($form) {
  $output = drupal_render($form);
  $search = $form['name']['#value'];
  if ($search) {
    $result = pager_query("SELECT * FROM {variable} WHERE name LIKE '%%%s%%'", 10, 0, NULL, $search);
    while ($item = db_fetch_object($result)) {
      $list[] = "$item->name  --  $item->value";
    }
    $output .= theme('item_list', $list);
    $output .= theme('pager', NULL, 10, 0);
  }
  return $output;
}

function custom_theme($existing, $type, $theme, $path) {
  return array(
    'custom_search' => array(
      'arguments' => array('form'),
    ),
  );
}

You'll probably need to clear your cache to use it, if indeed you want to use it as obviously it still suffers from the issue?! I originally tried just using; $form['pager'] = array('#value' => theme('pager', NULL, 10, 0)); in the form but decided it looks better to return the list within hook_theme (that's just a personal preference, I like to keep forms as literally just forms and put everything else in other functions).

edit: I did notice at one point, rather than get the full form_id, form_build_id and everything I was just returning eg. /custom_search?page=1&name=comment&op=Search but I can't seem to get back to that now!

Pobster

pobster’s picture

Apologies for bumping this, but having never come across this problem before (and not being able to find an immediate solution!) I too want to know the correct *Drupal* way to achieve paging within forms?

Pobster

mercmobily’s picture

Hi,

I second the request. I have a working way, although it's not exactly "clean".

Merc.