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 6, 2016
1 parent cb3fb34 commit b3424e0
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 27 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -389,6 +389,7 @@ answer newbie questions, and generally made Django that much better:
Jordi J. Tablada <jordi.joan@gmail.com>
Jorge Bastida <me@jorgebastida.com>
Jorge Gajon <gajon@gajon.org>
Josef Rousek <josef.rousek@gmail.com>
José Tomás Tocino García <josetomas.tocino@gmail.com>
Joseph Kocherhans <joseph@jkocherhans.com>
Josh Smeaton <josh.smeaton@gmail.com>
Expand Down
18 changes: 18 additions & 0 deletions django/forms/widgets.py
Expand Up @@ -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):
"""
Expand Down
41 changes: 40 additions & 1 deletion tests/forms_tests/field_tests/test_choicefield.py
Expand Up @@ -82,10 +82,49 @@ 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):
f = ChoiceField(choices=[('J', 'John'), ('P', 'Paul')])
self.assertWidgetRendersTo(
f,
'<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):
f = ChoiceField(choices=[('', 'select please'), ('P', 'Paul')])
self.assertWidgetRendersTo(
f,
'<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_array(self):
f = ChoiceField(choices=[['', 'select please'], ['P', 'Paul']])
self.assertWidgetRendersTo(
f,
'<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):
f = ChoiceField(choices=[(None, 'select please'), ('P', 'Paul')])
self.assertWidgetRendersTo(
f,
'<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):
f = ChoiceField(choices=[])
self.assertWidgetRendersTo(
f,
'<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
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
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
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 b3424e0

Please sign in to comment.