[Django] #29036: HTML5 required validation does not work for SelectDateWidget
Django
------------------------------------------+------------------------
Reporter: Vlastimil Zíma | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------+------------------------
`SelectDateWidget` uses `0` as a empty value. When the field uses HTML5
required attribute to make browser check the input, it does not work.
Browsers (tested with Firefox 52.5 and Chromium 62) consider selected `0`
as filled, thus `required` check incorrectly passes.
I suggest to use empty string as a value for
`SelectDateWidget.none_value`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29036>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Vlastimil Zíma | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Israel Saeta Pérez):
Hi Vlastimil!
I've tried to reproduce the issue you mention but it's not possible, since
when the form field is required the empty label option is not rendered.
From the documentation
https://docs.djangoproject.com/en/2.0/ref/forms/widgets/#django.forms.SelectDateWidget:
> If the DateField is not required, SelectDateWidget will have an empty
choice at the top of the list
You can see this in the code as well:
https://github.com/django/django/blob/47d238b69602711c06c369a5555bb554a4b3f7fb/django/forms/widgets.py#L955-L995
Therefore what you mention can never happen I believe. Can you provide a
code sample where you can reproduce the issue? Otherwise I'm in favor of
marking this as invalid.
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:1>
Django
Reporter: Vlastimil Zíma | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Herbert Fortes):
Hi,
I am searching for the 'Easy picking' ones. Let me comment:
It seems that when Django thinks that the field is not required
the value is 0, right?
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:2>
Django
Reporter: Vlastimil Zíma | Owner: nobody
Component: Forms | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------+--------------------------------------
Changes (by Tim Graham):
* status: new => closed
* resolution: => needsinfo
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:3>
Django
Reporter: Vlastimil Zíma | Owner: nobody
Component: Forms | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------+--------------------------------------
Comment (by Vlastimil Zíma):
Hi Israel,
we have a slightly more complicated use case, in which `required`
attribute is added by JavaScript depending on other values in the form.
Having 0 as a default value, makes things more complicated for us.
More cases might emerge based on browsers' behavior, which considers 0 as
selected.
Also looking around for HTML5 specification, it looks like hiding the
empty label for required select is not valid. According to
https://www.w3.org/TR/html5/sec-forms.html#placeholder-label-option the
empty labels must be present for `select`s with `required` attributes.
Based on Tims' closure, should I open a new ticket for that?
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:4>
Django
Reporter: Vlastimil Zíma | Owner: nobody
Component: Forms | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------+--------------------------------------
Changes (by Tim Graham):
* Attachment "29036.diff" added.
Django
attribute is added by JavaScript
Reporter: Vlastimil Zíma | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------+------------------------------------
Changes (by Tim Graham):
* status: closed => new
* stage: Unreviewed => Accepted
* resolution: needsinfo =>
Comment:
I guess fixing the use case you mentioned might be feasible. I've attached
an initial patch. More changes are required, for example
`SelectDateWidget.date_re` must be adapted. Someone else can continue the
work.
As for "hiding the empty label for required select is not valid", I'm not
sure what exactly the spec is saying. For example, what's a "placeholder
label option"? But yes, that seems like it would be a separate ticket,
although it couldn't be addressed until this one is fixed, I think.
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:5>
Django
attribute is added by JavaScript
Reporter: Vlastimil Zíma | Owner: Vlastimil Zíma
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------+------------------------------------------
Changes (by Vlastimil Zíma):
* owner: nobody => Vlastimil Zíma
* status: new => assigned
Comment:
Thanks for reopening. I've split the invalid HTML into a separate #29056.
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:6>
Django
attribute is added by JavaScript
--------------------------------+------------------------------------------
Reporter: Vlastimil Zíma | Owner: Vlastimil Zíma
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------+------------------------------------------
Changes (by Vlastimil Zíma):
* has_patch: 0 => 1
Comment:
PR submitted https://github.com/django/django/pull/9632
Instead of changing the `date_re`, I kept the pseudo-ISO format with zeros
for the invalid dates. I consider it easier to debug in case an error
should occur.
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:7>
Django
attribute is added by JavaScript
Reporter: Vlastimil Zíma | Owner: Vlastimil
| Zíma
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):
* cc: Carlton Gibson (added)
* stage: Accepted => Ready for checkin
Comment:
This looks good to me.
I think leaving the `date_re` using `0` for invalid values makes sense.
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:8>
Django
attribute is added by JavaScript
-------------------------------------+-------------------------------------
Reporter: Vlastimil Zíma | Owner: Vlastimil
| Zíma
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"fbc3c29e7cc5b3a3f62f1700af6ba474f05d59eb" fbc3c29]:
{{{
#!CommitTicketReference repository=""
revision="fbc3c29e7cc5b3a3f62f1700af6ba474f05d59eb"
Fixed #29036 -- Fixed HTML5 required validation on SelectDateWidget if the
attribute is added by JavaScript.
Thanks Tim Graham for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29036#comment:9>