diff options
| author | Josef Rousek <josef.rousek@gmail.com> | 2016-11-05 12:27:44 +0100 |
|---|---|---|
| committer | Tim Graham <timograham@gmail.com> | 2016-12-28 10:45:22 -0500 |
| commit | aaecf038cae61f114db396f74e06759c95f21e93 (patch) | |
| tree | 84ec007f12503ce6ccb5ae39850654c5da3e303f | |
| parent | 6dbe56ed7855f34585884a2381fb1cec22ddc824 (diff) | |
Fixed #27370 -- Prevented Select widget from using 'required' with a non-empty first value.
| -rw-r--r-- | AUTHORS | 1 | ||||
| -rw-r--r-- | django/forms/widgets.py | 22 | ||||
| -rw-r--r-- | tests/forms_tests/field_tests/test_choicefield.py | 2 | ||||
| -rw-r--r-- | tests/forms_tests/tests/test_forms.py | 40 | ||||
| -rw-r--r-- | tests/forms_tests/tests/tests.py | 8 | ||||
| -rw-r--r-- | tests/forms_tests/widget_tests/test_select.py | 20 | ||||
| -rw-r--r-- | tests/model_forms/tests.py | 4 |
7 files changed, 70 insertions, 27 deletions
@@ -397,6 +397,7 @@ answer newbie questions, and generally made Django that much better: Jorge Bastida <me@jorgebastida.com> Jorge Gajon <gajon@gajon.org> José Tomás Tocino García <josetomas.tocino@gmail.com> + Josef Rousek <josef.rousek@gmail.com> Joseph Kocherhans <joseph@jkocherhans.com> Josh Smeaton <josh.smeaton@gmail.com> Joshua Ginsberg <jag@flowtheory.net> diff --git a/django/forms/widgets.py b/django/forms/widgets.py index dd68662d43..4f028d34ad 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -680,6 +680,28 @@ class Select(ChoiceWidget): context['widget']['attrs']['multiple'] = 'multiple' return context + @staticmethod + def _choice_has_empty_value(choice): + """Return True if the choice's value is empty string or None.""" + value, _ = choice + return ( + (isinstance(value, six.string_types) and not bool(value)) or + value is None + ) + + def use_required_attribute(self, initial): + """ + Don't render 'required' if the first <option> has a value, as that's + invalid HTML. + """ + use_required_attribute = super(Select, self).use_required_attribute(initial) + # 'required' is always okay for <select multiple>. + if self.allow_multiple_selected: + return use_required_attribute + + first_choice = next(iter(self.choices), None) + return use_required_attribute and first_choice is not None and self._choice_has_empty_value(first_choice) + class NullBooleanSelect(Select): """ diff --git a/tests/forms_tests/field_tests/test_choicefield.py b/tests/forms_tests/field_tests/test_choicefield.py index 4c70177dac..1d8fe5a3cf 100644 --- a/tests/forms_tests/field_tests/test_choicefield.py +++ b/tests/forms_tests/field_tests/test_choicefield.py @@ -82,7 +82,7 @@ class ChoiceFieldTest(FormFieldAssertionsMixin, SimpleTestCase): f = ChoiceField(choices=[('J', 'John'), ('P', 'Paul')], disabled=True) self.assertWidgetRendersTo( f, - '<select id="id_f" name="f" disabled required><option value="J">John</option>' + '<select id="id_f" name="f" disabled><option value="J">John</option>' '<option value="P">Paul</option></select>' ) diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py index 4079be1235..2b93575bf2 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -502,12 +502,12 @@ class FormsTestCase(SimpleTestCase): language = ChoiceField(choices=[('P', 'Python'), ('J', 'Java')]) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """<select name="language" required> + self.assertHTMLEqual(str(f['language']), """<select name="language"> <option value="P">Python</option> <option value="J">Java</option> </select>""") f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """<select name="language" required> + self.assertHTMLEqual(str(f['language']), """<select name="language"> <option value="P" selected>Python</option> <option value="J">Java</option> </select>""") @@ -531,12 +531,12 @@ class FormsTestCase(SimpleTestCase): language = ChoiceField(choices=[('P', 'Python'), ('J', 'Java')], widget=Select(attrs={'class': 'foo'})) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """<select class="foo" name="language" required> + self.assertHTMLEqual(str(f['language']), """<select class="foo" name="language"> <option value="P">Python</option> <option value="J">Java</option> </select>""") f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """<select class="foo" name="language" required> + self.assertHTMLEqual(str(f['language']), """<select class="foo" name="language"> <option value="P" selected>Python</option> <option value="J">Java</option> </select>""") @@ -552,12 +552,12 @@ class FormsTestCase(SimpleTestCase): ) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """<select class="foo" name="language" required> + self.assertHTMLEqual(str(f['language']), """<select class="foo" name="language"> <option value="P">Python</option> <option value="J">Java</option> </select>""") f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """<select class="foo" name="language" required> + self.assertHTMLEqual(str(f['language']), """<select class="foo" name="language"> <option value="P" selected>Python</option> <option value="J">Java</option> </select>""") @@ -568,10 +568,10 @@ class FormsTestCase(SimpleTestCase): language = ChoiceField() f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """<select name="language" required> + self.assertHTMLEqual(str(f['language']), """<select name="language"> </select>""") f.fields['language'].choices = [('P', 'Python'), ('J', 'Java')] - self.assertHTMLEqual(str(f['language']), """<select name="language" required> + self.assertHTMLEqual(str(f['language']), """<select name="language"> <option value="P">Python</option> <option value="J">Java</option> </select>""") @@ -2363,37 +2363,37 @@ Password: <input type="password" name="password" required /> is_cool = NullBooleanField() p = Person({'name': 'Joe'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool" required> + self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool"> <option value="1" selected>Unknown</option> <option value="2">Yes</option> <option value="3">No</option> </select>""") p = Person({'name': 'Joe', 'is_cool': '1'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool" required> + self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool"> <option value="1" selected>Unknown</option> <option value="2">Yes</option> <option value="3">No</option> </select>""") p = Person({'name': 'Joe', 'is_cool': '2'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool" required> + self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool"> <option value="1">Unknown</option> <option value="2" selected>Yes</option> <option value="3">No</option> </select>""") p = Person({'name': 'Joe', 'is_cool': '3'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool" required> + self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool"> <option value="1">Unknown</option> <option value="2">Yes</option> <option value="3" selected>No</option> </select>""") p = Person({'name': 'Joe', 'is_cool': True}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool" required> + self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool"> <option value="1">Unknown</option> <option value="2" selected>Yes</option> <option value="3">No</option> </select>""") p = Person({'name': 'Joe', 'is_cool': False}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool" required> + self.assertHTMLEqual(str(p['is_cool']), """<select name="is_cool"> <option value="1">Unknown</option> <option value="2">Yes</option> <option value="3" selected>No</option> @@ -2759,7 +2759,7 @@ Good luck picking a username that doesn't already exist.</p> """<li class="required error"><ul class="errorlist"><li>This field is required.</li></ul> <label class="required" for="id_name">Name:</label> <input type="text" name="name" id="id_name" required /></li> <li class="required"><label class="required" for="id_is_cool">Is cool:</label> -<select name="is_cool" id="id_is_cool" required> +<select name="is_cool" id="id_is_cool"> <option value="1" selected>Unknown</option> <option value="2">Yes</option> <option value="3">No</option> @@ -2775,7 +2775,7 @@ Good luck picking a username that doesn't already exist.</p> <p class="required error"><label class="required" for="id_name">Name:</label> <input type="text" name="name" id="id_name" required /></p> <p class="required"><label class="required" for="id_is_cool">Is cool:</label> -<select name="is_cool" id="id_is_cool" required> +<select name="is_cool" id="id_is_cool"> <option value="1" selected>Unknown</option> <option value="2">Yes</option> <option value="3">No</option> @@ -2793,7 +2793,7 @@ Good luck picking a username that doesn't already exist.</p> <td><ul class="errorlist"><li>This field is required.</li></ul> <input type="text" name="name" id="id_name" required /></td></tr> <tr class="required"><th><label class="required" for="id_is_cool">Is cool:</label></th> -<td><select name="is_cool" id="id_is_cool" required> +<td><select name="is_cool" id="id_is_cool"> <option value="1" selected>Unknown</option> <option value="2">Yes</option> <option value="3">No</option> @@ -3516,7 +3516,7 @@ Good luck picking a username that doesn't already exist.</p> '<p><label for="id_f2">F2:</label> <input id="id_f2" maxlength="30" name="f2" type="text" /></p>' '<p><label for="id_f3">F3:</label> <textarea cols="40" id="id_f3" name="f3" rows="10" required>' '</textarea></p>' - '<p><label for="id_f4">F4:</label> <select id="id_f4" name="f4" required>' + '<p><label for="id_f4">F4:</label> <select id="id_f4" name="f4">' '<option value="P">Python</option>' '<option value="J">Java</option>' '</select></p>', @@ -3528,7 +3528,7 @@ Good luck picking a username that doesn't already exist.</p> '<li><label for="id_f2">F2:</label> <input id="id_f2" maxlength="30" name="f2" type="text" /></li>' '<li><label for="id_f3">F3:</label> <textarea cols="40" id="id_f3" name="f3" rows="10" required>' '</textarea></li>' - '<li><label for="id_f4">F4:</label> <select id="id_f4" name="f4" required>' + '<li><label for="id_f4">F4:</label> <select id="id_f4" name="f4">' '<option value="P">Python</option>' '<option value="J">Java</option>' '</select></li>', @@ -3542,7 +3542,7 @@ Good luck picking a username that doesn't already exist.</p> '<tr><th><label for="id_f3">F3:</label></th>' '<td><textarea cols="40" id="id_f3" name="f3" rows="10" required>' '</textarea></td></tr>' - '<tr><th><label for="id_f4">F4:</label></th><td><select id="id_f4" name="f4" required>' + '<tr><th><label for="id_f4">F4:</label></th><td><select id="id_f4" name="f4">' '<option value="P">Python</option>' '<option value="J">Java</option>' '</select></td></tr>', diff --git a/tests/forms_tests/tests/tests.py b/tests/forms_tests/tests/tests.py index c19d1a66cd..f0007fbe8f 100644 --- a/tests/forms_tests/tests/tests.py +++ b/tests/forms_tests/tests/tests.py @@ -109,12 +109,12 @@ class ModelFormCallableModelDefault(TestCase): ChoiceOptionModel.objects.create(id=3, name='option 3') self.assertHTMLEqual( ChoiceFieldForm().as_p(), - """<p><label for="id_choice">Choice:</label> <select name="choice" id="id_choice" required> + """<p><label for="id_choice">Choice:</label> <select name="choice" id="id_choice"> <option value="1" selected>ChoiceOption 1</option> <option value="2">ChoiceOption 2</option> <option value="3">ChoiceOption 3</option> </select><input type="hidden" name="initial-choice" value="1" id="initial-id_choice" /></p> -<p><label for="id_choice_int">Choice int:</label> <select name="choice_int" id="id_choice_int" required> +<p><label for="id_choice_int">Choice int:</label> <select name="choice_int" id="id_choice_int"> <option value="1" selected>ChoiceOption 1</option> <option value="2">ChoiceOption 2</option> <option value="3">ChoiceOption 3</option> @@ -145,12 +145,12 @@ class ModelFormCallableModelDefault(TestCase): 'multi_choice': [obj2, obj3], 'multi_choice_int': ChoiceOptionModel.objects.exclude(name="default"), }).as_p(), - """<p><label for="id_choice">Choice:</label> <select name="choice" id="id_choice" required> + """<p><label for="id_choice">Choice:</label> <select name="choice" id="id_choice"> <option value="1">ChoiceOption 1</option> <option value="2" selected>ChoiceOption 2</option> <option value="3">ChoiceOption 3</option> </select><input type="hidden" name="initial-choice" value="2" id="initial-id_choice" /></p> -<p><label for="id_choice_int">Choice int:</label> <select name="choice_int" id="id_choice_int" required> +<p><label for="id_choice_int">Choice int:</label> <select name="choice_int" id="id_choice_int"> <option value="1">ChoiceOption 1</option> <option value="2" selected>ChoiceOption 2</option> <option value="3">ChoiceOption 3</option> diff --git a/tests/forms_tests/widget_tests/test_select.py b/tests/forms_tests/widget_tests/test_select.py index 4b717b6043..d2660ea787 100644 --- a/tests/forms_tests/widget_tests/test_select.py +++ b/tests/forms_tests/widget_tests/test_select.py @@ -294,3 +294,23 @@ class SelectTest(WidgetTest): self.assertIsNot(widget.choices, obj.choices) self.assertEqual(widget.attrs, obj.attrs) self.assertIsNot(widget.attrs, obj.attrs) + + def test_doesnt_render_required_when_impossible_to_select_empty_field(self): + widget = self.widget(choices=[('J', 'John'), ('P', 'Paul')]) + self.assertIs(widget.use_required_attribute(initial=None), False) + + def test_renders_required_when_possible_to_select_empty_field_str(self): + widget = self.widget(choices=[('', 'select please'), ('P', 'Paul')]) + self.assertIs(widget.use_required_attribute(initial=None), True) + + def test_renders_required_when_possible_to_select_empty_field_list(self): + widget = self.widget(choices=[['', 'select please'], ['P', 'Paul']]) + self.assertIs(widget.use_required_attribute(initial=None), True) + + def test_renders_required_when_possible_to_select_empty_field_none(self): + widget = self.widget(choices=[(None, 'select please'), ('P', 'Paul')]) + self.assertIs(widget.use_required_attribute(initial=None), True) + + def test_doesnt_render_required_when_no_choices_are_available(self): + widget = self.widget(choices=[]) + self.assertIs(widget.use_required_attribute(initial=None), False) diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index f03b4433a8..1686629c3a 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -2635,11 +2635,11 @@ class OtherModelFormTests(TestCase): <p><label for="id_date_published">Date published:</label> <input id="id_date_published" name="date_published" type="text" value="{0}" required /> <input id="initial-id_date_published" name="initial-date_published" type="hidden" value="{0}" /></p> - <p><label for="id_mode">Mode:</label> <select id="id_mode" name="mode" required> + <p><label for="id_mode">Mode:</label> <select id="id_mode" name="mode"> <option value="di" selected>direct</option> <option value="de">delayed</option></select> <input id="initial-id_mode" name="initial-mode" type="hidden" value="di" /></p> - <p><label for="id_category">Category:</label> <select id="id_category" name="category" required> + <p><label for="id_category">Category:</label> <select id="id_category" name="category"> <option value="1">Games</option> <option value="2">Comics</option> <option value="3" selected>Novel</option></select> |
