When a form with conditional fields attached is saved or previewed and the form is missing visible required fields, the conditional_fields_node_editing_form() function is not being called. This function seems like it has to be called since it is responsible for removing controlled fields that are required unless necessary and also including the Javascript to hide/show/disable the controlled fields.

My thought is that this should be able to be fixed in conditional_fields_form_alter() function at the CCK case's if statement:

if ($form_state['submitted'] == FALSE && empty($form_state['post']) || $form_state['submitted'] == TRUE && $form_state['node_preview']) {
  conditional_fields_node_editing_form($form);
}

When a form is previewed or submitted without the necessary required fields set, the form is redrawn and its state is:
$form_state['submitted'] = FALSE
$form_state['post'] = not empty
$form_state['post']['op'] = Save || Preview

This would change the conditional_fields_form_alter() function at the CCK case's if statement to:

if ($form_state['submitted'] == FALSE && empty($form_state['post']) || $form_state['submitted'] == TRUE && $form_state['node_preview'] || $form_state['submitted'] == FALSE && !empty($form_state['post']) && in_array($form_state['post']['op'], array('Save', 'Preview'))) {
  conditional_fields_node_editing_form($form);
}

This new if statement successfully handles the following form scenarios:
First time loaded (ie clicked on node/add/)
Form being previewed and all required fields present
Form being previewed/saved with required fields missing

Obviously, this if statement is becoming a little unruly, so if my suggestion is correct, this if may need to be rewritten or simplified.

Also, adding this third scenario to the if statement causes conditional_fields_add_js() to be called twice, once as a result of conditional_fields_form_alter(), and again at validation. Both times it is invoked using conditional_fields_add_js() so it seems to me that a global/static variable could be used to determine if the Javascript setting has already been included. I am not as familiar with the rest of the code and other places that conditional_fields_add_js() is called, so correct me if this can't be done.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

flapsjack’s picture

I have made my proposed modifications to conditional_fields.js and created a patch.

This adds both the additional if scenario and the static variable to drupal_add_js().

These changes seem to be working for me with no side-effects.

peterpoe’s picture

After a bit of testing, it turns out that form_alter is not called when a form fails validation. The simplest solution I've found is to move add_js to the validation function on preview.
I still don't understand why and when form_alter is called twice. Sometimes it happens.

In any case, I made a lot of changes to the code, fixing a bunch of bugs with the usual nasty required fields and with orphaned fields settings. You can apply this patch or download from cvs.

Thanks and Merry Christmas!

peterpoe’s picture

Status: Active » Needs review
flapsjack’s picture

peterpoe,
One problem that I am seeing in this patch that I was having before is the if statement in the conditional_fields_form_alter() hook.

<?php
// When previewing, the form is sometimes called twice, and this is bad for drupal_add_js. See issue #350411
if (($form_state['submitted'] == FALSE && empty($form_state['post'])) || $form_state['node_preview']) {
  conditional_fields_node_editing_form($form, $form_state);
}
?>

This if only handles the form in its initial creation state and successful preview state. When the form's "Save" or "Preview" button is clicked and the form is in an erroneous state (ie required fields missing), $form_state looks like the following:

$form_state = array(
  'storage' => NULL,
  'submitted' => FALSE,
  'post' => <post values array>,
)

It seems to me that $form_state['node_preview'] is only set when the form has no errors and the "Preview" button was clicked.

Am I misunderstanding or missing something?

peterpoe’s picture

So, summarizing, this is what happens in hook_form_alter:

When opening the page:
submitted => FALSE
post => empty array

When clicking on preview AND form validates:
submitted => TRUE
usual filled arrays => values, clicked button, #field_info, node, node_preview

When clicking on save or pressing enter AND form fails validation:
form_alter is not called
cf fails

When clicking on preview AND form fails validation:
form_alter is not called

The solution was simple: add a static variable to conditional_fields_add_js to avoid adding js twice on validation fail. But I didn't understand what was going on. The patch here should solve all problems.
Thanks flapsjack!

peterpoe’s picture

Status: Needs review » Fixed

Committed.

flapsjack’s picture

peterpoe:

Thanks for your time and effort on maintaining this module. I have been watching your commits and help across the issues and you are very active in this project. I know a lot of people are enjoying this module, including myself.

Now, I do have a question about your change to conditional_fields_add_js. Your new implementation of the function will only add the js once. This includes adding the conditional_fields.js and any settings. Since I am not as familiar with the code as you, is there anywhere else that conditional_fields_add_js is called a second time to add additional settings, or has the potential to be called a second time? I put a print statement at the top of the function and it has never printed more than once, so it seems like the function is only called once. But, if you ever have to add an additional setting and pass it to conditional_fields_add_js, it will not be added.

This may never be an issue, but I just wanted to make sure you were aware of it. Also, if you do find this behavior necessary at some point, my previous patch in this issue allows the function to continue adding settings, but only adding conditional_fields.js once.

Thanks again for all your work.

maxtorete’s picture

Now, when validation fails, the module works fine, thanks a lot!

j.slemmer’s picture

I'm trying this patch (I downloaded the new dev for d6).
Patch seems to work, on failed validation it shows the right fields / group.

BUT I get this error on the homepage:

    * warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\drupal\sites\all\modules\conditional_fields\conditional_fields.module on line 194.
    * warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\drupal\sites\all\modules\conditional_fields\conditional_fields.module on line 194.
    * warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\drupal\sites\all\modules\conditional_fields\conditional_fields.module on line 194.
    * warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\drupal\sites\all\modules\conditional_fields\conditional_fields.module on line 194.
    * warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\drupal\sites\all\modules\conditional_fields\conditional_fields.module on line 194.
    * warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\drupal\sites\all\modules\conditional_fields\conditional_fields.module on line 194.
    * warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\drupal\sites\all\modules\conditional_fields\conditional_fields.module on line 194.
    * warning: Invalid argument supplied for foreach() in D:\xampp\htdocs\drupal\sites\all\modules\conditional_fields\conditional_fields.module on line 194.
j.slemmer’s picture

I've just tried to make a fix by my self and I've come up with this:

I replaced the original code on line 194,

From:

foreach ($node->$field['control_field_name'] as $value) {

To:

foreach ($node->content[$field['control_field_name']] as $value) {

This makes the error's go away, can anyone verify that this fixes the issue and doesn't create any new?

Status: Fixed » Closed (fixed)

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

peterpoe’s picture

joygroup, your solution has the problem that we were not looking for the fields form definitions there, but for their values, which are stored under $form[FIELD_NAME]. There is a fixed versioni in dev now that should also solve this problem.

suydam’s picture

I'm running the Dec 6, 2009 6.x-1.x-dev version and it doesn't fix the validation issue.

When submitting a form that fails validation, all conditional fields expose themselves, rather than being hidden/showing based on user input.

suydam’s picture

Status: Closed (fixed) » Needs work

I've done some digging.
As far as I can tell, in my CCK node during node edit, conditional_fields_node_editing_form_validate() never gets called.
So the javascript never gets added to the page.

I checked the edit form (prior to submit) and the validation function is added:
http://screencast.com/t/MWU2N2Zk

But, when you submit the form, the validation function is never called:
http://screencast.com/t/YzEwMmVmMWEt

Any ideas?

suydam’s picture

Even more digging....


  • This error only happens during form_set_error on the node form DURING ADD.
    If you go back and edit an existing node, it's not an issue at all.
  • This error only occurs when you give anonymous users the permission to create nodes. I cannot get it to happen for non-anonymous users.
  • AND THE JACKPOT: This is a compatibility issue with Content Profile. My node type is to be used as the user profile.

Anyone have any ideas?

peterpoe’s picture

Status: Needs work » Closed (fixed)

Closing again.
#15: For compatibility issues, you can refer to the corresponding issue:
#619290: Make Conditional Fields compatible with Content Profile module

fourmi4x’s picture

Hi, on my installation with the latest dev of Conditional fields, I still have a critical bug:
- On form submission with validation errors, conditional fields are correctly hidden, however
- On form submission through the Enter key, all the conditional field appear.

I solved it temporarily by disabling the Enter key with Javascript, but it would be great to have the Enter key working!

mstef’s picture

This is definitely not fixed - just tested again in the latest 6.x-2.x-dev. I'm still experiencing this issue without using Content Profile for the given node type..

Solutions?

mstef’s picture

Status: Closed (fixed) » Active

Exactly the same results as #17. Using 6.x-2.x-dev.

mstef’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev
mstef’s picture

Issue seems to be at:

function conditional_fields_node_after_build($form, &$form_state) {
  // Avoid running twice when the form is rebuilt with AHAH
  if (!empty($form_state['clicked_button']['#ahah'])) {
    return $form;
  }

When pressing enter on the form to submit, this function bails out here.

I see the use in not rerunning this, but this is something that clearly needs a fix..

ferrum’s picture

unfortunately also not fixed for 6.x-2.0, same result as #17

AaronBauman’s picture

subscribe

stBorchert’s picture

Status: Active » Needs review
FileSize
1.71 KB

Here is a patch that fixes the js-issue for me. The javascript is only added once (even if you rebuild the form).

fourmi4x’s picture

Hi,
Thanks for your patch, I tested it but I can't see any difference, the bug is still there on my side.

EDIT : I thought the problem was appearing only with conditional fields triggered by checkboxes (and not radio buttons), but I've realized the problem is also there with radio buttons

peterpoe’s picture

I can't reproduce any of the issues:
#17: Tried to submit pressing enter (fresh install, cck 2.9, cf 2.0) and found no error.
#21: I don't understand how pressing enter on the form could trigger ahah behavior.
There is probably some issue with other installed modules, please provide more context like modules installed that act on the node form, field types and widgets used.

jlashomb’s picture

I've been having the same problem, and have found that it has something to do with an unlimited value user-reference field. The "Add another field" AHAH info seems to stick around when you press enter to submit, and this causes conditional_fields_node_after_build to immediately return the $form the one time that conditional_fields_node_after_build is called.

I don't know if this help narrow down the problem more, but I'm going to continue investigating, and post any new info I find.

peterpoe’s picture

Priority: Critical » Normal
FileSize
882 bytes

#24, while solving the problem with the enter key, introduced new issues with the Add more button (the form element was not displayed).
So I tried a different approach:
Instead of:

  if (!empty($form_state['clicked_button']['#ahah'])) {
    return $form;
  }

We use a static variable:

  static $run;
  if (!empty($run[$form['#build_id']])) {
    return $form;
  }
  $run[$form['#build_id']] = TRUE;

A less hacky approach would be better though...

(Downgrading Priority to normal since this bug happens only when the form is submitted with the enter key and fails validation)

peterpoe’s picture

Status: Needs review » Closed (outdated)