CKeditor skin CSS 404 after "Enable preprocess on all JS"

Created on 16 November 2022, over 1 year ago
Updated 14 April 2023, about 1 year ago

After Drupal update 9.4.3 to 9.4.8, CKEditor breaks, so rich text fields are not usable anymore.

I get this error in the console on every page with CKEditor
Refused to apply style from 'https://xyz.nl/node/1458/skins/moono-lisa/editor.css?t=rlfq6q' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

Disabling "Enable preprocess on all JS" fixes this problem.

This was an issue in 4.x and after update to 5.0.0 the problem persists.

πŸ› Bug report
Status

Needs review

Version

5.0

Component

Compression/Minification

Created by

πŸ‡³πŸ‡±Netherlands Drumanuel

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 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 about 1 year ago
  • πŸ‡΅πŸ‡Ή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.

Production build 0.69.0 2024