Opened 5 years ago

Closed 5 years ago

#31404 closed Cleanup/optimization (fixed)

Use HTML5 boolean attribute syntax for SelectMultiple's multiple attribute created by SelectFilter2.js.

Reported by: zriv Owned by: zriv
Component: contrib.admin Version: dev
Severity: Normal Keywords:
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

In Django 2.1, as ticket #29041, SelectMultiple's multiple attribute was changed to use the HTML5 boolean attribute syntax, rather than the XHTML multiple="multiple" syntax. However, the select elements that are created by SelectFilter2.js for "selector-chosen" divisions still use the XHTML syntax. This mix of syntax styles can be verified by looking at the "Change user" page in the Django admin. "Available groups" and "Available user permissions" (class="selector-available") use the HTML5 syntax, while "Chosen groups" and "Chosen user permissions" (class="selector-chosen") do not.

Based on previous changes to Django, I believe that this discrepancy is unintended. Therefore, this patch changes SelectFilter2.js to also create multiple attributes that use the HTML5 syntax, bringing it in line with the behavior of other parts of Django.

The syntax of the test covering this change may appear somewhat convoluted at first, but this was done purposefully to avoid using jQuery's .attr(). When .attr() is called to get certain types of attributes, jQuery "normalizes" the output to hide differences between browsers. In the case of multiple, it appears that jQuery always returns the value "multiple" if the multiple attribute is present, regardless of whether multiple was defined as a boolean or contains a string. (You can even set multiple to a random string and it will still return the value "multiple".) So, to test the actual value of the multiple attribute, the jQuery object is first translated to the DOM element, then getAttribute('multiple') is called on it.

As per the Django contribution documentation, I have edited the release notes, and have tried to mimic the wording of the release notes from ticket #29041.

Thank you for taking the time to review this issue and pull request. Any feedback is of course greatly appreciated.

Change History (5)

comment:1 by Carlton Gibson, 5 years ago

Has patch: set
Triage Stage: UnreviewedAccepted

PR — Thanks for the patch. Seems reasonable. Will Accept to review. 👍

comment:2 by zriv, 5 years ago

Triage Stage: AcceptedUnreviewed

comment:3 by Mariusz Felisiak, 5 years ago

Component: Formscontrib.admin
Owner: changed from nobody to zriv
Status: newassigned
Summary: Use HTML5 boolean attribute syntax for the multiple attribute of the select element in selector-chosen divisions.Use HTML5 boolean attribute syntax for SelectMultiple's multiple attribute created by SelectFilter2.js.
Triage Stage: UnreviewedAccepted
Last edited 5 years ago by Mariusz Felisiak (previous) (diff)

comment:4 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In b9336b7:

Fixed #31404 -- Changed selector-chosen's multiple attribute to HTML5 boolean syntax.

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