[Django] #27015: Hidden widget shouldn't have maxlength/minlength attributes
Django
------------------------------------------------+------------------------
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | 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 |
------------------------------------------------+------------------------
This was reported against django-contrib-comments:
https://github.com/django/django-contrib-comments/pull/94
The `widget_attrs()` method of `forms.CharField` is currently transmitted
its min_length/max_length attributes to the related widget. It should not
do it for hidden widgets as the specs don't list those attributes as
permitted attributes.
https://www.w3.org/TR/html-markup/input.hidden.html
--
Ticket URL: <https://code.djangoproject.com/ticket/27015>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | 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 bmispelon):
* easy: 1 => 0
* stage: Unreviewed => Accepted
Comment:
That seems like a reasonable feature request.
I think the widget should still output those attribute if they've been
specified by the user (not everyone cares about valid HTML and they might
actually want to use those attribute with javascript for example).
This might make the implementation a bit tricky (which is why I removed
the "easy pickings" flag).
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:1>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by claudep):
If we simply test for the widget type in `CharField.widget_attrs()`, I
think this will do it (without touching any purposefully defined widget
attributes).
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:2>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by charettes):
Replying to [comment:2 claudep]:
> If we simply test for the widget type in `CharField.widget_attrs()`, I
think this will do it (without touching any purposefully defined widget
attributes).
Or rely on `Widget.is_hidden`?
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:3>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/7026 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:4>
Django
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:5>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
Technically, `minvalue/maxvalue` is only incompatible with `<input
type="hidden">`, not all widgets that might have `is_hidden = True`.
I know it's a bit far-fetched, and the current code is probably good
enough, but I think we should still mention this in the release notes,
just on the off-chance someone has some kind of custom widget that is
hidden but relies on `minvalue/maxvalue` being passed.
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:6>
Django
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by timgraham):
I'm skeptical as `Widget.is_hidden` isn't documented as something that's
meant to be overridden. If you have an example usage, I'll be interested
to see it.
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:7>
Django
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
I don't have a concrete example and don't care enough about this to stand
in the way of the current patch.
As far as I can tell `Widget.is_hidden` isn't documented (only
`BoundField.is_hidden` is) but that's never really stopped anyone in the
past from using things :)
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:8>
Django
Reporter: claudep | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"3569ba0333ef7f90640aa2042339451705d20114" 3569ba03]:
{{{
#!CommitTicketReference repository=""
revision="3569ba0333ef7f90640aa2042339451705d20114"
Fixed #27015 -- Prevented HTML-invalid minlength/maxlength on hidden
inputs
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27015#comment:9>