Skip to content

Commit

Permalink
Fixed #27370 -- Fixed select HTML validator issue
Browse files Browse the repository at this point in the history
Fixed issue where we caused HTML to be invalid by specifying required
on select field with nonempty first value.

validator/validator#47
  • Loading branch information
stlk committed Nov 11, 2016
1 parent cb3fb34 commit 4c9a258
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 27 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,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>
Expand Down
24 changes: 24 additions & 0 deletions django/forms/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,30 @@ def render_options(self, selected_choices):
output.append(self.render_option(selected_choices, option_value, option_label))
return '\n'.join(output)

def choice_has_empty_value(self, choice):
"""
Checks whenever choice value is empty string or None.
"""
choice_value = choice[0]
if isinstance(choice_value, six.string_types):
return len(choice_value) == 0
return choice_value is None

def use_required_attribute(self, initial):
"""
If first choice has a value, rendering required attribute causes HTML
validation error.
"""
super_result = super(Select, self).use_required_attribute(initial)

# Further checks are needed only when HTML element select is used and
# only one choice can be selected.
if self.allow_multiple_selected or hasattr(self, 'renderer'):
return super_result

first_choice = next(iter(self.choices), None)
return super_result and first_choice is not None and self.choice_has_empty_value(first_choice)


class NullBooleanSelect(Select):
"""
Expand Down
33 changes: 32 additions & 1 deletion tests/forms_tests/field_tests/test_choicefield.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,41 @@ def test_choicefield_disabled(self):
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>'
)

def test_choicefield_doesnt_render_required_when_impossible_to_select_empty_field(self):
self.assertWidgetRendersTo(
ChoiceField(choices=[('J', 'John'), ('P', 'Paul')]),
'<select id="id_f" name="f"><option value="J">John</option>'
'<option value="P">Paul</option></select>'
)

def test_choicefield_does_render_required_when_possible_to_select_empty_field_str(self):
self.assertWidgetRendersTo(
ChoiceField(choices=[('', 'select please'), ('P', 'Paul')]),
'<select id="id_f" name="f" required><option selected value="">select please</option>'
'<option value="P">Paul</option></select>'
)

def test_choicefield_does_render_required_when_possible_to_select_empty_field_list(self):
self.assertWidgetRendersTo(
ChoiceField(choices=[['', 'select please'], ['P', 'Paul']]),
'<select id="id_f" name="f" required><option selected value="">select please</option>'
'<option value="P">Paul</option></select>'
)

def test_choicefield_does_render_required_when_possible_to_select_empty_field_none(self):
self.assertWidgetRendersTo(
ChoiceField(choices=[(None, 'select please'), ('P', 'Paul')]),
'<select id="id_f" name="f" required><option selected value="">select please</option>'
'<option value="P">Paul</option></select>'
)

def test_choicefield_doesnt_render_required_when_no_choices_are_available(self):
self.assertWidgetRendersTo(ChoiceField(choices=[]), '<select id="id_f" name="f"></select>')

@ignore_warnings(category=UnicodeWarning)
def test_utf8_bytesrings(self):
# Choice validation with UTF-8 bytestrings as input (these are the
Expand Down
40 changes: 20 additions & 20 deletions tests/forms_tests/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -502,12 +502,12 @@ class FrameworkForm(Form):
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>""")
Expand All @@ -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']), """<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>""")
Expand All @@ -552,12 +552,12 @@ class FrameworkForm(Form):
)

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>""")
Expand All @@ -568,10 +568,10 @@ class FrameworkForm(Form):
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>""")
Expand Down Expand Up @@ -2318,37 +2318,37 @@ class Person(Form):
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>
Expand Down Expand Up @@ -2714,7 +2714,7 @@ class Person(Form):
"""<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>
Expand All @@ -2730,7 +2730,7 @@ class Person(Form):
<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>
Expand All @@ -2748,7 +2748,7 @@ class Person(Form):
<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>
Expand Down Expand Up @@ -3473,7 +3473,7 @@ class MyForm(Form):
'<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>',
Expand All @@ -3485,7 +3485,7 @@ class MyForm(Form):
'<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>',
Expand All @@ -3499,7 +3499,7 @@ class MyForm(Form):
'<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>',
Expand Down
8 changes: 4 additions & 4 deletions tests/forms_tests/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ def test_callable_initial_value(self):
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>
Expand Down Expand Up @@ -145,12 +145,12 @@ def test_initial_instance_value(self):
'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>
Expand Down
4 changes: 2 additions & 2 deletions tests/model_forms/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2643,11 +2643,11 @@ class Meta:
<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>
Expand Down

0 comments on commit 4c9a258

Please sign in to comment.