Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
To duplicate the issue:
* Create a managed_file element in a form
* Try to upload a file that will result in an error (i.e. with a not allowed extension)
* Now there is a nice error message just before the upload form element
* Upload a valid file.
* The error message produced from the previous attempt is still there.
Here is a possible solution (sorry can't create a patch now):
file.module lines 268-285 current code:
// Add the special AJAX class if a new file was added.
if (isset($form['#file_upload_delta']) && $current_file_count < $form['#file_upload_delta']) {
$form[$current_file_count]['#attributes']['class'][] = 'ajax-new-content';
}
// Otherwise just add the new content class on a placeholder.
else {
$form['#suffix'] .= '<span class="ajax-new-content"></span>';
}
$output = theme('status_messages') . drupal_render($form) ;
$js = drupal_add_js();
$settings = call_user_func_array('array_merge_recursive', $js['settings']['data']);
$commands = array();
$commands[] = ajax_command_replace(NULL, $output, $settings);
return array('#type' => 'ajax', '#commands' => $commands);
proposed modification:
// Add the special AJAX class if a new file was added.
if (isset($form['#file_upload_delta']) && $current_file_count < $form['#file_upload_delta']) {
$form[$current_file_count]['#attributes']['class'][] = 'ajax-new-content';
}
// Otherwise just add the new content class on a placeholder.
else {
$form['#suffix'] .= '<span class="ajax-new-content"></span>';
}
$output = "<div class='file-upload-js-error'>" . theme('status_messages') . "</div>" . drupal_render($form) ; // we wrap the error message in a div
$js = drupal_add_js();
$settings = call_user_func_array('array_merge_recursive', $js['settings']['data']);
$commands = array();
$commands[] = ajax_command_remove(".file-upload-js-error"); // we remove a possible error message first
$commands[] = ajax_command_replace(NULL, $output, $settings);
return array('#type' => 'ajax', '#commands' => $commands);
Comment | File | Size | Author |
---|---|---|---|
#9 | core_file_module-fix_ajax_error_messages-1020916-9.patch | 553 bytes | phreaknerd |
#8 | 1020916-file_ajax_upload-3.patch | 789 bytes | bambilicious |
#6 | 1020916-file_ajax_upload-2.patch | 766 bytes | bambilicious |
#5 | file_ajax_upload.patch | 766 bytes | pembeci |
#2 | file_ajax_upload.patch | 742 bytes | pembeci |
Comments
Comment #1
pembeci CreditAttribution: pembeci commentedComment #2
pembeci CreditAttribution: pembeci commentedI was on a Windows machine, finally found the time connecting a Linux and produced the patch.
Comment #4
pembeci CreditAttribution: pembeci commentedComment #5
pembeci CreditAttribution: pembeci commentedCan't apply patch. Path problem? Trying again.
Comment #6
bambilicious CreditAttribution: bambilicious commentedI've reproduced this bug and the patch works perfect for me.
However, as the new strings introduced in #5 does not contains any variables or escape-sequences, I've changed "..." to '...'.
Please correct me if I'm wrong.
Comment #8
bambilicious CreditAttribution: bambilicious commentedp0 format was used in previous patch, here's p1.
Comment #9
phreaknerd CreditAttribution: phreaknerd commentedHere is an alternative approach. It additionally prevents the file field to infinitely nest itself in a new div-container on each ajax request.
Comment #10
phreaknerd CreditAttribution: phreaknerd commentedBump: Can anyone review those two lines of code, please? ;-)
Comment #11
phreaknerd CreditAttribution: phreaknerd commented#9: core_file_module-fix_ajax_error_messages-1020916-9.patch queued for re-testing.
Comment #12
texas-bronius CreditAttribution: texas-bronius commentedJust chiming in:
Am I allowed to mark RTBC? I'm gonna mark it... Thanks for sharing this simple fix!
Comment #13
phreaknerd CreditAttribution: phreaknerd commented#9: core_file_module-fix_ajax_error_messages-1020916-9.patch queued for re-testing.
Comment #14
amateescu CreditAttribution: amateescu commentedBugs are fixed in the latest development branch, and we also need an automated test here.
The test behavior should be pretty straightforward: upload a file that's not allowed, check that the error is present, upload a file that is allowed, check that the error is gone.
It's usually easier to find an existing test for the form element and stick the additional assertion in there. For example, assuming that uploading a file that is not allowed is already tested, you could add the new upload and assert just below that.
If the bug cannot be reproduced in the latest 8.x version, feel free to bump it back to 7.x :)
Comment #15
David_Rothstein CreditAttribution: David_Rothstein commentedAlthough this issue is older, #1792032: File validation error message not removed after subsequent upload of valid file was already committed to Drupal 8 and also has a test.