Opened 6 years ago
Closed 6 years ago
#30440 closed Cleanup/optimization (fixed)
Temporary "required" attr is not removed if the browser doesn't supported ":valid" selector.
Reported by: | Ben Muschol | Owned by: | Ben Muschol |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | admin select selectfilter filteredselectfield |
Cc: | David Sanders | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
The code in question: https://github.com/django/django/blob/stable/2.2.x/django/contrib/admin/static/admin/js/SelectFilter2.js#L175
If an error is thrown on line 181, the attribute is never removed. This leads to incorrect form validation when submitting.
I believe this would be fixed by moving the "field.removeAttr('required');" outside of the try/catch block
Steps to reproduce:
- Use a browser which does not support :valid pseudo-selector (I am using Chrome 73 on Mac)
- Open a page with the filtered multiple select field
- Submit the form without making any changes
- The browser will erroneously preform the "Missing required element" validation
Change History (3)
comment:1 by , 6 years ago
Cc: | added |
---|---|
Summary: | "required" attribute is not removed if the browser doesn't supported ":valid" pseudo-selector in SelectFilter2.js → Temporary "required" attr is not removed if the browser doesn't supported ":valid" selector. |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
Version: | 2.2 → master |
comment:2 by , 6 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Sounds good, I've added a patch here: https://github.com/django/django/pull/11331
Let me know if I'm doing something wrong in terms of github style/ticket management
comment:3 by , 6 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fixed in 8eb413371462eb5d2ea2a5b305f5954ee112c527.
Agreed, we can add this small cleanup, you check comment from the author of original patch.