Incompatible with Drupal Core 10.5 / CKEditor 45

Since ckeditor 45.0.0 the icons were moved into its own package @ckeditor5/ckeditor5-icons

Therefore the import has to be adjusted in abbreviationview.js

From

import { icons } from 'ckeditor5/src/core';

To

import { icons } from 'ckeditor5/src/icons';

But I think this is a breaking change. Maybe we need a new branch with a new drupal core supporting version?

Hey @sunlix, I've checked this by applying the changes that you have mentioned but issues is still persist, could you please check again?

The issue was caused by missing or undefined icon references in the JavaScript build file located at:
`/ckeditor_abbreviation/js/build/abbreviation.js`

I modified the file by replacing the undefined icon references with fallback values.

Changes Made:

Replaced:
e.icons.check
With:
(e.icons && e.icons.check) || '<svg></svg>'

Replaced:
e.icons.cancel
With:
(e.icons && e.icons.cancel) || '<svg></svg>'

Replaced:
e.icons.eraser
With:
(e.icons && e.icons.eraser) || '<svg></svg>'

These changes ensure the script does not break when the icon is undefined by falling back to a basic empty '<svg>' element.

Precise error in console that i was getting with Drupal 11 and CKEditor 45.2:

"TypeError: e.icons is undefined"

Given that the icon package is at play here, that i'm not finding that error message on the internet anywhere, and that CKEditor Abbreviation is the only extra module for CKEditor we have, i think it has to be related.

The merge request deletes the built file, resulting in an unsurprising error of this kind: The requested URL /modules/contrib/ckeditor_abbreviation/js/build/abbreviation.js was not found on this server.

Given that it was committed before i presume there is a step committers should take to generate the file and then commit it?

And agree that a new major version release makes sense and we can have it version 6 require Drupal core 10.5 / 11.2 or later.

Really rotten of CKEditor to fail this badly on a breaking change; one thing for the plugin or even CKEditor to not work but for it to blank out all text on save is the worst possible behavior.

Sorry for the deleted build artifact. I messed up with the MR because the default branch is 4.0.x and I oversaw the target branch and rebased like a monkey :D

I added the build artifact with the changes, that should running again. But the tests are failing.
Didnt find out the reason. I will dig in later.

The current test piplines are running with Drupal 11.1.7 and 10.4.7.
So the tests are running against ckeditor 44 and not 45.

I added OPT_IN_TEST_NEXT_MINOR to test on 11.2.1-dev with ckeditor 45.

With ckeditor 45 the tests are green.

How to proceed here? Branch a new major and set new version constraint on 5.0.1?
Or is there a way to support ckeditor 44 and 45 in parallel?

Maintainer here! Sorry for the delay; it's been a busy few weeks for me.

I have been able to reproduce the problem on Drupal 10.5.0, and can confirm that the code in the patch fixes the issue; but I cannot merge code with broken tests. I also notice some issues in the code: I'll leave some comments in the merge request shortly. For these two reasons I am marking this issue as "Needs Work".

I'll do some more research into compatibility with forward and backward compatibility with Drupal core. There does appear to be some activity in #3523018: Update CKEditor 5 to 45.2.0 (core), and #3531194: D11.2 / D10.5: Uncaught CKEditorError: Cannot read properties of undefined (reading 'viewUid') (editor_advanced_link, another module that integrates with CKEditor) to keep an eye on.

While I was updating the issue summary, I tried to find a link to @ckeditor5/ckeditor5-icons in the NPM Package List, but couldn't. @sunlix, do you have a link to this new package?

@mparker17

sure.

NPM: https://www.npmjs.com/package/@ckeditor/ckeditor5-icons
GitHub: https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-icons

Please note that this package is new since 45.0.0 of ckeditor5.

(crosspost)

This is great! Works and the code change is straightforward. I would change to RTBC except for the decision on how to commit and release.

My 2¢ is to do the new major with different version constraints— and hope CKEditor does not make a habit of this level of breaking change!

@sunlix, thanks I've updated the issue summary.

My multiple version testing results...

In the table below:

• ✅ means that I was able to see the CKEditor on the node/edit page, I was able to enter an abbreviation using the CKEditor Abbreviation button, and the abbreviation worked on the node/view page
• ❌ means CKEditor did not load on the node/edit page, and thus I was not able to proceed further with testing. I've listed the error shown in the console (but not the stack trace, in part because my JS was minified)

All four sites were set up identically: Standard install profile; CKEditor Abbreviation added to Basic HTML text format; went to /node/add/article to test.

... I'll interpret this in another comment; just wanting to save it here.

Trying to debug the source of the errors led me to a fix!

I've reviewed the changes by @sunlix and I'm satisfied with them. I also contributed to the patch (with my Maintainer hat on), and I'm satisfied with my own changes — but another maintainer (@boromino, @sickness29) should review as well.

Moving to RTBC so another maintainer can review!

For what it's worth, I'm running the updated patch on an 11.2.0 site, and it seems to work fine for me.

@mparker17

would you like to have another commit to clean up the package.json a bit?

I have deleted the unused plugin so the dependencies are smaller and give a proper name and description along with the private flag.
Do you have a meaning?

  "name": "drupal-ckeditor-abbreviation",
  "private": true,
  "version": "5.0.1",
  "description": "Drupal CKEditor 5 integration for abbreviation plugin",

  ...

  "dependencies": {
    "@ckeditor/ckeditor5-core": "~45.2.0",
    "@ckeditor/ckeditor5-icons": "~45.2.0",
    "@ckeditor/ckeditor5-typing": "~45.2.0",
    "@ckeditor/ckeditor5-ui": "~45.2.0",
    "@ckeditor/ckeditor5-utils": "~45.2.0"
  }

@sunlix, Yes, please add another commit to clean up package.json!

Thank you very much!

Latest patch also fixed the problem for me.

I have tested on Drupal 11.2.2 and 10.4.8, the former also with translation. It works on both. Code looks good.

However, there seem to remain unresolved change requests?

@boromino, oops, thank you! All the threads were resolved but I forgot to /approve the merge request. I've done that now. Sorry for the confusion, and thanks for reviewing!

Awesome, thanks @boromino!

(sorry for the late reply, I was on vacation)