Steps to reproduce:

  1. Create a node type with an image field with minimum dimensions of 300x500 pixels
  2. Create a new node of that type and attempt to upload an image which is smaller than 300x500 pixels
  3. Error message presented: "The specified file test.jpg could not be uploaded. The image is too small; the minimum dimensions are 300x500 pixels."
  4. Attempt to upload an image which is at least 300x500 pixels

Expected results:
Image is successfully uploaded and the error message is removed.

Actual results:
Image is successfully uploaded but the error message from the previous upload is still present.

Analysis:
From what I can tell the problem is in file_ajax_upload() in file.module. The markup generated by theme('status_messages') is prepended to the rendered form and ends up outside the form's ajax-wrapper div. When the form is submitted a second time only the ajax-wrapper div is replaced and the message is not removed.

The simplest way to get the status messages inside the ajax-wrapper is to replace line 238 of file.module:
  $output = theme('status_messages') . drupal_render($form);
with
  $form['#prefix'] .= theme('status_messages');
  $output = drupal_render($form);

I verified that this change fixes the problem however I'm new enough to Drupal that someone else should look at this. There might be a better way to solve this or an edge case which is broken by this change.

Files: 
CommentFileSizeAuthor
#25 Pics of result of patch testing.zip107.83 KBjessum
#15 file-repeated-error-message-on-ajax-upload-1792032-15.patch542 bytesndobromirov
PASSED: [[SimpleTest]]: [MySQL] 40,911 pass(es).
[ View ]
#10 file_validation_error_message-1792032-10-test_only.patch2.71 KBbjorpe
FAILED: [[SimpleTest]]: [MySQL] 56,053 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#10 file_validation_error_message-1792032-10-complete.patch3.24 KBbjorpe
PASSED: [[SimpleTest]]: [MySQL] 55,996 pass(es).
[ View ]
#8 1792032-8-1.jpg93.99 KBsidharthap
#8 1792032-8-2.jpg39.8 KBsidharthap
#6 file_validation_error_message-1792032-5-success.png73.13 KBandymartha
#5 file_validation_error_message-1792032-5.patch541 bytesbalsama
PASSED: [[SimpleTest]]: [MySQL] 51,471 pass(es).
[ View ]

Comments

earnie’s picture

Version:7.15» 8.x-dev

Does this issue exist in 8.x-dev?

lxs’s picture

Yes, I am able to reproduce the same issue with 8.x-dev (September 24, 2012 - 08:19).

It also looks like $output = theme('status_messages') . drupal_render($form); still exists in file_ajax_upload() in Drupal 8 (now on line 951). I checked and my above patch does still fix the problem.

larowlan’s picture

Issue tags:+Needs tests, +#pnx-sprint

Tagging

penyaskito’s picture

This could be a good starting point for a novice in creating patches and writing tests. It should be backported to D7 once is fixed in 8.x.

balsama’s picture

Status:Active» Needs review
StatusFileSize
new541 bytes
PASSED: [[SimpleTest]]: [MySQL] 51,471 pass(es).
[ View ]

Drupal 8 patch attached.

andymartha’s picture

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new73.13 KB

The latest patch, file_validation_error_message-1792032-5.patch, worked for me in correcting the error message bug in the latest version of drupal 8 as of March 6, 2013. Please see attached screenshot.

tim.plunkett’s picture

Status:Reviewed & tested by the community» Needs work

Still tagged "Needs tests".

sidharthap’s picture

StatusFileSize
new39.8 KB
new93.99 KB

I tested the patch #5, It works for me. Please find the attached before and after screen shots.

Before :
before
After :
after

bjorpe’s picture

Assigned:Unassigned» bjorpe

Started working on an automated test.

bjorpe’s picture

Status:Needs work» Needs review
StatusFileSize
new3.24 KB
PASSED: [[SimpleTest]]: [MySQL] 55,996 pass(es).
[ View ]
new2.71 KB
FAILED: [[SimpleTest]]: [MySQL] 56,053 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Test added, see attached patches.

This bug/fix seems to be applicable to all validations, not just image size. To keep the test simple, it checks file extension validation.

phl3tch’s picture

Status:Needs review» Reviewed & tested by the community
alexpott’s picture

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Great issue summary, nice test, nice patch :)

Committed ff1eea3 and pushed to 8.x. Thanks!

bjorpe’s picture

Assigned:bjorpe» Unassigned

Can't backport at the moment

karthikkumarbodu’s picture

Hi, Thanks for the fix can the patch be applied in the current Drupal 7.22 core?

Karthik Kumar Bodu

ndobromirov’s picture

StatusFileSize
new542 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,911 pass(es).
[ View ]

Added a patch for drupal 7

ndobromirov’s picture

Status:Patch (to be ported)» Needs review
balajidharma’s picture

Status:Needs review» Reviewed & tested by the community
David_Rothstein’s picture

Status:Reviewed & tested by the community» Needs review

Closed #1020916: File ajax upload doesn't clear previous error message after successful ajax upload as a duplicate (although technically this was the duplicate since it's newer).

Is there a reason the tests aren't being backported to Drupal 7 also?

ndobromirov’s picture

I needed the patch in 7 and I was in a hurry in the middle of a project.
Probably sometime this week(end) I'll attempt to back-port the tests also.

Can confirm that the patch works on live with no side effects for almost 2 months now.

karthikkumarbodu’s picture

Hi, i believe there is a fix for this issue in Drupal 7 and its not fixed in Drupal 6.23.

mgifford’s picture

acbramley’s picture

Patch correctly fixes the issue for me, but the steps to reproduce need to be slightly tweaked:

1) Choose a file that doesn't pass validation
* Error appears
2) Click the upload button
3) Choose a file that doesn't pass validation again
4) File appears in dialog
5) Click upload
* Error appears
6) Choose a file that does pass validation
7) Click upload
* Error remains

After applying the patch, the error disappears after clicking upload, but it might be better to remove it after you choose a correct file?

lmeurs’s picture

As acbramley says in #22: moving the messages to inside the AJAX wrapper works great, thank you! His remark about error messages being removed right after selecting a file could be easily fixed by wrapping the error messages. The patch would look like:

<?php
- $output = theme('status_messages') . drupal_render($form);
+
$form['#prefix'] .= '<div class="file-upload-js-error">' . theme('status_messages') . '</div>';
+
$output = drupal_render($form);
?>

Explanation

There are two types of file validation:

  1. The file extension is validated client side,
  2. The file size is validated server side.

When selecting a file, Drupal.file.validateExtension() is fired which removes possible earlier inserted error messages right away. This only happens to error messages inserted client side by this same function, since these messages have the .file-upload-js-error class. By wrapping the server side messages in a DIV with the same class, they also immediately get removed as soon as a new file is only selected.

But there is a catch...

Situation: After uploading a too large file, a server side error is returned. When now selecting a file with an invalid extension the client side extension validator is not being fired.

This is because the onchange event has not been bound to the file field after the server side validation error. Drupal.behaviors.fileValidateAutoAttach.attach uses selectors to find the file field (child element), but file_managed_file_process() bases these selectors on the managed file element's ID (parent element). This ID gets incremented on each AJAX file upload / removal, but the ID of the file field only gets incremented in case of an invalid upload.

On top of this, the file field's selector is composed by appending "-upload" to the managed file element's ID which results in selectors like "#edit-attachment--5-upload" when the file field's ID at the same time could be "edit-attachment-upload" or "edit-attachment-upload--3" (the incrementation and position of "-upload" differ).

Conclusion

The server side error messages will be removed immediately when a new file is selected, as long as they are being wrapped as suggested and the selector problem is fixed (or use my workaround from #2235977-1: JS file validation is broken (because ajaxPageState is broken?)).

jessum’s picture

Assigned:Unassigned» jessum
jessum’s picture

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new107.83 KB

I reproduced the issue and found the error due to the wrong picture size (see attached file: Pics of result of patch.zip).

After finding the problem error I retried the small picture with the patch and then with the large picture and the error message disappeared (see attached file: Pics of result of patch.zip).

Thanks to Leslie Glynn for mentoring my first review.

David_Rothstein’s picture

Status:Reviewed & tested by the community» Needs work

The tests still need to be backported - see above.

sarjeet.singh’s picture

Thanks lmeurs, your solution in #22 is working fine for me. We can commit it core.

thomastorfs’s picture

I can confirm that the patch in #15 works. When can we expect to see this in core?