Any AHAH form (noticed while making new one), but double checked with previous ones, will fail when following these steps:

1. View form.
2. Use the ahah element that replaces part of form.
3. Submit form. (and receive validation errors)
4. Submit form again. (as if fixing errors)
5. Stare at the AHAH callback page with some nice JSON.

For some reason after step 3 the form action is set to the AHAH callback, and this was never done before 6.13 and below.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

boombatower’s picture

For an example of this you can use any AHAH form that allows those steps (or even ahah_helper_demo).

boombatower’s picture

Status: Active » Needs review

The following reverts the changes to form.inc which fixes the issue.

Index: includes/form.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/form.inc,v
retrieving revision 1.265.2.28
diff -u -r1.265.2.28 form.inc
--- includes/form.inc	16 Sep 2009 17:54:19 -0000	1.265.2.28
+++ includes/form.inc	30 Sep 2009 02:41:46 -0000
@@ -1,5 +1,5 @@
 <?php
-// $Id: form.inc,v 1.265.2.28 2009/09/16 17:54:19 goba Exp $
+// $Id: form.inc,v 1.265.2.25 2009/05/26 08:18:46 goba Exp $
 
 /**
  * @defgroup forms Form builder functions
@@ -119,8 +119,9 @@
     drupal_process_form($form_id, $form, $form_state);
     if ($cacheable && !empty($form['#cache'])) {
       // Caching is done past drupal_process_form so #process callbacks can
-      // set #cache.
-      form_set_cache($form_build_id, $original_form, $form_state);
+      // set #cache. By not sending the form state, we avoid storing
+      // $form_state['storage'].
+      form_set_cache($form_build_id, $original_form, NULL);
     }
   }
 
@@ -131,14 +132,14 @@
   // the form will simply be re-rendered with the values still in its
   // fields.
   //
-  // If $form_state['storage'] or $form_state['rebuild'] has been set
-  // and the form has been submitted, we know that we're in a complex
-  // multi-part process of some sort and the form's workflow is NOT 
-  // complete. We need to construct a fresh copy of the form, passing
-  // in the latest $form_state in addition to any other variables passed
-  // into drupal_get_form().
+  // If $form_state['storage'] or $form_state['rebuild'] have been
+  // set by any submit or validate handlers, however, we know that
+  // we're in a complex multi-part process of some sort and the form's
+  // workflow is NOT complete. We need to construct a fresh copy of
+  // the form, passing in the latest $form_state in addition to any
+  // other variables passed into drupal_get_form().
 
-  if ((!empty($form_state['storage']) || !empty($form_state['rebuild'])) && !empty($form_state['submitted']) && !form_get_errors()) {
+  if (!empty($form_state['rebuild']) || !empty($form_state['storage'])) {
     $form = drupal_rebuild_form($form_id, $form_state, $args);
   }
 
@@ -271,7 +272,6 @@
  *   in here when it is called.
  * For example:
  *
- * @code
  * // register a new user
  * $form_state = array();
  * $form_state['values']['name'] = 'robo-user';
@@ -289,14 +289,9 @@
  * $form_state['values']['name'] = 'robo-user';
  * $form_state['values']['op'] = t('Save');
  * drupal_execute('story_node_form', $form_state, (object)$node);
- * @endcode
  */
 function drupal_execute($form_id, &$form_state) {
   $args = func_get_args();
-
-  // Make sure $form_state is passed around by reference.
-  $args[1] = &$form_state;
-  
   $form = call_user_func_array('drupal_retrieve_form', $args);
   $form['#post'] = $form_state['values'];
   drupal_prepare_form($form_id, $form, $form_state);
@@ -761,8 +756,8 @@
 
   foreach ($handlers as $function) {
     if (function_exists($function))  {
-      // Check to see if a previous _submit handler has set a batch, but 
-      // make sure we do not react to a batch that is already being processed 
+      // Check to see if a previous _submit handler has set a batch, but
+      // make sure we do not react to a batch that is already being processed
       // (for instance if a batch operation performs a drupal_execute()).
       if ($type == 'submit' && ($batch =& batch_get()) && !isset($batch['current_set'])) {
         // Some previous _submit handler has set a batch. We store the call

Assuming it has something to do with rebuilding the form when menu item is AHAH callback and thus sets form action to that URL.

Thus I assume it is:

-  if ((!empty($form_state['storage']) || !empty($form_state['rebuild'])) && !empty($form_state['submitted']) && !form_get_errors()) {
+  if (!empty($form_state['rebuild']) || !empty($form_state['storage'])) {
boombatower’s picture

Assigned: Unassigned » boombatower
FileSize
1.46 KB

I have confirmed my hunch. The patch fixes the issue.

Someone who is familiar with the change may want to double check the logic.

boombatower’s picture

grendzy’s picture

That button issue caused some other problems, #587254: D 6.14 breaks multipage forms & #584536: Multipage forms are not working. Should this be marked as a duplicate?

RoelandG’s picture

Status: Needs review » Closed (won't fix)

I tested this patch with a clean install of drupal 6.14, enabled the poll module and applied the patch.

Unfortunately the patch isn't working:
1. Fire an ajax request ("add another choise") while creating a new poll node with empty fields.
2. Receive form errors because no fields are filled with data.
3. Submit the main form

Now the action is changed to 'poll/js'. After the page is submitted again the page is logically redirect to this js callback path 'poll/js'.

Will try to figure out what is going wrong. Hope I find something and can submit a new solution.

stu1984’s picture

Am having this very problem with a form I am trying to create. Was not working in Drupal 6.14 so decided to move the form module I had made into a previous version of Drupal (6.12) and see if it would. Amazingly it worked fine with none of the issues I was previously having. As an experiment to see if I could recreate the error, I moved a copy of the form.inc file from my 6.14 includes into the 6.12 directory to see if boombatower's theory was correct. I found that the error reappeared straight away so there definitely was a problem with this file. Doing some file comparisons between the two versions of form.inc, I managed to track the problem down to the same lines of code as mentioned in post 2 above. The solution I found involved removing the NOT operator from form_get_errors(), so the line now reads:

if ((!empty($form_state['storage']) || !empty($form_state['rebuild'])) && !empty($form_state['submitted']) && form_get_errors()) {

However when I made these changes to the form.inc in my Drupal 6.14 directory, the problem remained. This makes me think that there is possibly another file in 6.14 that is also causing the same error. Does anyone else know which includes would be used when working with a form as I have not been using Drupal for that long so am not completely sure which includes would be used where? Hope this info helps someone diagnose this problem and find a solution.

sun’s picture

Status: Closed (won't fix) » Closed (duplicate)
dsnopek’s picture

Reverting the patch from #302240: button broken due to fix various problems when using form storage in comment #13 or using the patch from this issue comment #3, doesn't fix the problem for me. I suspect this problem is unrelated to those changes.

dsnopek’s picture

I just did the following procedure using Drupal 6.12, 6.13, 6.14, and 6.15:

  1. Enable the "Poll" module
  2. Go to Create Content -> Poll
  3. Enter a "Question" and the two "Choice" fields
  4. Click "Add Another Choice"
  5. Fill in the third "Choice" field
  6. Delete the text you put into the "Question" field and click the "Save" button.
  7. You will come back to the form with a validation error, saying "Question field is required".
  8. Fill it in again and click "Save".
  9. You will be presented with JSON.

Since this problem has existed in so many versions and since it is exhibited by a core module (ie. "Poll"), my feeling is that this is a problem that has always existed with AHAH in Drupal, and not something recently introduced.

It appears that it is the responsibility of the module author to handle $form['#action'] correctly. There is a patch to ahah_helper to do this automatically (#578252: $form["#action"] gets set to /ahah_helper/path (with patch to fix!) comment #3) to assist with this -- although, I haven't tried it yet. Even if the current patch doesn't work, it seems like trying to get some reusable helper code to handle the $form['#action'] problem is the right way to go.

tbazadaykin’s picture

micke_b’s picture

I had the same problem but found a fix that has been used for the ahah_helper module(same as above):

http://drupal.org/node/578252

The fix in Post #10 (http://drupal.org/files/issues/ahah_helper-form-action_0.patch) on that page solved this problem for me.

I just used the fix in my _form hook AFTER I turn on the cache:

....._form()

...code...

$form = array(
'#cache' => TRUE,
);

// FIX goes HERE

...code...

hope it helps
/Micke

pembeci’s picture

Version: 6.14 » 6.15
Assigned: boombatower » Unassigned
Status: Closed (duplicate) » Active
Issue tags: +ahah, +form validation

#12 : Micke, which version of Drupal are you using? I tried what you suggested at 6.15 and it didn't work.

I don't think this is a duplicate of #302240: button broken due to fix various problems when using form storage . As dsnopek said, applying the last patch there or #3 here didn't help. Considering the latest comments, it seems we still don't have a general solution yet. So I am updating the status.

pembeci’s picture

Form workflow across page requests book page can help to track this problem. It explains what happens after an ahah call is made.

blackdog’s picture

Version: 6.15 » 6.16

This still seems to be an issue with Drupal 6.16.

It doesn't appear every time, but when it first appears the form is broken, and you have to start over to submit it.

Scenario:

We've build a multistep form with some AHAH fields, and when abusing the form, to get errors, and then submitting again, JSON data is shown.

blackdog’s picture

Priority: Normal » Critical

This still exists.

Flow:

Loads form, enters some values.
Loads more fields with AHAH.
Tries to submit, but validation throws errors (with form_set_error)
Fixes the error(s), hits Submit again, raw JSON data is outputted.

Small update:

It only seem to hit when the form errors is with the AHAH fields, not when other fields throws errors.

aaron’s picture

subscribe

sbefort’s picture

subscribe

sbefort’s picture

AHAH is obviously not grown up. Cheers to writing the raw jQuery.

fizk’s picture

Please fix!

I don't understand how this goes unfixed for so long....so few people must be using AHAH.

GuyPaddock’s picture

Also having this issue...

jbourke’s picture

Subscribe - Having same exact issue.

codeglyph’s picture

Yes, i'm having this issue too... yet another bug in the freaking a-hah! implementation :(

One simple yet dirty workaround seems to be inserting the following code in your hook_form_alter() function

function mymodule_form_alter(&$form, $form_state, $form_id) {
$form['#action']=' ';
}

This resets the form action to current page. Would reflect on the page in a <form action =" " > form attribute. Wonder what inept browsers will do with that...

Anyway this is not a long-term solution.

If you need more serious processing you can use some uri generation based on the form's #nid information. Like on this page:
http://www.netaustin.net/2009/01/26/dynamic-form-altering-using-ahah-rig...

Damien Tournoud’s picture

Title: 6.14 broke AHAH with form validation » Problem with various AHAH forms and validation
Version: 6.16 » 6.x-dev

Cannot reproduce in 7.x, so this only affects 6.x for some reason.

sun’s picture

Status: Active » Postponed (maintainer needs more info)

This issue was definitely fixed in #302240: button broken due to fix various problems when using form storage for D7. The patch for D6 over there should have fixed this bug for D6, too. Not sure whether recent test reports here were actually against that latest D6 code, past March 2, 2010.

The last detailed test report seems to be #10, dates back to January 2010.

joep.hendrix’s picture

The problem still exists in D6.17

joep.hendrix’s picture

#23
That workes for me! Thanks!

noobee’s picture

Thank you so much for posting your patch.

I ran into EXACTLY this same issue, and it's been driving me bonkers. I figured it must be something like this, but don't know enough about the internal flow of forms to do anything about it.

Silly question: one applies a patch using "ed", or something else? It's been ages (maybe 15 years?) since I did one. (Started using Drupal recently, obviously.)

Thanks!

alpritt’s picture

Status: Postponed (maintainer needs more info) » Active

Have confirmed this is still an issue with a fresh Drupal 6 HEAD, using the same test case as in #10.

Here's what is happening:

1. The AHAH enabled button is pressed, which requests the menu path 'poll/js'. The callback for that path is poll_choice_js().
2. poll_choice_js() contains this line:

$form = drupal_rebuild_form($form_id, $form_state, $args, $form_build_id);

When the form is rebuilt, the $form['#action'] is set to the current URI in system_elements() (an implementation of hook_elements()). Since the current path is now 'poll/js' that's what $form['#action'] gets set to.

3. The rebuilt form is saved to the cache.
4. The main form is submitted and the form is retrieved from the newly built cache complete with the now incorrect $form['#action'] value.
5. Since there is a validation error the form gets rendered and printed with the validation error message and the now incorrect form action.
5. The user submits the form. Since 'poll/js' is the form action poll_choice_js() is called which processes the POST data and returns JSON to the browser.

larowlan’s picture

For what it's worth this behaviour still occurs in Drupal 6.17 but can be avoided if you explicitly set an action on your forms.
Eg

$form = array(
  '#cache' => TRUE,
  '#action' => url('some/path/here')
);

If you leave it up to Drupal to set the action, you get the validation error/issue as described above.

mauritsl’s picture

I can confirm that this is still an issue, even in 6.19.

The workaround in the last comment works fine, thanks for sharing.

DrupalCuckoo’s picture

Hi,

I'm using the core poll module to create poll nodes.

Where do I have to put this code?
Is a custom module with a hook_form_alter function needed?
What should be entered as path?

thanks for any help.

best regards.

DrupalCuckoo’s picture

hi,

I've an answer to my own questions ;) This is what I did:

Since I already have a "helper module" where I collect all kind of different functions to manipulate drupal, I've added a hook_form_alter function:

/**
 * Implementation of hook_form_alter()
 */
function helper_module_form_alter(&$form, &$form_state, $form_id) {
  
  $node_form_id = $form['type']['#value'].'_node_form';
  
  switch($form_id) {

    case $node_form_id:
      if(module_exists('poll') && ($node_form_id == 'poll_node_form')) {
        $form['#action'] = $form['nid']['#value'] ? url('node/'.$form['nid']['#value'].'/edit') : url('node/add/poll');
      }
    break;
    
    default: break;
    
  }
  
}

I chose this way because ohterwise I'd have to alter the original module code and that's never a clean solution.

Would this be considered as a good solution?

chris.jichen’s picture

Although it is a hack, but I tried this, it actually works.
Thanks.

lizac’s picture

Hi! My problem is somewhat related to this but I can't just add more options to Drupal Poll. and I'm getting this error
http://drupal.org/node/630000#comment-3483566

Help, please! =(

larowlan’s picture

Hi
FYI it's not really a hack, the ahah js looks for the form action before doing the ahah request, after the request it restores the original action. If there's no action there, it can't do anything so this bug occurs.
Lee

jpstrikesback’s picture

I found that after multiple AHAH "moments" & validates even after setting #action it could get lost a second time.

If anyone else experiences this the following patch for ahah helper get's around this issue:

see: http://drupal.org/node/578252#comment-2536114

like so:

   $form['#cache'] = TRUE;

  // When not set.. try to remember the old action OR set one up in storage
  if (!isset($form_state['storage']['old_action'])) {
    $form_state['storage']['old_action'] = isset($form['#action']) ? $form['#action'] : $_SERVER['REQUEST_URI'];
  }
  // If the form action change, restore the old one.
  if (isset($form_state['storage']['old_action']) && ($form['#action']!=$form_state['storage']['old_action'])) {
    $form['#action'] = $form_state['storage']['old_action'];
  }
gpeng’s picture

this is still not working for me....

mmilano’s picture

comment #30 fixed it for me. Thanks.

tom.pauwaert’s picture

subscribed :)

asghar’s picture

Hi

Guys , I think it is more helpful link

Thanks

webchick’s picture

Subscribing. This is still an issue in 6.x, with steps to reproduce in #29, and workaround in #30. But it would be nice if core didn't require special-casing here, as lots of people are repeatedly smacking into this wall.

http://jbenner.net/blog/prevent-ahah-the-right-way-from-breaking-with-va... (as mentioned before) contains a detailed analysis.

Thanks to deviantintegral for a pointer to both this issue and that blog post.

deviantintegral’s picture

Subscribing. poll.module is still broken in 6.x-dev.

davepoon’s picture

Subscribing.

anil_89’s picture

Title: Problem with various AHAH forms and validation » whan i am change country and display state accourding to country useing by ahah than ahah change form action
Priority: Critical » Major

function get_zone() {
global $user;
$form_state = array('storage' => NULL, 'submitted' => FALSE);
$form_build_id = $_POST['form_build_id'];
// Get the form from the cache.
$form = form_get_cache($form_build_id, $form_state);
//$form['#parameters'][0]['action']= $form['#action'];
$args = $form['#parameters'];
$form_id = array_shift($args);
// We will run some of the submit handlers so we need to disable redirecting.
$form_state['post'] = $form['#post'] = $_POST;
$form['#programmed'] = $form['#redirect'] = FALSE;
// We need to process the form, prepare for that by setting a few internals
// variables.
$form['#post'] = $_POST;
$form['#programmed'] = FALSE;
$form['#submit'] = FALSE;
$form_state['post'] = $_POST;
$form_state['action'] = $form['#action']=FALSE;
$form = drupal_rebuild_form($form_id, $form_state, $args, $form_build_id);
// Render the new output.
if($user->uid){
$choice_form = $form['Contact']['profile_state'];
}else{
$choice_form = $form['account']['location']['profile_state'];
}
unset($choice_form['#prefix'], $choice_form['#suffix']); // Prevent duplicate wrappers.
$output = drupal_render($choice_form);
drupal_json(array('status' => false, 'data' => $output));
}

and ahah menu is

$items['zone/js'] = array(
'title' => 'Zone',
'page callback' => 'get_zone',
'access arguments' => array('access content'),
'type' => MENU_CALLBACK
);

please give idea how to i solve

jpstrikesback’s picture

Title: whan i am change country and display state accourding to country useing by ahah than ahah change form action » Problem with various AHAH forms and validation
dwigglesworth’s picture

comment #30 fixed it for me - thanks.

StefTM’s picture

Warning: Parameter 2 to webform_client_form() expected to be a reference .../includes/form.inc on line 366

It happened after migrating the website to a new webserver with PHP 5.3.

I had the same problem and after reading lots of comments and possible solutions, I just updated *everything* at the latest version (both manually and with the automated updates).

I also had to disable some old modules that were not compatible with my Drupal version.

Now all seems fine!

larowlan’s picture

StefTM, your error is unrelated to this issue.
If your error persists, please create a new issue for webform tagged as PHP 5.3 Compatibility.

neels’s picture

http://api.drupal.org/api/drupal/modules--upload--upload.module/function...

I have a question: Is this possibly related to the upload module in that particular function? What #action would I put in to fix it?

yang_yi_cn’s picture

experiencing this too! subscribing

iva2k’s picture

Status: Active » Needs review
FileSize
856 bytes

Wow, I'm amazed how long it has been an issue (2 years!).

@Drupal core team, can you spend few minutes fixing it for good? You will close so many painful and lengthy bugs across so many modules.

I propose a 2-line fix that does the following:
1. stash form['#action'] into form_state[], best place seems to be drupal_process_form() as it is supposed to store what's worthy of preserving in form_state, and it is part of AHAH flow.
2. restore form['#action'] from form_state[] (if it has been stored but has not been set in the form) in drupal_rebuild_form() right before the call to drupal_prepare_form()

This will avoid auto-generated value of form['#action'] coming from system_elements()/request_uri() if the form had a cached version. It eliminates a need for any workarounds with fixed paths and workarounds in hook_form and ahah callbacks, however any such workarounds will continue to work.

Here's a patch that I tested on Drupal 6.22.

Please review and commit!

Status: Needs review » Needs work

The last submitted patch, form.inc_.ahah591696.patch, failed testing.

iva2k’s picture

FileSize
956 bytes

I haven't done core patches yet. Don't have git checkout of Drupal tree, any hints of how to do it with diff? Retrying from drupal root directory.

iva2k’s picture

FileSize
956 bytes
iva2k’s picture

Status: Needs work » Needs review
FileSize
956 bytes

Status: Needs review » Needs work

The last submitted patch, form.inc_.ahah591696-1.patch, failed testing.

iva2k’s picture

Status: Needs work » Needs review
FileSize
3.09 KB

Ok, got git and made git diff.

lifer’s picture

All hunks fail when I try to apply the #58 patch.
I have tried from root and from AHAH-folder.
Maybe I am using another Drupal (6.24) or AHAH (6.x-2.2) version?

I have tried applying the patch by careful manual editing the form.inc.
No errors (so I lean against the assumption that I did it correctly), but it didn't do the trick, although it might do it for some.

I then tried to follow the link/guide provided by #42 and it looks like that solves the problem:

http://jbenner.net/blog/prevent-ahah-the-right-way-from-breaking-with-va... (as mentioned before) contains a detailed analysis.

Thanks!! :)

I wonder why there has been no AHAH module update with this (major) bug fixed - looking at the dates not even a dev-version!

czigor’s picture

I can confirm #59 except that the patch applied cleanly to Drupal 6.26.

patch -p1 <form_inc_ahah_validate_problem-591696-58.patch 

The patch in itself does not solve the problem, you need the

  $form_state['action'] = $form['#action'];

line in your ahah callback as stated in the link above.

Thanks iva2k and lifer!

iva2k’s picture

Interesting that the patch works for me without changing ahah callbacks. That is the whole idea of the patch - I've read the referenced post which gave me enough hints to understand what's going on and then I dug in and fixed it in core.

@czigor and @lifer
Your observation:
>The patch in itself does not solve the problem, you need the ... line in your ahah callback
perhaps applies to very different ahah sequence than the one I used as a test case. Can you post a step-by-step how you checked if patch solves the problem (including which ahah-enabled module to use)?

czigor’s picture

I was testing a custom ahah form which is based on the examples module's autocheckboxes ahah form example. Your patch made it live one submission longer compared to the sequence in the issue summary. That is:

1. View form.
2. Use the ahah element that replaces part of form.
3. Submit form. (and receive validation errors)
4. Submit form again. (as if fixing errors)
5. Submit form again. <- without patch this step is skipped
6. Stare at the AHAH callback page with some nice JSON.

Fergie’s picture

Thank you, codeglyph. That solved the problem.

anil_89’s picture

Status: Needs review » Fixed
fizk’s picture

Status: Fixed » Needs review

Not sure if any code has been committed to fix this issue.

rsharkey’s picture

I am using Drupal 6.19 and #30 fixed my issue.

I placed this code at the top of my form function in my module:

$form = array(
	  '#cache' => TRUE,
	  '#action' => url($base_url.'/partners/application')
);

Bear in mind that if you have already declared your "$form" array object in your form function, then this will be overwritten, since you are re-declaring the $form object. Even after placing this snippet at the top of the function, it did not work, since I had another declaration (further down the function) where I was declaring the $form object:

$form = array();

I just deleted this second piece and all worked as desired.

I hope this helps someone else out.

Thanks to #30 for pointing me in the correct direction.

petew’s picture

I couldn't find a general fix to this issue and it doesn't appear to have yet been fixed in core as yet, and I assume is unlikely to now. So in case it helps others...

I've only found fixes to solve the issue in specific situations, typically where you're creating your own form and are happy to hardcode the exact URL, but not a general fix to solve the problem. Apologies if someone has actually posted something that does this already and I somehow missed it. Plus i'm sure there's a few different ways to solve this issue.

In case it helps anyone, a simple general fix seems to be to create hook_form_alter in your own module, with the following code:

  $form['form_default_action'] = array(
    '#type' => 'value',
    '#value' => isset($form_state['values']['form_default_action']) ? $form_state['values']['form_default_action'] : $form['#action'],
  );
  $form['#action'] = $form['form_default_action']['#value'];

This approach simply stores the initial value, keeps that same value, and restores it in the form #action each time. (the #action value is initialised by the system module in its hook_elements for the form. We then just grab that on initial load and ensure the form keeps using it.)

Theoretically this should solve the problem for all forms, but no doubt there may be specific situations where this won't work, so YMMV.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.

apaderno’s picture

Priority: Major » Normal
Status: Fixed » Closed (outdated)