Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge ckeditor5.compat.js from CKEditor 5 to into CKEditor 4 module #6341

Closed
Tracked by #4122
quicksketch opened this issue Dec 21, 2023 · 3 comments · Fixed by backdrop/backdrop#4612
Closed
Tracked by #4122

Comments

@quicksketch
Copy link
Member

Description of the bug

CKEditor 5 in contrib needed to provide a ckeditor5.compat.js file that fixed incompatibilities with CKEditor 4. Now that both are in core, we can move this file's contents into the core CKEditor 4 module.

@quicksketch
Copy link
Member Author

PR filed at backdrop/backdrop#4612.

To test:

  • Set up a text format that uses CKEditor 5
  • Also set up a text format that uses CKEditor 4
  • Visit /node/add/page and confirm that CKEditor 4 and CKEditor 5 can be attached to the body field by switching between text formats. They should switch back and forth with no errors.

To see the previous bug that is present without this code, comment out the added lines in ckeditor.js from the PR. Then when switching to CKEditor 5, you'll see that the body field gets two toolbars. Both the CKEditor 4 and 5 toolbar are active at the same time(!) This happens because CKEditor 4 by default automatically attaches itself to any contenteditable element on the page. See backdrop-contrib/ckeditor5#43 for more info when this issue was originally discovered.

@quicksketch quicksketch changed the title Move ckeditor5.compat.js from CKEditor 5 to 4 module Merge ckeditor5.compat.js from CKEditor 5 to into CKEditor 4 module Dec 21, 2023
@indigoxela
Copy link
Member

indigoxela commented Dec 21, 2023

This works like a charm. 👍
It's much easier now that CKEditor 5 is in core.

One tiny grammar thingy... but code-wise ready to merge.

@quicksketch
Copy link
Member Author

Thanks, I fixed that one typo and merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants