#27370 closed Cleanup/optimization (fixed)

Django's Select widget adds a required="required" attribute, even if created with empty_label=True

Reported by: alexpirine Owned by: Josef Rousek
Component: Forms Version: master
Severity: Normal Keywords: HTML, Select, wdiget, ModelChoiceField
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by alexpirine)

I didn't think it was wrong in HTML, but the W3C validator says so, and after reading this discussion, it seems to be right:

https://github.com/validator/validator/issues/47

This is how I could explain it:

If a form field is required and has the empty_label=True argument, the generated <select> will have a required="required" attribute. This doesn't make sense since the first item will be selected anyways, and there is no way for the user to unselect a value.

So, the required="required" attribute is useless.

Example:

class TestForm(forms.Form):
    some_field = forms.ModelChoiceField(queryset=SomeModel.objects.all(), empty_label=None)

results in:

<select id="id_some_field" name="some_field" required>
<option value="1">Value 1</option>
<option value="2">Value 2</option>
<option value="3">Value 3</option>
<!-- ... -->
</select>

You can check the following code using the W3C validation service:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <title>Validation test</title>
</head>
<body>
  <select id="id_some_field" name="some_field" required>
    <option value="1">Value 1</option>
    <option value="2">Value 2</option>
    <option value="3">Value 3</option>
    <!-- ... -->
  </select>
</body>
</html>

In order to fix this, the generated <select> widget should not have a required attribute.

Change History (14)

comment:1 Changed 15 months ago by alexpirine

Description: modified (diff)

comment:2 Changed 15 months ago by alexpirine

Description: modified (diff)

comment:3 Changed 15 months ago by Claude Paroz

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization
Version: 1.10master

Here's the validator error message:
The first child option element of a select element with a required attribute, and without a multiple attribute, and without a size attribute whose value is greater than 1, must have either an empty value attribute, or must have no text content. Consider either adding a placeholder option label, or adding a size attribute with a value equal to the number of option elements.
At first sight, this should be doable without difficulty in the widget's use_required_attribute method.

comment:4 Changed 15 months ago by Andrew Nester

Agree with Claude Paroz - all we need to do is to set use_required_attribute = False in form where such field used.
Something like this:

class TestForm(forms.Form):
    use_required_attribute = False

    some_field = forms.ModelChoiceField(queryset=SomeModel.objects.all(), empty_label=None)

I guess we don't need some additional changes for this, what do you think @alexpirine?

comment:5 Changed 15 months ago by Claude Paroz

If Django has all needed elements to determine the value of use_required_attribute in such a situation, it should do it automatically and not require the user to add it explicitly.

comment:6 Changed 15 months ago by alexpirine

I agree with Claude.

Also, use_required_attribute applies to all fields, which is not desired here. We only want to remove the required attribute from the Select widget.

comment:7 Changed 15 months ago by Jon Dufresne

alexpirine, I believe Claude is referring to Widget.use_required_attribute, not Form.use_required_attribute. For reference, this method will be documented soon.

comment:8 Changed 15 months ago by Josef Rousek

Owner: changed from nobody to Josef Rousek
Status: newassigned

comment:9 Changed 15 months ago by Josef Rousek

Has patch: set

comment:10 Changed 15 months ago by alexpirine

I reviewed the code:

The overall logic and tests seem to be correct, except in case of an empty <select>.

But I can not make a full judgment about the code, since I lack the understanding of some parts of widgets.py. For instance I didn't know anything about the renderer attribute (I still can not find it in documentation).

comment:11 in reply to:  10 Changed 15 months ago by Josef Rousek

Replying to alexpirine:

I reviewed the code:

The overall logic and tests seem to be correct, except in case of an empty <select>.

But I can not make a full judgment about the code, since I lack the understanding of some parts of widgets.py. For instance I didn't know anything about the renderer attribute (I still can not find it in documentation).

Thanks for review! I'll fix those issues.

comment:12 Changed 14 months ago by Simon Charette

Triage Stage: AcceptedReady for checkin

Patch LGTM pending some cosmetic comments.

comment:13 Changed 14 months ago by Josef Rousek

I applied those cosmetic changes.

comment:14 Changed 13 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In aaecf038:

Fixed #27370 -- Prevented Select widget from using 'required' with a non-empty first value.

Note: See TracTickets for help on using tickets.
Back to Top