Index: modules/file/file.field.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/file/file.field.inc,v retrieving revision 1.35 diff -u -p -r1.35 file.field.inc --- modules/file/file.field.inc 1 Oct 2010 01:32:59 -0000 1.35 +++ modules/file/file.field.inc 15 Oct 2010 00:39:20 -0000 @@ -448,11 +448,19 @@ function file_field_widget_form(&$form, 'description' => '', ); - // Retrieve any values set in $form_state, as will be the case during AJAX - // rebuilds of this form. + // When the form is being rebuilt (for example, after clicking the "Upload", + // "Remove", or "Preview" buttons), there may be submitted values not + // reflected in $items, because $items is normally updated as part of + // field_attach_submit(), which is only called by button submit handlers that + // require an updated entity (e.g., "Preview"). So, grab $items from the form + // values, but because we then filter it and renumber the keys, and then + // create form elements based on the renumbered keys, the key/value + // associations in $form_state['values'] and $form_state['input'] become + // invalid and must be emptied. if (isset($form_state['values'][$field['field_name']][$langcode])) { $items = $form_state['values'][$field['field_name']][$langcode]; unset($form_state['values'][$field['field_name']][$langcode]); + unset($form_state['input'][$field['field_name']][$langcode]); } foreach ($items as $delta => $item) { @@ -649,7 +657,8 @@ function file_field_widget_process($elem // file, the entire group of file fields is updated together. if ($field['cardinality'] != 1) { $new_path = preg_replace('/\/\d+\//', '/', $element['remove_button']['#ajax']['path'], 1); - $new_wrapper = preg_replace('/-\d+-/', '-', $element['remove_button']['#ajax']['wrapper'], 1); + $parent_element = drupal_array_get_nested_value($form, array_slice($element['#array_parents'], 0, -1)); + $new_wrapper = $parent_element['#id'] . '-ajax-wrapper'; foreach (element_children($element) as $key) { if (isset($element[$key]['#ajax'])) { $element[$key]['#ajax']['path'] = $new_path; Index: modules/file/tests/file.test =================================================================== RCS file: /cvs/drupal/drupal/modules/file/tests/file.test,v retrieving revision 1.26 diff -u -p -r1.26 file.test --- modules/file/tests/file.test 1 Oct 2010 01:32:59 -0000 1.26 +++ modules/file/tests/file.test 15 Oct 2010 00:39:20 -0000 @@ -203,29 +203,21 @@ class FileFieldTestCase extends DrupalWe /** - * Test class to test file field upload and remove buttons, with and without AJAX. + * Test class to test file field widget, single and multi-valued, with and without AJAX, with public and private files. */ class FileFieldWidgetTestCase extends FileFieldTestCase { public static function getInfo() { return array( 'name' => 'File field widget test', - 'description' => 'Test upload and remove buttons, with and without AJAX.', + 'description' => 'Tests the file field widget, single and multi-valued, with and without AJAX, with public and private files.', 'group' => 'File', ); } /** - * Tests upload and remove buttons, with and without AJAX. - * - * @todo This function currently only tests the "remove" button of a single- - * valued field. Tests should be added for the "upload" button and for each - * button of a multi-valued field. Tests involving multiple AJAX steps on - * the same page will become easier after http://drupal.org/node/789186 - * lands. Testing the "upload" button in AJAX context requires more - * investigation into how jQuery uploads files, so that drupalPostAJAX() can - * emulate that correctly. + * Tests upload and remove buttons, with and without AJAX, for a single-valued File field. */ - function testWidget() { + function testSingleValuedWidget() { // Use 'page' instead of 'article', so that the 'article' image field does // not conflict with this test. If in the future the 'page' type gets its // own default file or image field, this test can be made more robust by @@ -241,11 +233,14 @@ class FileFieldWidgetTestCase extends Fi foreach (array('nojs', 'js') as $type) { // Create a new node with the uploaded file and ensure it got uploaded // successfully. + // @todo This only tests a 'nojs' submission, because drupalPostAJAX() + // does not yet support file uploads. $nid = $this->uploadNodeFile($test_file, $field_name, $type_name); $node = node_load($nid, NULL, TRUE); $node_file = (object) $node->{$field_name}[LANGUAGE_NONE][0]; $this->assertFileExists($node_file, t('New file saved to disk on node creation.')); - // Test file download. + + // Ensure the file can be downloaded. $this->drupalGet(file_create_url($node_file->uri)); $this->assertResponse(200, t('Confirmed that the generated URL is correct by downloading the shipped file.')); @@ -260,13 +255,8 @@ class FileFieldWidgetTestCase extends Fi $this->drupalPost(NULL, array(), t('Remove')); break; case 'js': - // @todo This can be simplified after http://drupal.org/node/789186 - // lands. - preg_match('/jQuery\.extend\(Drupal\.settings, (.*?)\);/', $this->content, $matches); - $settings = drupal_json_decode($matches[1]); $button = $this->xpath('//input[@type="submit" and @value="' . t('Remove') . '"]'); - $button_id = (string) $button[0]['id']; - $this->drupalPostAJAX(NULL, array(), array((string) $button[0]['name'] => (string) $button[0]['value']), $settings['ajax'][$button_id]['url'], array(), array(), NULL, $settings['ajax'][$button_id]); + $this->drupalPostAJAX(NULL, array(), array((string) $button[0]['name'] => (string) $button[0]['value'])); break; } @@ -279,34 +269,133 @@ class FileFieldWidgetTestCase extends Fi $node = node_load($nid, NULL, TRUE); $this->assertTrue(empty($node->{$field_name}[LANGUAGE_NONE][0]['fid']), t('File was successfully removed from the node.')); } + } - // Test partial form submissions using the Upload button on a multivalue field. - field_delete_field($field_name); + /** + * Tests upload and remove buttons, with and without AJAX, for a multi-valued File field. + */ + function testMultiValuedWidget() { + // Use 'page' instead of 'article', so that the 'article' image field does + // not conflict with this test. If in the future the 'page' type gets its + // own default file or image field, this test can be made more robust by + // using a custom node type. + $type_name = 'page'; + $field_name = strtolower($this->randomName()); $this->createFileField($field_name, $type_name, array('cardinality' => 3)); + $field = field_info_field($field_name); + $instance = field_info_instance('node', $field_name, $type_name); + + $test_file = $this->getTestFile('text'); + + foreach (array('nojs', 'js') as $type) { + // Visit the node creation form, and upload 3 files. Since the field has + // cardinality of 3, ensure the "Upload" button is displayed until after + // the 3rd file, and after that, isn't displayed. + // @todo This is only testing a non-AJAX upload, because drupalPostAJAX() + // does not yet emulate jQuery's file upload. + $this->drupalGet("node/add/$type_name"); + for ($delta = 0; $delta < 3; $delta++) { + $edit = array('files[' . $field_name . '_' . LANGUAGE_NONE . '_' . $delta . ']' => drupal_realpath($test_file->uri)); + // If the Upload button doesn't exist, drupalPost() will automatically + // fail with an assertion message. + $this->drupalPost(NULL, $edit, t('Upload')); + } + $this->assertNoFieldByXpath('//input[@type="submit"]', t('Upload'), t('After uploading 3 files, the "Upload" button is no longer displayed.')); + + // Test clicking each "Remove" button. For extra robustness, test them out + // of sequential order. They are 0-indexed, and get renumbered after each + // iteration, so array(1, 1, 0) means: + // - First remove the 2nd file. + // - Then remove what is then the 2nd file (was originally the 3rd file). + // - Then remove the first file. + $num_expected_remove_buttons = 3; + foreach (array(1, 1, 0) as $delta) { + // Ensure we have the expected number of Remove buttons, and that they + // are numbered sequentially. + $buttons = $this->xpath('//input[@type="submit" and @value="Remove"]'); + $this->assertTrue(is_array($buttons) && count($buttons) === $num_expected_remove_buttons, t('There are %n "Remove" buttons displayed (JSMode=%type).', array('%n' => $num_expected_remove_buttons, '%type' => $type))); + foreach ($buttons as $i => $button) { + $this->assertIdentical((string) $button['name'], $field_name . '_' . LANGUAGE_NONE . '_' . $i . '_remove_button'); + } + + // "Click" the remove button (emulating either a nojs or js submission). + $button_name = $field_name . '_' . LANGUAGE_NONE . '_' . $delta . '_remove_button'; + switch ($type) { + case 'nojs': + // drupalPost() takes a $submit parameter that is the value of the + // button whose click we want to emulate. Since we have multiple + // buttons with the value "Remove", and want to control which one we + // use, we change the value of the other ones to something else. + // Since non-clicked buttons aren't included in the submitted POST + // data, and since drupalPost() will result in $this being updated + // with a newly rebuilt form, this doesn't cause problems. + foreach ($buttons as $button) { + if ($button['name'] != $button_name) { + $button['value'] = 'DUMMY'; + } + } + $this->drupalPost(NULL, array(), t('Remove')); + break; + case 'js': + // drupalPostAJAX() lets us target the button precisely, so we don't + // require the workaround used above for nojs. + $this->drupalPostAJAX(NULL, array(), array($button_name => t('Remove'))); + break; + } + $num_expected_remove_buttons--; + + // Ensure we have a single Upload button, and that it is numbered + // sequentially after the Remove buttons. + $buttons = $this->xpath('//input[@type="submit" and @value="Upload"]'); + $this->assertTrue(is_array($buttons) && count($buttons) == 1 && ((string) $buttons[0]['name'] === ($field_name . '_' . LANGUAGE_NONE . '_' . $num_expected_remove_buttons . '_upload_button')), t('After removing a file, an "Upload" button is displayed (JSMode=%type).')); + } - $this->drupalGet("node/add/$type_name"); - for ($delta = 0; $delta < 3; $delta++) { - $edit = array('files[' . $field_name . '_' . LANGUAGE_NONE . '_' . $delta . ']' => drupal_realpath($test_file->uri)); - $this->drupalPost(NULL, $edit, t('Upload')); + // Ensure the page now has no Remove buttons. + $this->assertNoFieldByXPath('//input[@type="submit"]', t('Remove'), t('After removing all files, there is no "Remove" button displayed.', array('%n' => $num_expected_remove_buttons, '%type' => $type))); + + // Save the node and ensure it does not have any files. + $this->drupalPost(NULL, array('title' => $this->randomName()), t('Save')); + $matches = array(); + preg_match('/node\/([0-9]+)/', $this->getUrl(), $matches); + $nid = $matches[1]; + $node = node_load($nid, NULL, TRUE); + $this->assertTrue(empty($node->{$field_name}[LANGUAGE_NONE][0]['fid']), t('Node was successfully saved without any files.')); } - $this->assertNoFieldByXpath('//input[@type="submit"]', t('Upload'), t('After uploading 3 files, the "Upload" button is no longer displayed.')); + } + + /** + * Tests a file field with a "Private files" upload destination setting. + */ + function testPrivateFileSetting() { + // Use 'page' instead of 'article', so that the 'article' image field does + // not conflict with this test. If in the future the 'page' type gets its + // own default file or image field, this test can be made more robust by + // using a custom node type. + $type_name = 'page'; + $field_name = strtolower($this->randomName()); + $this->createFileField($field_name, $type_name); + $field = field_info_field($field_name); + $instance = field_info_instance('node', $field_name, $type_name); - // Test private download method. + $test_file = $this->getTestFile('text'); + + // Change the field setting to make its files private, and upload a file. $edit = array('field[settings][uri_scheme]' => 'private'); $this->drupalPost("admin/structure/types/manage/$type_name/fields/$field_name", $edit, t('Save settings')); - // Create a new node with the uploaded file and ensure it got uploaded - // successfully. $nid = $this->uploadNodeFile($test_file, $field_name, $type_name); $node = node_load($nid, NULL, TRUE); $node_file = (object) $node->{$field_name}[LANGUAGE_NONE][0]; $this->assertFileExists($node_file, t('New file saved to disk on node creation.')); - // Test file download. + + // Ensure the private file is available to the user who uploaded it. $this->drupalGet(file_create_url($node_file->uri)); $this->assertResponse(200, t('Confirmed that the generated URL is correct by downloading the shipped file.')); + // Ensure we can't change 'uri_scheme' field settings while there are some // entities with uploaded files. $this->drupalGet("admin/structure/types/manage/$type_name/fields/$field_name"); $this->assertFieldByXpath('//input[@id="edit-field-settings-uri-scheme-public" and @disabled="disabled"]', 'public', t('Upload destination setting disabled.')); + // Delete node and confirm that setting could be changed. node_delete($nid); $this->drupalGet("admin/structure/types/manage/$type_name/fields/$field_name"); Index: modules/simpletest/drupal_web_test_case.php =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/drupal_web_test_case.php,v retrieving revision 1.240 diff -u -p -r1.240 drupal_web_test_case.php --- modules/simpletest/drupal_web_test_case.php 5 Oct 2010 06:17:29 -0000 1.240 +++ modules/simpletest/drupal_web_test_case.php 15 Oct 2010 00:39:22 -0000 @@ -1801,7 +1801,7 @@ class DrupalWebTestCase extends DrupalTe * * @see ajax.js */ - protected function drupalPostAJAX($path, $edit, $triggering_element, $ajax_path = 'system/ajax', array $options = array(), array $headers = array(), $form_html_id = NULL, $ajax_settings = NULL) { + protected function drupalPostAJAX($path, $edit, $triggering_element, $ajax_path = NULL, array $options = array(), array $headers = array(), $form_html_id = NULL, $ajax_settings = NULL) { // Get the content of the initial page prior to calling drupalPost(), since // drupalPost() replaces $this->content. if (isset($path)) { @@ -1838,6 +1838,12 @@ class DrupalWebTestCase extends DrupalTe $extra_post .= '&' . urlencode('ajax_html_ids[]') . '=' . urlencode($id); } + // Unless a particular path is specified, use the one specified by the + // AJAX settings, or else 'system/ajax'. + if (!isset($ajax_path)) { + $ajax_path = isset($ajax_settings['url']) ? $ajax_settings['url'] : 'system/ajax'; + } + // Submit the POST request. $return = drupal_json_decode($this->drupalPost(NULL, $edit, array('path' => $ajax_path, 'triggering_element' => $triggering_element), $options, $headers, $form_html_id, $extra_post));