Problem/Motivation:

Javscript upload widget does not return to original state after error

Description from martynpanes:

If a file larger than the permitted limited is uploaded in a file field the response returned is the following generic error message:
"An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (2 MB) that this server supports." and the form is rebuilt with the file upload widget missing.

Whilst a browser refresh will return the file widget - a lot of users wont think to do this resulting in confusion and poor user experience.

Steps to reproduce:

1. Creat file upload field.
2. Attempt to upload file larger than server configuration

Original Post:

If a file larger than the permitted limited is uploaded in a file field the response returned is the following generic error message:
"An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (2 MB) that this server supports." and the form is rebuilt with the file upload widget missing.

Whilst a browser refresh will return the file widget - a lot of users wont think to do this resulting in confusion and poor user experience.

Files: 
CommentFileSizeAuthor
#51 core-file_upload_ajax_message-1489692-51-D8.patch988 bytesLiam Morland
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
#32 core_1489692_file_upload_ajax_message.patch694 bytesLiam Morland
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch core_1489692_file_upload_ajax_message.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#22 core_1489692_file_upload_ajax_message.patch694 bytesLiam Morland
FAILED: [[SimpleTest]]: [MySQL] 41,197 pass(es), 2 fail(s), and 2 exception(s).
[ View ]
#17 core_1489692_file_upload_ajax_message_D8.patch736 bytesLiam Morland
PASSED: [[SimpleTest]]: [MySQL] 57,481 pass(es).
[ View ]
#8 core_1489692_file_upload_ajax_message_D8.patch736 bytesLiam Morland
PASSED: [[SimpleTest]]: [MySQL] 55,022 pass(es).
[ View ]
#7 core_1489692_file_upload_ajax_message.patch694 bytesLiam Morland
PASSED: [[SimpleTest]]: [MySQL] 40,491 pass(es).
[ View ]
#1 Screen shot 2012-05-20 at 1.38.17 PM.png34 KBkpoeppe

Comments

jasonrichardsmith@gmail.com’s picture

Issue summary:View changes

updated issue for clarity

kpoeppe’s picture

Version:7.12» 8.x-dev
Issue tags:+D8UX usability, +tcdrupal2012
StatusFileSize
new34 KB

Summary: I could not reproduce the original problem. The file attach widget was available to me after I uploaded a file that exceeded the max size, and it was easy to upload a different file. However, the error message from my first attempt persisted after I successfully uploaded a smaller file. I don't think users need to continue to see the error message once a new file is successfully uploaded. (see attached screenshot)

Test process:
1) Created a file upload field. Set max file size to 40KB.
2) Attempted to upload a file larger than 40 KB.

Results:
I got an error message "The specified file Why Drupal 7.docx could not be uploaded. The file is 92.84 KB exceeding the maximum file size of 40 KB." The file upload widget (Browse button, Upload button) remained visible. I didn't have to refresh my browser.

Next, I successfully uploaded a new, smaller file.

However: the error message from my first attempt to upload a file was still on the screen. I don't think users need to continue to see the error message once a file is successfully uploaded. (see attached screenshot)

Liam Morland’s picture

Liam Morland’s picture

There are two different messages and file upload limits:

  1. If the uploaded file is larger than the limit configured in the content type, but smaller than the upload limit configured in php.ini, you will get the message "The specified file %name could not be uploaded. The file is %filesize exceeding the maximum file size of %maxsize." This message is in file.inc, in two parts.
  2. If the uploaded file is larger than the upload limit configured in php.ini, you will get the message, "An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (@size) that this server supports." This message is in file.module.

In this second case only, the upload wiget disappears and any other file already uploaded also disappears. If you save at this point, any other files uploaded will be removed from the node. I am seeing this in D7.22.

Liam Morland’s picture

However: the error message from my first attempt to upload a file was still on the screen. I don't think users need to continue to see the error message once a file is successfully uploaded. (see attached screenshot)

This is a duplicate of #1792032: File validation error message not removed after subsequent upload of valid file.

Liam Morland’s picture

After reading the fix to #1792032: File validation error message not removed after subsequent upload of valid file, I think this is the problem: In file_ajax_upload(), the first call to ajax_command_replace() replaces everything with the error message. The calls to ajax_command_replace() later in file_ajax_upload() include the rendered version of the form in the replacement, so the form still shows.

Liam Morland’s picture

Version:8.x-dev» 7.x-dev
Status:Active» Needs review

Here is a D7 patch. I replaced the call to ajax_command_replace() with a call to ajax_command_prepend(), so the error message gets prepended without disturbing the form.

Liam Morland’s picture

StatusFileSize
new694 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,491 pass(es).
[ View ]
Liam Morland’s picture

Version:7.x-dev» 8.x-dev
StatusFileSize
new736 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,022 pass(es).
[ View ]

D8 version.

Status:Needs review» Needs work

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

Liam Morland’s picture

Sorry, my D8 testing box is not working now. I just did what I thought was equivalent.

Liam Morland’s picture

Status:Needs work» Needs review
Issue tags:-tcdrupal2012

The failures do not appear to be related to anything changed by the patch. Trying again in case it was a problem with the testing process.

Liam Morland’s picture

Issue tags:-D8UX usability

I didn't mean to change the tagging.

Liam Morland’s picture

Fixing tags.

Liam Morland’s picture

Fixing tags.

Liam Morland’s picture

Liam Morland’s picture

Component:file system» file.module
Liam Morland’s picture

StatusFileSize
new736 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,481 pass(es).
[ View ]

Reroll.

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

Liam Morland’s picture

Status:Needs work» Needs review
Liam Morland’s picture

Liam Morland’s picture

Liam Morland’s picture

Version:8.x-dev» 7.x-dev
StatusFileSize
new694 bytes
FAILED: [[SimpleTest]]: [MySQL] 41,197 pass(es), 2 fail(s), and 2 exception(s).
[ View ]

D8 code has been completely rewritten, removing the function that was patched, but the issue remains in D7.

Liam Morland’s picture

Liam Morland’s picture

Issue summary:View changes

edited some formatting

Liam Morland’s picture

Liam Morland’s picture

a.milkovsky’s picture

Issue summary:View changes
Status:Needs review» Reviewed & tested by the community

I confirm the patch. It's better to prepend error instead of relapsing the whole file widget.
It should be changed.
Thank you, Liam.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 22: core_1489692_file_upload_ajax_message.patch, failed testing.

Status:Needs work» Needs review
Liam Morland’s picture

Status:Needs review» Reviewed & tested by the community

Restoring previous status.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 22: core_1489692_file_upload_ajax_message.patch, failed testing.

a.milkovsky’s picture

omg.
@Liam, maybe you need to re-roll it with last dev version?

Liam Morland’s picture

Status:Needs work» Needs review
StatusFileSize
new694 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch core_1489692_file_upload_ajax_message.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here is a reroll, but I don't think that is the problem. The errors seem to be unrelated to the patch and it passes some of the time.

Liam Morland’s picture

Status:Needs review» Reviewed & tested by the community

Restoring previous status.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 32: core_1489692_file_upload_ajax_message.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 32: core_1489692_file_upload_ajax_message.patch, failed testing.

Liam Morland’s picture

Status:Needs work» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 32: core_1489692_file_upload_ajax_message.patch, failed testing.

Liam Morland’s picture

Status:Needs work» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 32: core_1489692_file_upload_ajax_message.patch, failed testing.

Status:Needs work» Needs review
Liam Morland’s picture

Status:Needs review» Reviewed & tested by the community
Roya-Rateshtari’s picture

Hi,

This patch worked for me.

Thanks

David_Rothstein’s picture

Version:7.x-dev» 8.0.x-dev
Status:Reviewed & tested by the community» Needs review
Issue tags:+needs backport to D7

Are we really sure this doesn't affect Drupal 8? I see what looks like equivalent code in core/modules/file/src/Controller/FileWidgetAjaxController.php:

<?php
   
if (empty($request_form_build_id) || $form_build_id !== $request_form_build_id) {
     
// Invalid request.
     
drupal_set_message(t('An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (@size) that this server supports.', array('@size' => format_size(file_upload_max_size()))), 'error');
     
$response = new AjaxResponse();
     
$status_messages = array('#theme' => 'status_messages');
      return
$response->addCommand(new ReplaceCommand(NULL, drupal_render($status_messages)));
    }
?>

Also, if it's true that this one case should be ajax_command_prepend() but the other two calls later in the function should remain ajax_command_replace(), this is screaming for a code comment explaining why :) I would at least think that if this one needs to be ajax_command_prepend() then the one after it should be too...

Liam Morland’s picture

I don't know if it effects D8 or not. It looked to me like the code had been rewritten, so I moved this to just being a D7 issue.

I only noticed the problem in the place that I fixed it. It seems reasonable that they should all be the same. I just haven't run into the problem with the other code.

Status:Needs review» Needs work

The last submitted patch, 32: core_1489692_file_upload_ajax_message.patch, failed testing.

Liam Morland’s picture

Status:Needs work» Needs review
StatusFileSize
new988 bytes
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

Re-roll of D8 patch.

Status:Needs review» Needs work

The last submitted patch, 51: core-file_upload_ajax_message-1489692-51-D8.patch, failed testing.