From b3424e0d5b7af1bfef1de01bd25b2173cc6fb4af Mon Sep 17 00:00:00 2001 From: Josef Rousek Date: Sat, 5 Nov 2016 12:27:44 +0100 Subject: [PATCH] Fixed #27370 -- Fixed select HTML validator issue Fixed issue where we caused HTML to be invalid by specifying required on select field with nonempty first value. https://github.com/validator/validator/issues/47 --- AUTHORS | 1 + django/forms/widgets.py | 18 ++++++++ .../field_tests/test_choicefield.py | 41 ++++++++++++++++++- tests/forms_tests/tests/test_forms.py | 40 +++++++++--------- tests/forms_tests/tests/tests.py | 8 ++-- tests/model_forms/tests.py | 4 +- 6 files changed, 85 insertions(+), 27 deletions(-) diff --git a/AUTHORS b/AUTHORS index 64706927e0de3..1f13f8958b283 100644 --- a/AUTHORS +++ b/AUTHORS @@ -389,6 +389,7 @@ answer newbie questions, and generally made Django that much better: Jordi J. Tablada Jorge Bastida Jorge Gajon + Josef Rousek José Tomás Tocino García Joseph Kocherhans Josh Smeaton diff --git a/django/forms/widgets.py b/django/forms/widgets.py index 7c7e2c90febb9..9382c41e793d6 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -573,6 +573,24 @@ def render_options(self, selected_choices): output.append(self.render_option(selected_choices, option_value, option_label)) return '\n'.join(output) + def option_has_empty_value(self, choice): + choice_value = choice[0] + if isinstance(choice_value, str): + return len(choice_value) == 0 + return choice_value is None + + def use_required_attribute(self, initial): + # If first item has a value, rendering required attribute causes HTML validation error. + # More info: https://github.com/validator/validator/issues/47#issuecomment-77534952 + super_result = super(Select, self).use_required_attribute(initial) + + # Do checks only for select itself, not for its children + if self.allow_multiple_selected or hasattr(self, 'renderer'): + return super_result + else: + first_choice = next(iter(self.choices), None) + return super_result and not first_choice is None and self.option_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 4c70177dac1a7..69cfad900c691 100644 --- a/tests/forms_tests/field_tests/test_choicefield.py +++ b/tests/forms_tests/field_tests/test_choicefield.py @@ -82,10 +82,49 @@ def test_choicefield_disabled(self): f = ChoiceField(choices=[('J', 'John'), ('P', 'Paul')], disabled=True) self.assertWidgetRendersTo( f, - '' '' ) + def test_choicefield_doesnt_render_required_when_impossible_to_select_empty_field(self): + f = ChoiceField(choices=[('J', 'John'), ('P', 'Paul')]) + self.assertWidgetRendersTo( + f, + '' + ) + + def test_choicefield_does_render_required_when_possible_to_select_empty_field_str(self): + f = ChoiceField(choices=[('', 'select please'), ('P', 'Paul')]) + self.assertWidgetRendersTo( + f, + '' + ) + + def test_choicefield_does_render_required_when_possible_to_select_empty_field_array(self): + f = ChoiceField(choices=[['', 'select please'], ['P', 'Paul']]) + self.assertWidgetRendersTo( + f, + '' + ) + + def test_choicefield_does_render_required_when_possible_to_select_empty_field_none(self): + f = ChoiceField(choices=[(None, 'select please'), ('P', 'Paul')]) + self.assertWidgetRendersTo( + f, + '' + ) + + def test_choicefield_doesnt_render_required_when_no_choices_are_available(self): + f = ChoiceField(choices=[]) + self.assertWidgetRendersTo( + f, + '' + ) + @ignore_warnings(category=UnicodeWarning) def test_utf8_bytesrings(self): # Choice validation with UTF-8 bytestrings as input (these are the diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py index 5ae4ca042fb88..5993400332a85 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -502,12 +502,12 @@ class FrameworkForm(Form): language = ChoiceField(choices=[('P', 'Python'), ('J', 'Java')]) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) @@ -531,12 +531,12 @@ class FrameworkForm(Form): language = ChoiceField(choices=[('P', 'Python'), ('J', 'Java')], widget=Select(attrs={'class': 'foo'})) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) @@ -552,12 +552,12 @@ class FrameworkForm(Form): ) f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) f = FrameworkForm({'name': 'Django', 'language': 'P'}, auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) @@ -568,10 +568,10 @@ class FrameworkForm(Form): language = ChoiceField() f = FrameworkForm(auto_id=False) - self.assertHTMLEqual(str(f['language']), """ """) f.fields['language'].choices = [('P', 'Python'), ('J', 'Java')] - self.assertHTMLEqual(str(f['language']), """ """) @@ -2318,37 +2318,37 @@ class Person(Form): is_cool = NullBooleanField() p = Person({'name': 'Joe'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': '1'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': '2'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': '3'}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': True}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ """) p = Person({'name': 'Joe', 'is_cool': False}, auto_id=False) - self.assertHTMLEqual(str(p['is_cool']), """ @@ -2714,7 +2714,7 @@ class Person(Form): """
    • This field is required.
  • - @@ -2730,7 +2730,7 @@ class Person(Form):

    - @@ -2748,7 +2748,7 @@ class Person(Form):

    • This field is required.
    - @@ -3473,7 +3473,7 @@ class MyForm(Form): '

    ' '

    ' - '

    ' '' '' '

    ', @@ -3485,7 +3485,7 @@ class MyForm(Form): '
    • ' '
    • ' - '
  • ' '' '' '
    • ', @@ -3499,7 +3499,7 @@ class MyForm(Form): '' '' - '' '' '' '', diff --git a/tests/forms_tests/tests/tests.py b/tests/forms_tests/tests/tests.py index 81a6feef2a50c..9fffe9a35c9af 100644 --- a/tests/forms_tests/tests/tests.py +++ b/tests/forms_tests/tests/tests.py @@ -109,12 +109,12 @@ def test_callable_initial_value(self): ChoiceOptionModel.objects.create(id=3, name='option 3') self.assertHTMLEqual( ChoiceFieldForm().as_p(), - """

    -

    @@ -145,12 +145,12 @@ def test_initial_instance_value(self): 'multi_choice': [obj2, obj3], 'multi_choice_int': ChoiceOptionModel.objects.exclude(name="default"), }).as_p(), - """

    -

    diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index 289d8604b3764..90b19ad4107dd 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -2643,11 +2643,11 @@ class Meta:

    -

    -