From fab46ce6f5a0a58c4e5e39c9e5e412702beb4a64 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Fri, 12 Aug 2016 10:59:01 -0700 Subject: [PATCH] Fixed #27037 -- Prevented required attribute on ClearableFileInput when initial data exists. --- django/forms/boundfield.py | 31 +++++++++++-------- django/forms/widgets.py | 14 ++++++--- docs/releases/1.10.1.txt | 3 ++ tests/forms_tests/tests/test_forms.py | 9 ++++++ .../test_checkboxselectmultiple.py | 8 +++++ .../widget_tests/test_clearablefileinput.py | 6 ++++ .../widget_tests/test_hiddeninput.py | 7 +++++ .../widget_tests/test_textinput.py | 6 ++++ 8 files changed, 66 insertions(+), 18 deletions(-) diff --git a/django/forms/boundfield.py b/django/forms/boundfield.py index e0ee6b4e78..1b5cbfc47c 100644 --- a/django/forms/boundfield.py +++ b/django/forms/boundfield.py @@ -134,18 +134,7 @@ class BoundField(object): the form is not bound or the data otherwise. """ if not self.form.is_bound: - data = self.form.initial.get(self.name, self.field.initial) - if callable(data): - if self._initial_value is not UNSET: - data = self._initial_value - else: - data = data() - # If this is an auto-generated default date, nix the - # microseconds for standardized handling. See #22502. - if (isinstance(data, (datetime.datetime, datetime.time)) and - not self.field.widget.supports_microseconds): - data = data.replace(microsecond=0) - self._initial_value = data + data = self.initial else: data = self.field.bound_data( self.data, self.form.initial.get(self.name, self.field.initial) @@ -231,11 +220,27 @@ class BoundField(object): id_ = widget.attrs.get('id') or self.auto_id return widget.id_for_label(id_) + @property + def initial(self): + data = self.form.initial.get(self.name, self.field.initial) + if callable(data): + if self._initial_value is not UNSET: + data = self._initial_value + else: + data = data() + # If this is an auto-generated default date, nix the + # microseconds for standardized handling. See #22502. + if (isinstance(data, (datetime.datetime, datetime.time)) and + not self.field.widget.supports_microseconds): + data = data.replace(microsecond=0) + self._initial_value = data + return data + def build_widget_attrs(self, attrs, widget=None): if not widget: widget = self.field.widget attrs = dict(attrs) # Copy attrs to avoid modifying the argument. - if not widget.is_hidden and self.field.required and self.form.use_required_attribute: + if widget.use_required_attribute(self.initial) and self.field.required and self.form.use_required_attribute: attrs['required'] = True if self.field.disabled: attrs['disabled'] = True diff --git a/django/forms/widgets.py b/django/forms/widgets.py index 13d22672bf..d6b84c10ab 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -248,6 +248,9 @@ class Widget(six.with_metaclass(RenameWidgetMethods)): """ return id_ + def use_required_attribute(self, initial): + return not self.is_hidden + class Input(Widget): """ @@ -429,6 +432,9 @@ class ClearableFileInput(FileInput): return False return upload + def use_required_attribute(self, initial): + return super(ClearableFileInput, self).use_required_attribute(initial) and not initial + class Textarea(Widget): def __init__(self, attrs=None): @@ -795,12 +801,10 @@ class CheckboxSelectMultiple(RendererMixin, SelectMultiple): renderer = CheckboxFieldRenderer _empty_value = [] - def build_attrs(self, extra_attrs=None, **kwargs): - attrs = super(CheckboxSelectMultiple, self).build_attrs(extra_attrs, **kwargs) - # Remove the 'required' attribute because browser validation would + def use_required_attribute(self, initial): + # Don't use the 'required' attribute because browser validation would # require all checkboxes to be checked instead of at least one. - attrs.pop('required', None) - return attrs + return False class MultiWidget(Widget): diff --git a/docs/releases/1.10.1.txt b/docs/releases/1.10.1.txt index 2ebd36cf0f..b834915701 100644 --- a/docs/releases/1.10.1.txt +++ b/docs/releases/1.10.1.txt @@ -45,3 +45,6 @@ Bugfixes * Fixed crash of ``django.views.static.serve()`` with ``show_indexes`` enabled (:ticket:`26973`). + +* Fixed ``ClearableFileInput`` to avoid the ``required`` HTML attribute when + initial data exists (:ticket:`27037`). diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py index 773b89a7bc..0e0ea9f1c2 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -2360,6 +2360,15 @@ Password: 'File1:', ) + # A required file field with initial data should not contain the + # required HTML attribute. The file input is left blank by the user to + # keep the existing, initial value. + f = FileForm(initial={'file1': 'resume.txt'}, auto_id=False) + self.assertHTMLEqual( + f.as_table(), + 'File1:', + ) + def test_basic_processing_in_view(self): class UserRegistration(Form): username = CharField(max_length=10) diff --git a/tests/forms_tests/widget_tests/test_checkboxselectmultiple.py b/tests/forms_tests/widget_tests/test_checkboxselectmultiple.py index 10d025e4c2..2d73f2b589 100644 --- a/tests/forms_tests/widget_tests/test_checkboxselectmultiple.py +++ b/tests/forms_tests/widget_tests/test_checkboxselectmultiple.py @@ -112,3 +112,11 @@ class CheckboxSelectMultipleTest(WidgetTest): """ self.check_html(widget, 'letters', ['a', 'c'], html=html) + + def test_use_required_attribute(self): + widget = self.widget(choices=self.beatles) + # Always False because browser validation would require all checkboxes + # to be checked instead of at least one. + self.assertIs(widget.use_required_attribute(None), False) + self.assertIs(widget.use_required_attribute([]), False) + self.assertIs(widget.use_required_attribute(['J', 'P']), False) diff --git a/tests/forms_tests/widget_tests/test_clearablefileinput.py b/tests/forms_tests/widget_tests/test_clearablefileinput.py index 720f17b035..3727d0c0df 100644 --- a/tests/forms_tests/widget_tests/test_clearablefileinput.py +++ b/tests/forms_tests/widget_tests/test_clearablefileinput.py @@ -143,3 +143,9 @@ class ClearableFileInputTest(WidgetTest): html = self.widget.render('myfile', NoURLFieldFile()) self.assertHTMLEqual(html, '') + + def test_use_required_attribute(self): + # False when initial data exists. The file input is left blank by the + # user to keep the existing, initial value. + self.assertIs(self.widget.use_required_attribute(None), True) + self.assertIs(self.widget.use_required_attribute('resume.txt'), False) diff --git a/tests/forms_tests/widget_tests/test_hiddeninput.py b/tests/forms_tests/widget_tests/test_hiddeninput.py index 039e89d5cc..d1604a31e4 100644 --- a/tests/forms_tests/widget_tests/test_hiddeninput.py +++ b/tests/forms_tests/widget_tests/test_hiddeninput.py @@ -8,3 +8,10 @@ class HiddenInputTest(WidgetTest): def test_render(self): self.check_html(self.widget, 'email', '', html='') + + def test_use_required_attribute(self): + # Always False to avoid browser validation on inputs hidden from the + # user. + self.assertIs(self.widget.use_required_attribute(None), False) + self.assertIs(self.widget.use_required_attribute(''), False) + self.assertIs(self.widget.use_required_attribute('foo'), False) diff --git a/tests/forms_tests/widget_tests/test_textinput.py b/tests/forms_tests/widget_tests/test_textinput.py index 33e455a160..65b5da4e03 100644 --- a/tests/forms_tests/widget_tests/test_textinput.py +++ b/tests/forms_tests/widget_tests/test_textinput.py @@ -76,3 +76,9 @@ class TextInputTest(WidgetTest): def test_attrs_safestring(self): widget = TextInput(attrs={'onBlur': mark_safe("function('foo')")}) self.check_html(widget, 'email', '', html='') + + def test_use_required_attribute(self): + # Text inputs can safely trigger the browser validation. + self.assertIs(self.widget.use_required_attribute(None), True) + self.assertIs(self.widget.use_required_attribute(''), True) + self.assertIs(self.widget.use_required_attribute('resume.txt'), True)