Problem/Motivation
CKEditor v47 is out: https://github.com/ckeditor/ckeditor5/releases/tag/v47.0.0
It appears to have a large amount of breaking changes, so ideally we will get this in early in the Drupal 11.3 cycle to avoid the problems we had updating late to CKEditor v45 in Drupal 11.2.
Steps to reproduce
Proposed resolution
Update to CKEditor v47.
Remaining tasks
None
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3538768
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:
- 3538768-10.6.x
changes, plain diff MR !13437
- 3538768-update-ckeditor-5
changes, plain diff MR !13416
Comments
Comment #2
longwaveWe didn't do this as quickly as I hoped, and v47 is now out; let's try to upgrade to that.
Comment #3
longwaveyarn build:ckeditor5-typesdeletescore/modules/ckeditor5/js/build/ckeditor5.types.jsdoc, with the following output:Need to figure out what has broken here.
Comment #5
longwaveThe big problem here is that DLL builds no longer seem to be supported, so this is apparently blocked on #3527914: Use UMD installation method for CKEditor5
Comment #6
longwaveActually DLL builds still work, we just need to make some plugin updates I think. A few exports have been changed and
injectCssTransitionDisabler()is nowCssTransitionDisablerMixinwhich I couldn't find an upgrade guide for, but seems to work.Comment #7
longwaveComment #8
longwaveI don't really understand the jsdoc build step, but in PHPStorm I can get contextual documentation about any of the imports in the custom plugins without the jsdoc file anyway. I think the break is because of https://ckeditor.com/docs/ckeditor5/latest/updating/nim-migration/migrat... - but perhaps we just don't need this any more?
Comment #9
longwaveDidn't mean to change status.
Comment #10
smustgrave commentedUgh another big update? Some contrib modules have barely bounced back from the last update that broke everything
Comment #11
longwaveThe changes aren't too bad, just if you are importing code from the API you might need to make some minor renames as per the linked document. The much bigger change comes in #3527914: Use UMD installation method for CKEditor5 which we will have to adapt to soon, because they are stopping support of the DLL loading method at the end of the year.
Some of the jsdoc types need updating as per https://ckeditor.com/docs/ckeditor5/latest/updating/nim-migration/migrat... - this is quite tedious and perhaps can be deferred to a followup. As an example
becomes
I'm still not clear about #8, because PHPStorm appears to discover some types whether or not the types whether
ckeditor5.types.jsdocexists or not.Comment #12
longwaveI think I fixed the jsdoc parser so it outputs the correct information for CKEditor5 v46 and above, still not sure if this is needed but I think it's as correct now as it was in the previous version.
I've also updated some of the existing jsdoc references for the renames that have occurred in CKEditor.
I think PHPStorm is now picking up the TypeScript types from CKEditor which is why I don't see much difference locally either way, but then e.g. this line still doesn't work locally and I don't understand why:
Still, that's probably out of scope here now given the jsdoc file appears to be largely correct again.
Comment #13
longwaveComment #14
godotislateMR changes for CKEditor libraries look pretty good, though I had one question on the MR about the changes to the jQuery UI map files and whether they are expected/in scope here.
Found the PR https://github.com/ckeditor/ckeditor5/pull/13222/files where
injectCssTransitionDisablerwas deprecated, and it was not really otherwise documented.I pulled this down locally, and I see this issue in PHPStorm as well, along with other ones like
@param {module:link/link~LinkDecoratorDefinition} decoratoror@param {module:core/editor/editor~Editor} editor. Though other ones like@extends module:core/command~Commandare picked up just fine, so not sure whether there's an issue with the JSDoc or PHPStorm.Also, it seems like the script might not pulling all the exports? For example, this was added to the JSDoc
But
export function getCookieandexport function setCookiein the same file aren't in the JSDoc, but this seems like an existing issue with the script and nothing introduced here. On a related note, there's this in the CKEditor v46 update docs, https://ckeditor.com/docs/ckeditor5/latest/updating/guides/update-to-46....Maybe JSDoc build script needs some updating, but I think that would be for a follow up.
Comment #15
longwaveOops, didn't spot those jQuery UI changes, I remember this happening before but I don't remember why, the only change is that
,"ignoreList":[]is added/removed from the end of the map. Reverted this.Re #14 I think the missing parts are a new bug in the jsdoc script. Previously it only looked for a single default export per file, now there can be multiple exports but I think we only pick up the first. Will see if I can improve this.
Comment #16
longwaveFixed up the jsdoc script to handle multiple exports per file. The second regex wasn't matching anything any more so I have removed that one.
Comment #17
godotislateOK, jsdoc is picking up all typedefs from each file now, so I think this looks good.
One note is that I don't think PHPStorm is integrating with the jsdoc file at all, and even with some Googling, I'm not finding how to work with jsdoc files in any IDE, but I'm not sure that needs to be addressed here.
Comment #18
longwaveI suspect we can remove the jsdoc script and helper file entirely, will open a followup to discuss that.
I think I figured out why PHPStorm works with some files and not others; it's picking up the TypeScript definitions instead of the jsdoc, but CKEditor doesn't have @module tags in all the TS definitions.
command.d.ts has
and this makes
@extends module:core/command~Commandwork. But editor.d.ts doesn't have a@moduletag so it doesn't get detected. Unsure if this is deliberate or not on CKEditor's part.Comment #20
catchCommitted/pushed to 11.x, thanks! Better to get this in asap.
I think we should probably backport this to 10.6.x too, so moving to 'to be ported'.
Comment #23
godotislate10.6.x MR https://git.drupalcode.org/project/drupal/-/merge_requests/13437
Comment #24
longwaveI get the same output in 10.6.x with
Comment #26
catchCommitted/pushed to 10.6.x, thanks!
Comment #29
catchComment #31
jurriaanroelofs commented@catch @longwave
I was wondering if beyond the dll build major changes, whether anyone noticed and tested there are breaking CSS styling changes from CKEditor5 v46. The CKEditor5 team decided to add opinionated CSS styles, affecting things like font-family and font-size within the editor.
In my DXPR Builder module this is especially annoying because we use the inline editor, but in Drupal core's use case this will probably still be a breaking change as users are accustomed to seeing the typography defined in Claro theme or Gin theme in their node form text editors, but now it will be Helvetica/Arial with different font sizes.
I created an issue about the problem here: https://github.com/ckeditor/ckeditor5/issues/19414