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.

Command icon 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

tanner.svg created an issue. See original summary.

tanner.svg’s picture

tanner.svg’s picture

Status: Active » Needs review
dydave’s picture

Version: 2.1.0-beta1 » 2.1.x-dev
Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community
Related issues: +#3111124: Force inclusion of drupal.collapse shim

Great 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!

liam morland made their first commit to this issue’s fork.

liam morland’s picture

Version: 2.1.x-dev » 2.x-dev

When this is resolved, the paragraph starting with "It's worth noting" on the project page should be deleted.

liam morland’s picture

Status: Reviewed & tested by the community » Fixed

Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

liam morland’s picture

Version: 2.x-dev » 2.1.x-dev
dydave’s picture

Thanks 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!

liam morland’s picture

Done

dydave’s picture

Thanks so much Liam (@liam morland) for your prompt, positive and constructive response! 🙏
(As always 😊)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.