- First commit to issue fork.
- 🇵🇹Portugal gueguerreiro Lisboa
We were having the same CKEditor 4 issue on one of our websites.
Now knowing that it's because of advagg's "Enable preprocess on all JS" option, I was able to debug this further.
The problem technically happens because of this line:
// Force all JS to be preprocessed. if ($config->get('js_preprocess')) { foreach ($js as $path => &$values) { // However CKEditor must not be combined or errors *will* occur. if ($path == 'core/assets/vendor/ckeditor/ckeditor.js') { continue; } (.......) }
There's an exception built in to the "Enable preprocess on all JS" options, specifically for CKEditor, because it leads to errors.
The problem here is that, on some installs, because of 🐛 Remove the "replace" section from core/composer.json Fixed , and the fact that CKEditor 4 was deprecated in Drupal 9.5, it is causing the contributed CKEditor (drupal.org/project/ckeditor) to be installed and used instead. And when that happens, the CKEditor JS file being used will no longer be the one from Core, which means the exception is never triggered (because it explicitly checks for an hardcoded path to the JS file, that is expected to be in the core folder), and the errors occur (As the comment from the code promises would happen).
This is made more complicated because I believe this is only happening on **some** installs. Other installs are correctly still using Drupal Core's composer. I believe this is tied to contributed modules that depend on CKEditor 4. If you have one on your website, and are using Durpal 9.5 or higher, then the contributed CKEditor 4 will be installed. Otherwise you will stay on Drupal Core's CKEditor 4. But I'm not 100% sure on any of this.
With that in mind, in theory, the fix could be a simple change of that line of code to:
if ($path == 'core/assets/vendor/ckeditor/ckeditor.js' || $path == 'modules/contrib/ckeditor/vendor/ckeditor.js') { continue; }
So that it supports both cases.
Although, technically, not all users will be using the default "modules/contrib" structure for composer installed modules. So I'm creating a longer PR for those edge cases, that checks for the module path of CKEditor, and if it's not part of the core folder, we use that instead.
Locally, on one of our affected websites, this is working as intended. The CKEditor works even with the "Enable preprocess on all JS" option enabled.
- @gueguerreiro opened merge request.
- Status changed to Needs review
over 1 year ago 6:57pm 14 April 2023 - 🇵🇹Portugal gueguerreiro Lisboa
Opened a PR with the changes. Like I said in my comment, this is a longer PR to account for users that should work for user that are still using Drupal Core's version of CKEditor 4 (So no breaking change), and for users that have contributed modules installed anywhere other than modules/contrib.
The URL for the patch is https://git.drupalcode.org/project/advagg/-/merge_requests/23.diff, if someone needs to apply it through composer.