[Django] #31012: Required SelectDateWidget renders invalid HTML
Django
-----------------------------------------+------------------------
Reporter: Kevin Brown | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 3.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
When you render a `SelectDateWidget` for a field marked as required, I
noticed it produced output that does not meet the HTML standard per
https://www.w3.org/TR/html5/sec-forms.html#placeholder-label-option. This
was called out in the 3.0 release notes so I checked out #29056 and
determined that was likely the cause.
----
The specific issue is that a `SelectDateWidget`, when specified as a
required field, will generate a `placeholder` attribute on each of the
`<select>` boxes which it generates. The `placeholder` attribute is not
valid for a `<select>` box and instead a "placeholder" option must be
specified as an `<option>` with an empty value (`value=""`) with the text
containing the placeholder to display.
From a Django forms perspective, this would be equivalent to setting the
`empty_label` for the `SelectDateWidget` to `(_('Year'), _('Month'),
_('Day'))`, which would generate each of the placeholder label options for
each of the corresponding `<select>` boxes used by the widget.
It's also important to note that the HTML generated for the
`SelectDateWidget` in previous versions of Django was valid per the HTML
standard. They did not contain a placeholder label option, but because
they were not a `<select multiple>` they always contained an option with a
selectedness set to true (usually the first option). This meant that the
`required` attribute generated for them was useless, since it would never
not have a value or be empty, but it did mean it was still valid HTML.
----
Here's some relevant parts of the standard when it comes to required
select boxes.
> If a select element has a required attribute specified, does not have a
multiple attribute specified, and has a display size of 1; and if the
value of the first option element in the select element's list of options
(if any) is the empty string, and that option element's parent node is the
select element (and not an optgroup element), then that option is the
select element's placeholder label option.
>
> If a select element has a required attribute specified, does not have a
multiple attribute specified, and has a display size of 1, then the select
element must have a placeholder label option.
>
> Constraint validation: If the element has its required attribute
specified, and either none of the option elements in the select element's
list of options have their selectedness set to true, or the only option
element in the select element's list of options with its selectedness set
to true is the placeholder label option, then the element is suffering
from being missing.
You can also find the list of accepted attributes for a `<select>` at
https://html.spec.whatwg.org/multipage/form-elements.html#the-select-
element and https://developer.mozilla.org/en-
US/docs/Web/HTML/Element/select#Attributes. Note that neither of those
pages specify that a `<select>` element may have a `placeholder`
attribute.
--
Ticket URL: <https://code.djangoproject.com/ticket/31012>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Kevin Brown | Owner: nobody
Component: Forms | Version: 3.0
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Baptiste Mispelon):
* status: new => closed
* resolution: => duplicate
Comment:
Hi,
Thanks for the report.
I've reopened the original ticket and updated its severity to "release
blocker" so I will close this one as a duplicate and the discussion can
continue over there.
--
Ticket URL: <https://code.djangoproject.com/ticket/31012#comment:1>
Django
Reporter: Kevin Brown | Owner: nobody
Component: Forms | Version: 3.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: closed => new
* severity: Normal => Release blocker
* resolution: duplicate =>
* has_patch: 0 => 1
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
Comment:
This isn't quite a duplicate: a regression was introduced in #31012. We'll
track resolving that here.
I'm reviewing this now, but will Accept and mark as RB, since Baptiste is
on the case already.
[https://github.com/django/django/pull/12116 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/31012#comment:2>
Django
Reporter: Kevin Brown | Owner: nobody
Component: Forms | Version: 3.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Update: good analysis Kevin, thank you, exactly on mark. To the PR...
--
Ticket URL: <https://code.djangoproject.com/ticket/31012#comment:3>
Django
Reporter: Kevin Brown | Owner: nobody
Component: Forms | Version: 3.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hasan Ramezani):
* cc: Hasan Ramezani (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/31012#comment:4>
Django
Reporter: Kevin Brown | Owner: nobody
Component: Forms | Version: 3.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
Comment:
As per the PR, I'm minded to revert
f038214d917c982613f5a15db8dfe325b1f7479b for 3.0 and allow a new ticket to
add `required` handling as a New Feature.
--
Ticket URL: <https://code.djangoproject.com/ticket/31012#comment:5>
Django
Reporter: Kevin Brown | Owner: nobody
Component: Forms | Version: 3.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"ee4a19053a32d41cdd79e087b1968980804ce658" ee4a1905]:
{{{
#!CommitTicketReference repository=""
revision="ee4a19053a32d41cdd79e087b1968980804ce658"
Fixed #31012 -- Reverted "Fixed #29056 -- Fixed HTML5 validation of
required SelectDateWidget."
This reverts commit f038214d917c982613f5a15db8dfe325b1f7479b.
The initial issue was incorrect. Django 2.2, and before, did not
generate invalid HTML as reported. With f03821 in place invalid HTML
was generated.
Thanks to Kevin Brown for follow-up report and investigation.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31012#comment:6>
Django
Reporter: Kevin Brown | Owner: nobody
Component: Forms | Version: 3.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"947f8e3485b1406d18ea062fd275c083d7ffbed5" 947f8e3]:
{{{
#!CommitTicketReference repository=""
revision="947f8e3485b1406d18ea062fd275c083d7ffbed5"
[3.0.x] Fixed #31012 -- Reverted "Fixed #29056 -- Fixed HTML5 validation
of required SelectDateWidget."
This reverts commit f038214d917c982613f5a15db8dfe325b1f7479b.
The initial issue was incorrect. Django 2.2, and before, did not
generate invalid HTML as reported. With f03821 in place invalid HTML
was generated.
Thanks to Kevin Brown for follow-up report and investigation.
Backport of ee4a19053a32d41cdd79e087b1968980804ce658 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31012#comment:7>