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.
While creating the plupload widget (http://drupal.org/sandbox/vingborg/1138512), I realised that the plupload form element requires at least one file when it is submitted.
The attached patch fixes that issue against the master. It's for Drupal 7 only, but backporting it to the Drupal 6 branch should be trivial.
Comment | File | Size | Author |
---|---|---|---|
#20 | 1146562_handling_required_files_correctly_20.patch | 3.37 KB | mr.york |
#18 | 1146562_handling_required_files_correctly_18.patch | 3.16 KB | slashrsm |
#7 | handling_required_files-1146562-7.patch | 3.05 KB | rv0 |
#4 | handling-required-files-1146562-4.patch | 1.45 KB | David_Rothstein |
#1 | handling-required_files_D7.patch | 1.23 KB | vingborg |
Comments
Comment #1
vingborg CreditAttribution: vingborg commentedThis is the same patch as above, but against the latest revision in the repository (update this morning).
Comment #2
gregglesIt would be great to get review from one of the 7.x maintainers on this.
Based on vingborg's work at http://drupal.org/sandbox/vingborg/1138512 I am inclined to trust it and will consider committing it with a proper review from someone using the 7.x branch.
Comment #3
Scott J CreditAttribution: Scott J commentedI have been using this patch on D7, and it does fix the issue, and doesn't seem to cause any other problems.
Comment #4
David_Rothstein CreditAttribution: David_Rothstein commentedCan you clarify how you tested this in D7? I tried this with Media, and although it allowed the "upload" to proceed with no files, the plupload browser never winds up closing, leaving the user in a confused state. I think that may be an issue with Media itself (more than this patch) but since I don't know of anything else this module integrates with in D7, I'd be hesistant to commit it until we figure it out :)
The basic approach looks fine, though, so if the code works in D6 it's certainly reasonable to commit there.
Meanwhile, I rerolled the patch (attached) to fix a couple small issues:
Comment #5
sidney CreditAttribution: sidney commentedI'm using this in D7 with the mupload sandbox module http://drupal.org/sandbox/vingborg/1138512
I just made a content type with an image field set to allow unlimited instances and gave the field a widget type of muploader. Without this patch editing the body text of the page or the alt text of a existing image in the image field and trying to save would result in an error message about at least one file upload being required.
Comment #6
Fidelix CreditAttribution: Fidelix commentedNone of these patches apply to the dev version of plupload.
Comment #7
rv0 CreditAttribution: rv0 commentedRerolled the patch, structured the javascript a bit differently.
Why weren't the previous patches committed?
(patch created at Coworks)
Comment #8
attiks CreditAttribution: attiks commented#7 works like a charm
Comment #9
sachbearbeiter CreditAttribution: sachbearbeiter commentedis it commited to dev now?
Comment #10
janadam#7 works for me too
Comment #11
jsheffers CreditAttribution: jsheffers commentedEverything works for me, but I get an error message when removing images.
Notice: Undefined index: 1 in file_ajax_upload() (line 265 of /home/mysite/public_html/modules/file/file.module).
Notice: Undefined index: #suffix in file_ajax_upload() (line 274 of /home/mysite/public_html/modules/file/file.module).
It doesn't seem to effect the functionality and the image deletes correctly, but it's not good for a user to see that kind of error. Any ideas. I have Plupload 7.Dev with the patch from number 7.
Comment #12
mrryanjohnston CreditAttribution: mrryanjohnston commentedSame issue as #11, except after deleting, all of the pictures are removed except for the last one in the queue.
Comment #13
tomfilepp CreditAttribution: tomfilepp commentedHas this been committed to the project on any level? I'm having some whitespace issues applying this patch to the May 25th dev.
Comment #14
rv0 CreditAttribution: rv0 commented@tomfilepp
Did you use the patch in #7?
weird that nobody reported the whitespace + that it hasn't been committed.
Comment #15
jacks0_0 CreditAttribution: jacks0_0 commentedSame issue as #11, using dev version of pupload and applied the patch #7
Comment #16
jsheffers CreditAttribution: jsheffers commentedThis solution works for me, but there is still an Ajax error when removing images, and drag to reorder is nor functioning correctly either. It allows you to drag it, but it won't let you "put it down." When you finally do get it in the right order, it doesn't show that way on the node.
Comment #17
kytom CreditAttribution: kytom commentedI have the exact same problems as #16. When dragging, releasing mouse does nothing - image still gets dragged and I have to click the arrow icon again to let it go.
More importantly however, even if I reorder them, the change isn't reflected in the node. I come back to the edit page and the changes aren't saved. Is there any way to make this work, some workaround perhaps? This is somewhat of a crucial functionality for me.
Comment #18
slashrsm CreditAttribution: slashrsm commentedAttached patch is basically the same as #7. I just deleted some whitespace in empty lines, added another comment and added missing Drupal.t() around a string in .js. Please check if you agree with the changes.
As regards ajax errors: Could this be a Mupload issue? I get the same errors with Plupload 7.x-1.x with and without this patch. On the other hand I do not get any errors using Filefield sources Plupload: http://drupal.org/sandbox/atlea/1414774. That makes me think that Mupload causes this issue.
Since it looks like ajax errors have nothing to do with this patch itself, I'd recommend calling this RTBC as soon as someone gives this last patch a review. We can open another issue about ajax errors. Does this sound OK?
Comment #19
rv0 CreditAttribution: rv0 commentedIn case anyone hasn't noticed this module yet: http://drupal.org/project/plup
I've been using it in latest projects
Comment #20
mr.york CreditAttribution: mr.york commentedMultiple plupload form elements incorrectly detected which required.
Small fix.
Comment #21
slashrsm CreditAttribution: slashrsm commentedIt looks like commited patch from #1414486: Allow for Multiple Plupload elements in a form fixed that issue also.