From 0c993fd704dab80a845602a878bd612e9216b137 Mon Sep 17 00:00:00 2001 From: Angie Byron <webchick@24967.no-reply.drupal.org> Date: Sat, 10 Jul 2010 00:03:37 +0000 Subject: [PATCH] #776956 by CrashTest_, Damien Tournoud, effulgentsia, chx: Fixed critical bug: Complex widgets are not respecting '#disabled' state. --- includes/form.inc | 26 ++++++++---------- modules/file/file.js | 21 ++++++++------- modules/simpletest/tests/form.test | 33 +++++++++++++++++------ modules/simpletest/tests/form_test.module | 13 +++++++++ 4 files changed, 60 insertions(+), 33 deletions(-) diff --git a/includes/form.inc b/includes/form.inc index b6732481eed7..b214b673c59e 100644 --- a/includes/form.inc +++ b/includes/form.inc @@ -1505,6 +1505,14 @@ function form_builder($form_id, $element, &$form_state) { $element[$key]['#access'] = FALSE; } + // Make child elements inherit their parent's #disabled and #allow_focus + // values unless they specify their own. + foreach (array('#disabled', '#allow_focus') as $property) { + if (isset($element[$property]) && !isset($element[$key][$property])) { + $element[$key][$property] = $element[$property]; + } + } + // Don't squash existing parents value. if (!isset($element[$key]['#parents'])) { // Check to see if a tree of child elements is present. If so, @@ -2522,12 +2530,8 @@ function form_process_radios($element) { '#attributes' => $element['#attributes'], '#parents' => $element['#parents'], '#id' => drupal_html_id('edit-' . implode('-', $parents_for_id)), + '#ajax' => isset($element['#ajax']) ? $element['#ajax'] : NULL, ); - foreach (array('#ajax', '#disabled', '#allow_focus') as $property) { - if (isset($element[$property])) { - $element[$key][$property] = $element[$property]; - } - } } } } @@ -2623,12 +2627,8 @@ function form_process_checkboxes($element) { '#return_value' => $key, '#default_value' => isset($value[$key]) ? $key : NULL, '#attributes' => $element['#attributes'], + '#ajax' => isset($element['#ajax']) ? $element['#ajax'] : NULL, ); - foreach (array('#ajax', '#disabled', '#allow_focus') as $property) { - if (isset($element[$property])) { - $element[$key][$property] = $element[$property]; - } - } } } } @@ -2808,13 +2808,9 @@ function form_process_tableselect($element) { '#attributes' => $element['#attributes'], '#parents' => $element['#parents'], '#id' => drupal_html_id('edit-' . implode('-', $parents_for_id)), + '#ajax' => isset($element['#ajax']) ? $element['#ajax'] : NULL, ); } - foreach (array('#ajax', '#disabled', '#allow_focus') as $property) { - if (isset($element[$property])) { - $element[$key][$property] = $element[$property]; - } - } } } } diff --git a/modules/file/file.js b/modules/file/file.js index efea0ec07852..7325229b3dd9 100644 --- a/modules/file/file.js +++ b/modules/file/file.js @@ -92,17 +92,18 @@ Drupal.file = Drupal.file || { $enabledFields = $(this).parents('div.form-managed-file').find('input.form-file'); } - var $disabledFields = $('div.form-managed-file input.form-file').not($enabledFields); - - // Disable upload fields other than the one we're currently working with. - $disabledFields.attr('disabled', 'disabled'); - - // All the other mousedown handlers (like Drupal's AJAX behaviors) are - // excuted before any timeout functions will be called, so this effectively - // re-enables the file fields after other processing is complete even though - // it is only a 1 second timeout. + // Temporarily disable upload fields other than the one we're currently + // working with. Filter out fields that are already disabled so that they + // do not get enabled when we re-enable these fields at the end of behavior + // processing. Re-enable in a setTimeout set to a relatively short amount + // of time (1 second). All the other mousedown handlers (like Drupal's AJAX + // behaviors) are excuted before any timeout functions are called, so we + // don't have to worry about the fields being re-enabled too soon. + // @todo If the previous sentence is true, why not set the timeout to 0? + var $fieldsToTemporarilyDisable = $('div.form-managed-file input.form-file').not($enabledFields).not(':disabled'); + $fieldsToTemporarilyDisable.attr('disabled', 'disabled'); setTimeout(function (){ - $disabledFields.attr('disabled', ''); + $fieldsToTemporarilyDisable.attr('disabled', ''); }, 1000); }, /** diff --git a/modules/simpletest/tests/form.test b/modules/simpletest/tests/form.test index 8821ec97ceb6..29fb5eb534b0 100644 --- a/modules/simpletest/tests/form.test +++ b/modules/simpletest/tests/form.test @@ -185,26 +185,43 @@ class FormsTestCase extends DrupalWebTestCase { // element. drupalPost() emulates a browser by not submitting input for // disabled elements, so we need to un-disable those elements first. $this->drupalGet('form-test/disabled-elements'); + $disabled_elements = array(); foreach ($this->xpath('//*[@disabled]') as $element) { + $disabled_elements[] = (string) $element['name']; unset($element['disabled']); } + + // All the elements should be marked as disabled, including the ones below + // the disabled container. + $this->assertEqual(count($disabled_elements), 20, t('The correct elements have the disabled property in the HTML code.')); + $this->drupalPost(NULL, $edit, t('Submit')); $returned_values['hijacked'] = drupal_json_decode($this->content); // Ensure that the returned values match the form's default values in both // cases. foreach ($returned_values as $type => $values) { - foreach (element_children($form) as $key) { - if (isset($form[$key]['#default_value'])) { - $expected_value = $form[$key]['#default_value']; + $this->assertFormValuesDefault($values, $form); + } + } - if ($key == 'checkboxes_multiple') { - // Checkboxes values are not filtered out. - $values[$key] = array_filter($values[$key]); - } - $this->assertIdentical($expected_value, $values[$key], t('Default value for %type: expected %expected, returned %returned.', array('%type' => $key, '%expected' => var_export($expected_value, TRUE), '%returned' => var_export($values[$key], TRUE)))); + /** + * Assert that the values submitted to a form matches the default values of the elements. + */ + function assertFormValuesDefault($values, $form) { + foreach (element_children($form) as $key) { + if (isset($form[$key]['#default_value'])) { + $expected_value = $form[$key]['#default_value']; + + if ($key == 'checkboxes_multiple') { + // Checkboxes values are not filtered out. + $values[$key] = array_filter($values[$key]); } + $this->assertIdentical($expected_value, $values[$key], t('Default value for %type: expected %expected, returned %returned.', array('%type' => $key, '%expected' => var_export($expected_value, TRUE), '%returned' => var_export($values[$key], TRUE)))); } + + // Recurse children. + $this->assertFormValuesDefault($values, $form[$key]); } } diff --git a/modules/simpletest/tests/form_test.module b/modules/simpletest/tests/form_test.module index 09cea654fd71..cbc4c0ead7f4 100644 --- a/modules/simpletest/tests/form_test.module +++ b/modules/simpletest/tests/form_test.module @@ -852,6 +852,19 @@ function _form_test_disabled_elements($form, &$form_state) { ), ); + // The #disabled state should propagate to children. + $form['disabled_container'] = array( + '#disabled' => TRUE, + ); + foreach (array('textfield', 'textarea', 'hidden') as $type) { + $form['disabled_container']['disabled_container_' . $type] = array( + '#type' => $type, + '#title' => $type, + '#default_value' => $type, + '#test_hijack_value' => 'HIJACK', + ); + } + $form['submit'] = array( '#type' => 'submit', '#value' => t('Submit'), -- GitLab