If within code processing AJAX request I set default value of checkboxes this default value is ignored by forms system and all checkboxes are instead created as unchecked.

I tracked down the issue to its origin and provide patch fixing this issue.

Files: 
CommentFileSizeAuthor
#17 1100170-17.patch1.02 KBjibran
FAILED: [[SimpleTest]]: [MySQL] 49,390 pass(es), 2 fail(s), and 1,362 exception(s).
[ View ]
#10 form_process_checkboxes_default_value_ajax_fix-1100170-10.patch704 bytestimos
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch form_process_checkboxes_default_value_ajax_fix-1100170-10.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#5 form_process_checkboxes_default_value_ajax_fix-1100170-5.patch663 bytesDanylo Dragan
PASSED: [[SimpleTest]]: [MySQL] 36,054 pass(es).
[ View ]
form_process_checkboxes_default_value_ajax_fix.patch605 bytesDanylo Dragan
PASSED: [[SimpleTest]]: [MySQL] 31,539 pass(es).
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, form_process_checkboxes_default_value_ajax_fix.patch, failed testing.

Danylo Dragan’s picture

Status:Needs work» Needs review
Danylo Dragan’s picture

Version:7.0» 7.2
Priority:Normal» Major

This bug still exists in Drupal 7.2. The patch I originally provided applies perfectly to Drupal 7.2 and fixes this issue. I raise priority to major for this bugfix to be included in the upcoming Drupal 7.3. Please review this bug and accompanying fix. Thank you!

sun’s picture

Version:7.2» 8.x-dev
Priority:Major» Normal
Status:Needs review» Needs work
Issue tags:+needs backport to D7

Could you create a unified patch using git, please? See http://drupal.org/patch/create for more information.

Danylo Dragan’s picture

Version:8.x-dev» 7.x-dev
Priority:Normal» Major
Status:Needs work» Needs review
StatusFileSize
new663 bytes
PASSED: [[SimpleTest]]: [MySQL] 36,054 pass(es).
[ View ]

Here is unified patch using git for 7.x-dev branch. I raise priority to major in the hope that this bugfix will be included in the upcoming Drupal 7.9. Please review it. Thank you!

marcingy’s picture

Version:7.x-dev» 8.x-dev

Needs to be fixed in d8 first and then backported

marcingy’s picture

Priority:Major» Normal

And back to normal as well

timos’s picture

It has not been backported on D7 yet.
I tested (manually) the patch on a 7.9 and it seems work.

timos’s picture

Hello

After more use of my patched installation, i found at least one bug caused by the #5 patch, but, actually it seems come from node_reference module and not really from the patch.

If you use the references module and you create a new node reference field like that :

  • create a new field, node reference type and an autocomplete text field as widget and submit
  • on the field-settings edit form choose only on content type as referenceable and submit

So you get the global edit field form, where the 'content type referenceable' settings is displayed again, and there, all the content types are checked and not only this you selected in the first step.

If you uncheck all the content type but one and you submit. The setting is saved and if you test the field, only the content type checked can be referenced. But if you edit again, both field-settings form or global field edit form, all your content types are checked again.

This behaviour is due to the #5 patch (if the patch is not applied, the behaviour is ok)

So i digged down in the references module (because, it's the only form where i found this bug) and i think the problem comes from the way how the #default_value node reference field is built.
It is done in the node_reference.module line 82 :

<?php
$settings
= $field['settings'];

 

$form = array();
 
$form['referenceable_types'] = array(
   
'#type' => 'checkboxes',
   
'#title' => t('Content types that can be referenced'),
   
'#multiple' => TRUE,
   
'#default_value' => $settings['referenceable_types'],
   
'#options' => array_map('check_plain', node_type_get_names()),
  );
?>

I didn't manage to find where the $field['settings']['referenceable_types'] array is populated (I suppose it's populated by the field API) but the result can be see with the debug function :

<?php
'#default_value' =>
        array (
         
'atelier' => 'atelier',
         
'article' => 0,
         
'date' => 0,
         
'page' => 0,
         
'recette' => 0,
        ),
?>

The handler of default value for checkboxes is defined by the patch like that :

<?php
'#default_value' => isset($value[$key]) || (is_array($element['#default_value']) && in_array($key, $element['#default_value'])) ? $key : NULL,
?>

So in the situation i describe above the condition match and the default value for the checkbox element is set to a not NULL value and the render for checkbox element is checked.

So, the problem comes from the node references module and i posted an issue for that (http://drupal.org/node/1365412) , but maybe we have to anticipate this kind of error with just a little change :

<?php
'#default_value' => isset($value[$key]) || (is_array($element['#default_value']) && in_array($key, $element['#default_value']) && $element['#default_value'][$key] != 0) ? $key : NULL,
?>

where the condition doesn't match if the $key is in the array but $element['#default_value'][$key] equals to 0, and so the checkbox element is not checked when it is rendered.

I'll try to make a new patch with this little add.

Cheers

timos’s picture

StatusFileSize
new704 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch form_process_checkboxes_default_value_ajax_fix-1100170-10.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

here the patch with the little change mentioned above
thanks for review
cheers

Status:Needs review» Needs work

The last submitted patch, form_process_checkboxes_default_value_ajax_fix-1100170-10.patch, failed testing.

Damien Tournoud’s picture

Issue tags:+Needs tests

This might be a legit bug, but without a test to reproduce it, this patch cannot be accepted.

The solution looks like a hack at first sight, so the real issue might be elsewhere, but anyway, we cannot proceed without a test to reproduce it.

timos’s picture

Status:Needs work» Needs review

Hi

Sorry i just see this issue is for D8 (actually i saw it, but as the #5 patch is for D7, i go on in this way).

As, this issue is still in the D7 version, how can i do to test this patch for D7 ? do i have to create a new issue for D7 or just change the version for tag for this issue to submit the patch, then port again the issue to D8 ?

timos’s picture

hefox’s picture

Status:Needs review» Needs work

In an ajax callback, the current values of element, i.e. the $form_state['input'], overrides the default value, so if the element already exists in the form and has input, so changing #default_value doesn't matter. See _form_builder_handle_input_element

So is this a bug or by design?

joachim’s picture

> So is this a bug or by design?

If it's by design it really needs documenting, as it's a real headdesker.

jibran’s picture

StatusFileSize
new1.02 KB
FAILED: [[SimpleTest]]: [MySQL] 49,390 pass(es), 2 fail(s), and 1,362 exception(s).
[ View ]

Patch from #10 re-rolled against d8.

jibran’s picture

Status:Needs work» Needs review
joachim’s picture

Thinking more about this I reckon it's by design.

If you display a form element to the user, and they enter data, and then it's AJAXed in some way, the user is going to expect that their input is not lost.

Obviously, there will be particular cases where you *do* want that to happen -- perhaps the AJAX callback is meant to process and modify the user's input, or there's just some sort of good reason to replace it.

But the basic behaviour seems right to me. Though it would be good to get input on this from the FormAPI expects.

Also, either way, it needs documenting.

Status:Needs review» Needs work

The last submitted patch, 1100170-17.patch, failed testing.

Chris Gillis’s picture

How can it be by design when the checkboxes element (or I assume this might be all multiple-value elements) is handled differently from other elements? I have a simple form that looks like:

<?php
  $form
['role'] = array(
     
'#type' => 'select',
     
'#title' => t('Role'),
     
'#options' => $roles_options,
     
'#default_value' => $role_selected,
  );
 
$form['dates'] = array(
     
'#type' => 'checkboxes',
     
'#title' => t('Dates'),
     
'#options' => $dates_options,
     
'#default_value' => $dates_selected,
  );
?>

When the form is rebuit with AJAX, the role field shows the computed value, but not the dates field? Inconsistency like that is clearly a bug to be fixed...

Chris Gillis’s picture

A simple workaround is to unset the input in form state. So in my example I would simply add:

<?php
unset($form_state['input']['dates']);
?>
joachim’s picture

> How can it be by design when the checkboxes element (or I assume this might be all multiple-value elements) is handled differently from other elements

Hmm, that's a very good point!

juanjo_dv’s picture

Issue summary:View changes

On my case, I made an ajax callback which adds options to a checkboxes list, I want those new options be checked by default. I understand the design behavior, but I think it's necessary some way to archieve the other case.

marcingy’s picture

@juanjo_dv you should be able to achieve that with https://api.drupal.org/api/drupal/developer%21topics%21forms_api_referen...

varghese’s picture

function fees_form_alter(&$form, &$form_state, $form_id) {
switch ($form_id) {
case 'fees_node_form':
$form['title'] = NULL;
$class_name[0] = 'Select';
$class_name= $class_name+get_class_list();
$form['field_common_class_id']['und']['#options'] = $class_name;
break;
case 'fees_paid_node_form':
$form['title'] = NULL;
$query = 'select n.nid,f.field_fees_name_value,c.field_common_class_id_value from node n
JOIN field_data_field_fees_name f ON n.nid = f.entity_id
JOIN field_data_field_common_class_id c ON n.nid = c.entity_id
JOIN field_data_field_status s ON n.nid = s.entity_id and s.field_status_value = 1';
$result = db_query($query);
$fees_list[] = 'Select';
foreach ($result as $record) {
$fees_list[$record->nid] = $record->field_fees_name_value;
}
$form['field_common_fees_id']['und']['#options'] = $fees_list;

$form['field_common_fees_id']['und']['#ajax'] = array('event' => 'change', 'callback' => 'ajax_student_list_fees_callback','wrapper' => 'student-list');
$form['field_common_student_user_id']['und']['#prefix'] = '';
$form['field_common_student_user_id']['und']['#suffix'] = '';
$form['field_common_student_user_id']['und']['#type'] = 'checkboxes';
// $form['field_common_student_user_id']['und']['#default_value'] = $selected;

$selected = isset($form_state['values']['field_common_fees_id']) ? $form_state['values']['field_common_fees_id']['und'][0]['value'] : 0;

if(isset($selected) && $selected > 0) {
$options = _ajax_student_list_fees_callback($selected);
$form['field_common_student_user_id']['und']['#default_value'] = array(115,121);
$form['field_common_student_user_id']['und']['#options'] = $options;
}
break;
}
return $form;
}

function ajax_student_list_fees_callback($form, $form_state){
return render($form['field_common_student_user_id']['und']);
}

But fees is not checked. May I know what I did wrong here?

GroovyCarrot’s picture

It's not ideal, but #22 was the only solution which worked for me in the form callback on core 7.38.

<?php
unset($form_state['input']['form_element']);
$form['form_element'] = array(
   
'#type' => 'checkboxes',
    ...
);
?>