AJAX behaviours are not correctly attached to checkboxes form element

Created on 8 April 2021, over 3 years ago
Updated 15 September 2023, about 1 year ago

Problem/Motivation

After replacing "checkboxes" form element via AJAX, it's behaviours are not correctly attached to the checkboxes.

Steps to reproduce

1. $ mkdir drupal
2. $ cd drupal/
3. $ curl -sSL https://www.drupal.org/download-latest/tar.gz | tar -xz --strip-components=1
4. $ php core/scripts/drupal quick-start standard --site-name BugDemo --host localhost --port 8080
5. Install the attached 'bugmodule' module.
6. Goto http://localhost:8080/bug.
7. Confirm that "checkboxes" element doesn't attach it's behaviours correclty.

Proposed resolution

The problem lies in randomized id attributes of the checkbox elements. See: https://www.drupal.org/node/2503277 .
With the single checkbox the id is not randomized so the behaviours attach loop catches it correctly.
With multiple checkboxes element all the ids of individual checkboxes are randomized. This leads to the problem.

See the problem

1. Disable JS aggregation and put a breakpoint here in the browsers js debugger. Breakpoint: /core/misc/ajax.js - line 27
2. Reload the page and check the 'settings.ajax' value in the debugger.

The selector for attaching behaviours is an id based on the key of this array. These keys are not updated when the id's are randomized so the selector doesn't match the element ids anymore.

Solution

We are moving away from HTML ids to use data-drupal-selector data attribute (see: https://www.drupal.org/node/2503277 ). For this reason we should also move to use it here. Still removing the id selector could break something and cause regression bugs. A simple way to avoid this is to just add the data attribute selector to the list. jQuery still selects the element only once. This allows a gradual shifting from using the ids for AJAX references and fixes this problem.

Objections

"We should fix the randomization of checkbox ids instead."

Answer: Why? The current solution fixes the bug and we don't know why the randomization is done in the first place. Removing it could cause regression bugs. Also note that this fix is generally beneficial. It could fix similar bugs elsewhere. If the randomization is unnecessary it could be a good idea to fix it. If the reader feels motivated to do it, go ahead. In any case the current fix should be done as it's in line with the goal to stop using HTML ids in AJAX references.

"jQuery selector performance will get worse."

Answer: This is a valid point. I ran a performance benchmark on https://jsbench.me/ with jQuery 3.5.1. Results compared to jQuery('#the-element'):
1. jQuery('[data-drupal-selector='the-element'], #the-element') was about 90%-100% percent slower.
2. jQuery('[data-drupal-selector='the-element']') was also about 90%-100% percent slower.
So yes, the performance is worse. But there's no way avoiding it when using data attribute selectors and Drupal is aiming for that. Having two selectors in the query doesn't seem to have noticeable performance impact. This could still affect the end user experience if the DOM is really big or there's a lot of ajax commands registered on the page. Some real world testing with heavy sites would be great. If you are worried, review it on your sites.
3. document.querySelector('[data-drupal-selector="the-element"]') is little bit faster. So there's at least some room for optimization.

"This causes unpredictable regression bugs."

Answer: At least some tests failed. This gives weight to the idea of fixing the bug on the checkboxes element. If there really are nasty regression bugs, maybe I need to try that route when I have time. Before that is hard to say which solution causes more problems.

Remaining tasks

- Review the solution.
- Write release notes.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Ajax 

Last updated about 17 hours ago

Created by

🇫🇮Finland jiv_e

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Production build 0.71.5 2024