Problem/Motivation
I was conducting an inventory of the scripts attached to the DOM on various pages and noticed that the core/drupal.collapse library is being attached globally by the ckeditor_details module. This library introduces unnecessary dependencies (e.g., core/drupal.form, core/jquery), as well as the three scripts included by the library, on pages that do not contain details accordions.
This global attachment increases page load size and may impact performance on pages where these scripts are not needed.
Proposed resolution
Since Drupal 10 and newer versions do not support Internet Explorer 11 (IE11), perhaps we can just remove the .module file altogether, thus removing the library dependencies.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 3500324-remove-global-collapse-attachment.patch | 527 bytes | tanner.svg |
Issue fork ckeditor_details-3500324
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #5
tanner.svg commentedComment #6
tanner.svg commentedComment #7
dydave commentedGreat job Tanner (@tanner.svg)! Great catch!
Thanks a lot!
The patch works great!
The changes in the merge request MR !6 remove the code that was added in issue #3111124: Force inclusion of drupal.collapse shim, corresponding to commit:
https://git.drupalcode.org/project/ckeditor_details/-/commit/6d2cc1d71c4...
See file history:
https://git.drupalcode.org/project/ckeditor_details/-/commits/2.1.x/cked...
The code was added 5 years ago, probably to support older versions of Internet Explorer that could maybe not be supported anymore.
I didn't check in detail which versions of Internet Explorer could be impacted, but I would assume, going forward, in particular with D11 versions, support for older versions of Internet Explorer could probably be dropped.
Bumping issue to Major, as this is definitely a significant performance improvement for the module.
Moving to RTBC, as I would personally be in favor of dropping support for these older browser versions, if this could result in a significant performance improvement.
Feel free to let us know if you would have any questions on this issue or the merge request, we would surely be glad to help.
Thanks in advance!
Comment #9
liam morlandWhen this is resolved, the paragraph starting with "It's worth noting" on the project page should be deleted.
Comment #12
liam morlandThanks!
Comment #14
liam morlandComment #15
dydave commentedThanks a lot Liam (@liam morland) for all the work you've been putting in the module recently. 🙏
Sorry again to bother you with this, but it would be great if you could please just take a minute to grant us credit on this issue.
Once again, this is mostly important for the companies on whose behalf the work on this issue was contributed: testing, review, development, etc...
Thanks in advance!
Comment #16
liam morlandDone
Comment #17
dydave commentedThanks so much Liam (@liam morland) for your prompt, positive and constructive response! 🙏
(As always 😊)