Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#27037 closed Bug (fixed)

'required' attribute on prefilled ClearableFileInput prevents valid form submissions

Reported by: Matt Westcott Owned by: nobody
Component: Forms Version: 1.10
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The addition of the required HTML attribute on required form fields (#22383) does not handle the situation where a ClearableFileInput is pre-populated with an initial value (causing it to display the "Currently: foo.txt" text alongside an empty <input type="file"> field) and submitted without selecting a new file. According to Django's validation logic this is a valid form submission, as the field is considered non-empty. However, the browser-side validation doesn't know about this nuance - at the HTML level, the <input type="file"> element is an empty field, and has the required attribute set, causing it to fail validation.

This breaks the conventional ModelForm-based 'edit' view pattern, on any model that has a required FileField. For example:

# models.py
class Document(models.Model):
    title = models.CharField(max_length=255)
    file = models.FileField()

# forms.py
class DocumentForm(forms.ModelForm):
    class Meta:
        model = Document
        fields = ['title', 'file']

# views.py
def edit(request, document_id):
    document = Document.objects.get(id=document_id)

    if request.method == 'POST':
        # ...
        return HttpResponse("OK")
    else:
        form = DocumentForm(instance=document)
        return render(request, 'edit.html', {'document': document, 'form': form})

# edit.html
	<form action="{% url 'edit_document' document.id %}" method="POST">
		{{ form.as_p }}
		<input type="submit">
	</form>

# renders as the following, which cannot be submitted as-is: 
<p><label for="id_file">File:</label> Currently: <a href="hello.txt">hello.txt</a> <br />Change: <input id="id_file" name="file" type="file" required /></p>

I suggest that ClearableFileInput should discard the required attribute whenever an initial value exists, in much the same way that CheckboxSelectMultiple does: https://github.com/django/django/blob/35225e2ade08ea32e36a994cd4ff90842c599e20/django/forms/widgets.py#L797-L799

Change History (6)

comment:1 by Tim Graham, 8 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:2 by Jon Dufresne, 8 years ago

PR

I took an approach that I hope is extendable for other widgets and custom widgets that need drop the required attribute.

comment:3 by Jon Dufresne, 8 years ago

Has patch: set

comment:4 by Claude Paroz, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: newclosed

In fab46ce6:

Fixed #27037 -- Prevented required attribute on ClearableFileInput when initial data exists.

comment:6 by Tim Graham <timograham@…>, 8 years ago

In d8cda35:

[1.10.x] Fixed #27037 -- Prevented required attribute on ClearableFileInput when initial data exists.

Backport of fab46ce6f5a0a58c4e5e39c9e5e412702beb4a64 from master

Note: See TracTickets for help on using tickets.
Back to Top