The file /core/modules/ckeditor/js/ckeditor.js depends on drupal.displace because it accepts parameters like this (last line of file)

})(Drupal, Drupal.debounce, CKEDITOR, jQuery, Drupal.displace, Drupal.AjaxCommands);

However the library file /core/modules/ckeditor/ckeditor.libraries.yml is missing that dependency.

It seems to be difficult to hit this bug. I guess that the libraries must normally end up being loaded in the order that works by happy coincidence. I found that I hit the bug when using Bootstrap theme and Admin Toolbar, but there might be other factors involved too. The error message I see in Firefox Console is "TypeError: displace is undefined[Learn More]"

Hopefully the need for the dependency is so obvious that this patch can be accepted even if the maintainer can't hit the bug.

CommentFileSizeAuthor
#2 ckeditor.dependency-2869570-2.patch467 bytesAdamPS
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

AdamPS created an issue. See original summary.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS at AlbanyWeb commented
FileSize
ckeditor.dependency-2869570-2.patch467 bytes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

Excellent catch! You're absolutely right.

Adding test coverage for this seems disproportionate.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented
Title: Missing dependency on drupal.debounce » Missing dependency of drupal.ckeditor on drupal.displace
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented

@lauriii asked me to take a look. This seems like the kind of "oopsie" bug we're unlikely to ever have again, so I think we can get away without a dedicated test for this. I can't really picture a test that can check for this kind of problem anyway.

  • lauriii committed 7e3527e on 8.4.x 
    Issue #2869570 by AdamPS: Missing dependency of drupal.ckeditor on...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Druid commented
Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

CKEditor definitely has a dependency for core/drupal.displace which makes sense. Thank you @webchick for confirming that test coverage for this bug isn't worth the effort.

Committed 7e3527e and pushed to 8.4.x. Thanks!

This could be also potentially backported to 8.3.x.

AdamPS’s picture

AdamPS CreditAttribution: AdamPS at AlbanyWeb commented

Thanks everyone

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

I discussed the backporting of this with @catch, we think this is good for a backport to 8.3.x - whilst on some sites it might need a hook_update_N to take affect, if some has the bug all they have to do is clear the cache and the bug is fixed.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Druid commented
Status: Patch (to be ported) » Fixed

Thank you @alexpott and @catch for confirming. Cherry-picked 3230622 and pushed to 8.3.x.

  • lauriii committed 3230622 on 8.3.x 
    Issue #2869570 by AdamPS: Missing dependency of drupal.ckeditor on...
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

+1 to everything in #5, #7 and #9.

Thanks!

Status: Fixed » Closed (fixed)

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