- 🇬🇧United Kingdom iancawthorne
I'm still finding this is an issue in Drupal 10. Attached is a re-rolled patch against 10.1.
After replacing "checkboxes" form element via AJAX, it's behaviours are not correctly attached to the checkboxes.
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.
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.
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.
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.
"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.
- Review the solution.
- Write release notes.
None.
None.
None.
Needs work
11.0 🔥
Last updated
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.
I'm still finding this is an issue in Drupal 10. Attached is a re-rolled patch against 10.1.